feat: adding max-retries limit to preupgrade retry limit

cosmovisor/README.md

@@ -43,6 +43,7 @@ All arguments passed to `cosmovisor` will be passed to the application binary (a
 * `DAEMON_RESTART_AFTER_UPGRADE` (*optional*), if set to `true`, will restart the subprocess with the same command-line arguments and flags (but with the new binary) after a successful upgrade. By default, `cosmovisor` stops running after an upgrade and requires the system administrator to manually restart it. Note that `cosmovisor` will not auto-restart the subprocess if there was an error.
 * `DAEMON_POLL_INTERVAL` is the interval length in milliseconds for polling the upgrade plan file. Default: 300.
 * `UNSAFE_SKIP_BACKUP` (defaults to `false`), if set to `false`, will backup the data before trying the upgrade. Otherwise it will upgrade directly without doing any backup. This is useful (and recommended) in case of failures and when needed to rollback. It is advised to use backup option, i.e., `UNSAFE_SKIP_BACKUP=false`
+* `PREUPGRADE_MAX_RETRIES` is the maximum number of times `pre-upgrade` is called in the application after exit status of `31`. More info on exit status codes [here](../docs/migrations/pre-upgrade.md). After the specified number of retires, if the pre-upgrade does not succeed, cosmovisor fails the upgrade. If the value is not set for this environment variable, the pre-upgrade runs until it receives a non `31` exit code.
 
 ### Folder Layout
 

cosmovisor/args.go

@@ -15,12 +15,13 @@ import (
 
 // environment variable names
 const (
-	envHome           = "DAEMON_HOME"
-	envName           = "DAEMON_NAME"
-	envDownloadBin    = "DAEMON_ALLOW_DOWNLOAD_BINARIES"
-	envRestartUpgrade = "DAEMON_RESTART_AFTER_UPGRADE"
-	envSkipBackup     = "UNSAFE_SKIP_BACKUP"
-	envInterval       = "DAEMON_POLL_INTERVAL"
+	envHome                 = "DAEMON_HOME"
+	envName                 = "DAEMON_NAME"
+	envDownloadBin          = "DAEMON_ALLOW_DOWNLOAD_BINARIES"
+	envRestartUpgrade       = "DAEMON_RESTART_AFTER_UPGRADE"
+	envSkipBackup           = "UNSAFE_SKIP_BACKUP"
+	envInterval             = "DAEMON_POLL_INTERVAL"
+	envPreupgradeMaxRetries = "PREUPGRADE_MAX_RETRIES"
 )
 
 const (
@@ -42,6 +43,7 @@ type Config struct {
 	RestartAfterUpgrade   bool
 	PollInterval          time.Duration
 	UnsafeSkipBackup      bool
+	PreupgradeMaxRetries  int
 
 	// currently running upgrade
 	currentUpgrade UpgradeInfo
@@ -146,6 +148,10 @@ func GetConfigFromEnv() (*Config, error) {
 	if err := cfg.validate(); err != nil {
 		return nil, err
 	}
+	if cfg.PreupgradeMaxRetries, err = strconv.Atoi(os.Getenv(envPreupgradeMaxRetries)); err != nil {
+		cfg.PreupgradeMaxRetries = -1 // Default if PREUPGRADE_MAX_RETRIES is not set
+	}
+
 	return cfg, nil
 }
 

cosmovisor/process.go

@@ -61,13 +61,14 @@ func (l Launcher) Run(args []string, stdout, stderr io.Writer) (bool, error) {
 	if err != nil || !needsUpdate {
 		return false, err
 	}
-	if err := doBackup(l.cfg); err != nil {
-		return false, err
-	}
 
-	if !SkipUpgrade(args, l.fw.currentInfo) {
-		err = doPreUpgrade(l.cfg)
-		if err != nil {
+	if !IsSkipUpgradeHeight(args, l.fw.currentInfo) {
+
+		if err := doBackup(l.cfg); err != nil {
+			return false, err
+		}
+
+		if err = doPreUpgrade(l.cfg, 0); err != nil {
 			return false, err
 		}
 	}
@@ -153,7 +154,10 @@ func doBackup(cfg *Config) error {
 }
 
 // doPreUpgrade runs the pre-upgrade command defined by the application
-func doPreUpgrade(cfg *Config) error {
+// cfg contains the cosmovisor config from env var
+// preupgrade_attempt is the current attempt at preupgrade by cosmovisor.
+func doPreUpgrade(cfg *Config, preupgrade_attempt int) error {
+
 	bin, err := cfg.CurrentBin()
 	preUpgradeCmd := exec.Command(bin, "pre-upgrade")
 
@@ -168,16 +172,21 @@ func doPreUpgrade(cfg *Config) error {
 			return fmt.Errorf("pre-upgrade command failed : %w", err)
 		}
 		if err.(*exec.ExitError).ProcessState.ExitCode() == 31 {
-			fmt.Println("pre-upgrade command failed. retrying.")
-			return doPreUpgrade(cfg)
+			preupgrade_attempt += 1
+			if cfg.PreupgradeMaxRetries != -1 && preupgrade_attempt >= cfg.PreupgradeMaxRetries {
+				return fmt.Errorf("pre-upgrade command failed. reached max attempt of retries. Err: %w", err)
+			}
+
+			fmt.Println("pre-upgrade command failed. retrying. Attempt: #", preupgrade_attempt)
+			return doPreUpgrade(cfg, preupgrade_attempt)
 		}
 	}
 	fmt.Println("pre-upgrade successful. continuing the upgrade.")
 	return nil
 }
 
-// skipUpgrade checks if pre-upgrade script must be run. If the height in the upgrade plan matches any of the heights provided in --safe-skip-upgrade, the script is not run
-func SkipUpgrade(args []string, upgradeInfo UpgradeInfo) bool {
+// IsSkipUpgradeHeight checks if pre-upgrade script must be run. If the height in the upgrade plan matches any of the heights provided in --safe-skip-upgrade, the script is not run
+func IsSkipUpgradeHeight(args []string, upgradeInfo UpgradeInfo) bool {
 	skipUpgradeHeights := UpgradeSkipHeights(args)
 	for _, h := range skipUpgradeHeights {
 		if h == int(upgradeInfo.Height) {

cosmovisor/process_test.go

@@ -160,7 +160,7 @@ func TestSkipUpgrade(t *testing.T) {
 	for i := range cases {
 		tc := cases[i]
 		require := require.New(t)
-		h := cosmovisor.SkipUpgrade(tc.args, tc.upgradeInfo)
+		h := cosmovisor.IsSkipUpgradeHeight(tc.args, tc.upgradeInfo)
 		require.Equal(h, tc.expectRes)
 	}
 }