feat: adding max-retries limit to preupgrade retry limit

cosmovisor/CHANGELOG.md

@@ -41,6 +41,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 + [\#8590](https://github.com/cosmos/cosmos-sdk/pull/8590) File watcher for cosmovisor. Instead of parsing logs from stdin and stderr, we watch the `<DAEMON_HOME>/data/upgrade-info.json` file updates using polling mechanism.
 + [\#10128](https://github.com/cosmos/cosmos-sdk/pull/10128) Change default value of `DAEMON_RESTART_AFTER_UPGRADE` to `true`.
++ [\#9973](https://github.com/cosmos/cosmos-sdk/pull/10056) Added support for pre-upgrade command in Cosmovisor to be called before the binary is upgraded. Added new environmental variable `DAEMON_PREUPGRADE_MAX_RETRIES` which holds the number of times to reattempt failing preupgrade.
+
 
 ### Improvements
 

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*, default = `true`), if `true`, will restart the subprocess with the same command-line arguments and flags (but with the new binary) after a successful upgrade. Otherwise (`false`), `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`
+* `DAEMON_PREUPGRADE_MAX_RETRIES` (defaults to `0`) 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. 
 
 ### 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 = "DAEMON_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,9 @@ func GetConfigFromEnv() (*Config, error) {
 	if err := cfg.validate(); err != nil {
 		return nil, err
 	}
+
+	cfg.PreupgradeMaxRetries, _ = strconv.Atoi(os.Getenv(envPreupgradeMaxRetries))
+
 	return cfg, nil
 }
 

cosmovisor/process.go

@@ -61,13 +61,13 @@ 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 +153,9 @@ 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
+// counter is the current attempt at preupgrade by cosmovisor.
+func doPreUpgrade(cfg *Config, counter int) error {
 	bin, err := cfg.CurrentBin()
 	preUpgradeCmd := exec.Command(bin, "pre-upgrade")
 
@@ -168,16 +170,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)
+			counter += 1
+			if counter >= 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: #", counter)
+			return doPreUpgrade(cfg, counter)
 		}
 	}
 	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)
 	}
 }