From 76e1fa30fb352800b3e79e08a76a474bc88b6193 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 12 Aug 2024 22:08:36 +0200 Subject: [PATCH] fix(cosmovisor): fix upgrade detection (#20585) --- .github/workflows/lint.yml | 13 + tools/cosmovisor/CHANGELOG.md | 9 +- tools/cosmovisor/README.md | 37 ++- tools/cosmovisor/args.go | 27 +- tools/cosmovisor/args_test.go | 2 +- tools/cosmovisor/cmd/cosmovisor/init.go | 43 +--- tools/cosmovisor/cmd/cosmovisor/init_test.go | 8 +- tools/cosmovisor/cmd/cosmovisor/main.go | 13 - tools/cosmovisor/cmd/cosmovisor/run.go | 2 +- tools/cosmovisor/go.mod | 4 +- tools/cosmovisor/process.go | 2 +- tools/cosmovisor/process_test.go | 250 +++++++++---------- tools/cosmovisor/scanner.go | 36 ++- 13 files changed, 210 insertions(+), 236 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 813453c5fb53..1b61aa2b1748 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -40,6 +40,19 @@ jobs: PATTERNS: | **/*.go *.go + !store/** + - name: run linting (short) + if: steps.lint_long.outcome == 'skipped' && env.GIT_DIFF + run: | + make lint + env: + GIT_DIFF: ${{ env.GIT_DIFF }} + LINT_DIFF: 1 + - uses: technote-space/get-diff-action@v6.1.2 + if: steps.lint_long.outcome == 'skipped' + with: + PATTERNS: | + store/** - name: run linting (short) if: steps.lint_long.outcome == 'skipped' && env.GIT_DIFF run: | diff --git a/tools/cosmovisor/CHANGELOG.md b/tools/cosmovisor/CHANGELOG.md index 220fc0745025..3931ccd3a3b2 100644 --- a/tools/cosmovisor/CHANGELOG.md +++ b/tools/cosmovisor/CHANGELOG.md @@ -36,13 +36,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] -## Features - -* [#19764](https://github.com/cosmos/cosmos-sdk/issues/19764) Use config file for cosmovisor configuration. +## v1.6.0 - 2024-08-12 ## Improvements -* [#20573](https://github.com/cosmos/cosmos-sdk/pull/20573) Bump `cosmossdk.io/x/upgrade` to v0.1.3 (including go-getter vulnerability fix) +* Bump `cosmossdk.io/x/upgrade` to v0.1.4 (including go-getter vulnerability fix) * [#19995](https://github.com/cosmos/cosmos-sdk/pull/19995): * `init command` writes the configuration to the config file only at the default path `DAEMON_HOME/cosmovisor/config.toml`. * Provide `--cosmovisor-config` flag with value as args to provide the path to the configuration file in the `run` command. `run --cosmovisor-config (other cmds with flags) ...`. @@ -52,6 +50,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Bug Fixes * [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions +* [#20585](https://github.com/cosmos/cosmos-sdk/pull/20585) Always parse stdout and stderr +* [#20585](https://github.com/cosmos/cosmos-sdk/pull/20585) Pass right home to command `status` +* [#20585](https://github.com/cosmos/cosmos-sdk/pull/20585) Fix upgrades applied automatically (check two casing of sync_info) ## v1.5.0 - 2023-07-17 diff --git a/tools/cosmovisor/README.md b/tools/cosmovisor/README.md index 06c7e61c9846..db15ab0a5d64 100644 --- a/tools/cosmovisor/README.md +++ b/tools/cosmovisor/README.md @@ -7,21 +7,22 @@ sidebar_position: 1 `cosmovisor` is a process manager for Cosmos SDK application binaries that automates application binary switch at chain upgrades. It polls the `upgrade-info.json` file that is created by the x/upgrade module at upgrade height, and then can automatically download the new binary, stop the current binary, switch from the old binary to the new one, and finally restart the node with the new binary. -* [Design](#design) -* [Contributing](#contributing) -* [Setup](#setup) - * [Installation](#installation) - * [Command Line Arguments And Environment Variables](#command-line-arguments-and-environment-variables) - * [Folder Layout](#folder-layout) -* [Usage](#usage) - * [Initialization](#initialization) - * [Detecting Upgrades](#detecting-upgrades) - * [Adding Upgrade Binary](#adding-upgrade-binary) - * [Auto-Download](#auto-download) -* [Example: SimApp Upgrade](#example-simapp-upgrade) - * [Chain Setup](#chain-setup) - * [Prepare Cosmovisor and Start the Chain](#prepare-cosmovisor-and-start-the-chain) - * [Update App](#update-app) +* [Cosmovisor](#cosmovisor) + * [Design](#design) + * [Contributing](#contributing) + * [Setup](#setup) + * [Installation](#installation) + * [Command Line Arguments And Environment Variables](#command-line-arguments-and-environment-variables) + * [Folder Layout](#folder-layout) + * [Usage](#usage) + * [Initialization](#initialization) + * [Detecting Upgrades](#detecting-upgrades) + * [Adding Upgrade Binary](#adding-upgrade-binary) + * [Auto-Download](#auto-download) + * [Example: SimApp Upgrade](#example-simapp-upgrade) + * [Chain Setup](#chain-setup) + * [Prepare Cosmovisor and Start the Chain](#prepare-cosmovisor-and-start-the-chain) + * [Update App](#update-app) ## Design @@ -87,11 +88,7 @@ The first argument passed to `cosmovisor` is the action for `cosmovisor` to take All arguments passed to `cosmovisor run` will be passed to the application binary (as a subprocess). `cosmovisor` will return `/dev/stdout` and `/dev/stderr` of the subprocess as its own. For this reason, `cosmovisor run` cannot accept any command-line arguments other than those available to the application binary. -:::warning -Use of `cosmovisor` without one of the action arguments is deprecated. For backwards compatibility, if the first argument is not an action argument, `run` is assumed. However, this fallback might be removed in future versions, so it is recommended that you always provide `run`. -::: - -`cosmovisor` reads its configuration from environment variables: +`cosmovisor` reads its configuration from environment variables, or its configuration file (use `--cosmovisor-config `): * `DAEMON_HOME` is the location where the `cosmovisor/` directory is kept that contains the genesis binary, the upgrade binaries, and any additional auxiliary files associated with each binary (e.g. `$HOME/.gaiad`, `$HOME/.regend`, `$HOME/.simd`, etc.). * `DAEMON_NAME` is the name of the binary itself (e.g. `gaiad`, `regend`, `simd`, etc.). diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 40e2553631b3..be94fbadaf6c 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -156,12 +156,12 @@ func (cfg *Config) CurrentBin() (string, error) { return binpath, nil } -// GetConfigFromFile will read the configuration from the file at the given path. -// If the file path is not provided, it will try to read the configuration from the ENV variables. +// GetConfigFromFile will read the configuration from the config file at the given path. +// If the file path is not provided, it will read the configuration from the ENV variables. // If a file path is provided and ENV variables are set, they will override the values in the file. func GetConfigFromFile(filePath string) (*Config, error) { if filePath == "" { - return GetConfigFromEnv() + return GetConfigFromEnv(false) } // ensure the file exist @@ -169,18 +169,19 @@ func GetConfigFromFile(filePath string) (*Config, error) { return nil, fmt.Errorf("config not found: at %s : %w", filePath, err) } + v := viper.New() // read the configuration from the file - viper.SetConfigFile(filePath) + v.SetConfigFile(filePath) // load the env variables // if the env variable is set, it will override the value provided by the config - viper.AutomaticEnv() + v.AutomaticEnv() - if err := viper.ReadInConfig(); err != nil { + if err := v.ReadInConfig(); err != nil { return nil, fmt.Errorf("failed to read config file: %w", err) } cfg := &Config{} - if err := viper.Unmarshal(cfg); err != nil { + if err := v.Unmarshal(cfg); err != nil { return nil, fmt.Errorf("failed to unmarshal configuration: %w", err) } @@ -203,7 +204,7 @@ func GetConfigFromFile(filePath string) (*Config, error) { // GetConfigFromEnv will read the environmental variables into a config // and then validate it is reasonable -func GetConfigFromEnv() (*Config, error) { +func GetConfigFromEnv(skipValidate bool) (*Config, error) { var errs []error cfg := &Config{ Home: os.Getenv(EnvHome), @@ -281,9 +282,11 @@ func GetConfigFromEnv() (*Config, error) { errs = append(errs, fmt.Errorf("%s could not be parsed to int: %w", EnvPreupgradeMaxRetries, err)) } - errs = append(errs, cfg.validate()...) - if len(errs) > 0 { - return nil, errors.Join(errs...) + if !skipValidate { + errs = append(errs, cfg.validate()...) + if len(errs) > 0 { + return nil, errors.Join(errs...) + } } return cfg, nil @@ -574,7 +577,7 @@ func (cfg Config) DetailString() string { return sb.String() } -// Export exports the configuration to a file at the given path. +// Export exports the configuration to a file at the cosmovisor root directory. func (cfg Config) Export() (string, error) { // always use the default path path := filepath.Clean(cfg.DefaultCfgPath()) diff --git a/tools/cosmovisor/args_test.go b/tools/cosmovisor/args_test.go index b379720763f0..74b75841ec5d 100644 --- a/tools/cosmovisor/args_test.go +++ b/tools/cosmovisor/args_test.go @@ -769,7 +769,7 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { s.T().Run(tc.name, func(t *testing.T) { s.setEnv(t, &tc.envVals) - cfg, err := GetConfigFromEnv() + cfg, err := GetConfigFromEnv(false) if tc.expectedErrCount == 0 { assert.NoError(t, err) } else if assert.Error(t, err) { diff --git a/tools/cosmovisor/cmd/cosmovisor/init.go b/tools/cosmovisor/cmd/cosmovisor/init.go index 66d99b68032e..5f78e2845c02 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init.go +++ b/tools/cosmovisor/cmd/cosmovisor/init.go @@ -6,7 +6,6 @@ import ( "io" "os" "path/filepath" - "time" "github.com/spf13/cobra" @@ -45,11 +44,18 @@ func InitializeCosmovisor(logger log.Logger, args []string) error { case exeInfo.IsDir(): return errors.New("invalid path to executable: must not be a directory") } - cfg, err := getConfigForInitCmd() + + // skipping validation to not check if directories exist + cfg, err := cosmovisor.GetConfigFromEnv(true) if err != nil { return err } + // process to minimal validation + if err := minConfigValidate(cfg); err != nil { + return err + } + if logger == nil { logger = cfg.Logger(os.Stdout) } @@ -98,37 +104,13 @@ func InitializeCosmovisor(logger log.Logger, args []string) error { if err != nil { return fmt.Errorf("failed to export configuration: %w", err) } - - logger.Info(fmt.Sprintf("config file present at: %s", filePath)) + logger.Info(fmt.Sprintf("cosmovisor config.toml created at: %s", filePath)) return nil } -// getConfigForInitCmd gets just the configuration elements needed to initialize cosmovisor. -func getConfigForInitCmd() (*cosmovisor.Config, error) { +func minConfigValidate(cfg *cosmovisor.Config) error { var errs []error - - // Note: Not using GetConfigFromEnv here because that checks that the directories already exist. - // We also don't care about the rest of the configuration stuff in here. - cfg := &cosmovisor.Config{ - Home: os.Getenv(cosmovisor.EnvHome), - Name: os.Getenv(cosmovisor.EnvName), - } - - var err error - if cfg.ColorLogs, err = cosmovisor.BooleanOption(cosmovisor.EnvColorLogs, true); err != nil { - errs = append(errs, err) - } - - if cfg.TimeFormatLogs, err = cosmovisor.TimeFormatOptionFromEnv(cosmovisor.EnvTimeFormatLogs, time.Kitchen); err != nil { - errs = append(errs, err) - } - - // if backup is not set, use the home directory - if cfg.DataBackupPath == "" { - cfg.DataBackupPath = cfg.Home - } - if len(cfg.Name) == 0 { errs = append(errs, fmt.Errorf("%s is not set", cosmovisor.EnvName)) } @@ -140,10 +122,7 @@ func getConfigForInitCmd() (*cosmovisor.Config, error) { errs = append(errs, fmt.Errorf("%s must be an absolute path", cosmovisor.EnvHome)) } - if len(errs) > 0 { - return cfg, errors.Join(errs...) - } - return cfg, nil + return errors.Join(errs...) } // copyFile copies the file at the given source to the given destination. diff --git a/tools/cosmovisor/cmd/cosmovisor/init_test.go b/tools/cosmovisor/cmd/cosmovisor/init_test.go index 95d971ab08ed..cea1349e3b39 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/init_test.go @@ -502,7 +502,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { fmt.Sprintf("making sure %q is executable", genBinExe), "checking on the current symlink and creating it if needed", fmt.Sprintf("the current symlink points to: %q", genBinExe), - fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), + fmt.Sprintf("cosmovisor config.toml created at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(s.T(), env) @@ -555,7 +555,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { fmt.Sprintf("the %q file already exists", genBinDirExe), fmt.Sprintf("making sure %q is executable", genBinDirExe), fmt.Sprintf("the current symlink points to: %q", genBinDirExe), - fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), + fmt.Sprintf("cosmovisor config.toml created at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(t, env) @@ -588,7 +588,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { fmt.Sprintf("copying executable into place: %q", genBinExe), fmt.Sprintf("making sure %q is executable", genBinExe), fmt.Sprintf("the current symlink points to: %q", genBinExe), - fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), + fmt.Sprintf("cosmovisor config.toml created at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(t, env) @@ -620,7 +620,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { require.NoError(t, err, "calling InitializeCosmovisor") bufferBz := buffer.Collect() bufferStr := string(bufferBz) - assert.Contains(t, bufferStr, fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt))) + assert.Contains(t, bufferStr, fmt.Sprintf("cosmovisor config.toml created at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt))) }) } diff --git a/tools/cosmovisor/cmd/cosmovisor/main.go b/tools/cosmovisor/cmd/cosmovisor/main.go index 8ed5a2e9f9ed..294dd66f7165 100644 --- a/tools/cosmovisor/cmd/cosmovisor/main.go +++ b/tools/cosmovisor/cmd/cosmovisor/main.go @@ -6,20 +6,7 @@ import ( ) func main() { - // error logger used only during configuration phase - cfg, _ := getConfigForInitCmd() - logger := cfg.Logger(os.Stderr) - if err := NewRootCmd().ExecuteContext(context.Background()); err != nil { - if errMulti, ok := err.(interface{ Unwrap() []error }); ok { - err := errMulti.Unwrap() - for _, e := range err { - logger.Error("", "error", e) - } - } else { - logger.Error("", "error", err) - } - os.Exit(1) } } diff --git a/tools/cosmovisor/cmd/cosmovisor/run.go b/tools/cosmovisor/cmd/cosmovisor/run.go index 6aa938c7d780..0e1513cf6766 100644 --- a/tools/cosmovisor/cmd/cosmovisor/run.go +++ b/tools/cosmovisor/cmd/cosmovisor/run.go @@ -13,7 +13,7 @@ var runCmd = &cobra.Command{ Use: "run", Short: "Run an APP command.", Long: `Run an APP command. This command is intended to be used by the cosmovisor binary. -Provide cosmovisor config file path in command args or set env variables to load configuration. +Provide '--cosmovisor-config' file path in command args or set env variables to load configuration. `, SilenceUsage: true, DisableFlagParsing: true, diff --git a/tools/cosmovisor/go.mod b/tools/cosmovisor/go.mod index c404d40619b3..fe89a5c39c17 100644 --- a/tools/cosmovisor/go.mod +++ b/tools/cosmovisor/go.mod @@ -1,8 +1,6 @@ module cosmossdk.io/tools/cosmovisor -go 1.22.2 - -toolchain go1.22.4 +go 1.22.4 require ( cosmossdk.io/log v1.4.0 diff --git a/tools/cosmovisor/process.go b/tools/cosmovisor/process.go index dfda812efe49..c288515005fd 100644 --- a/tools/cosmovisor/process.go +++ b/tools/cosmovisor/process.go @@ -28,7 +28,7 @@ type Launcher struct { } func NewLauncher(logger log.Logger, cfg *Config) (Launcher, error) { - fw, err := newUpgradeFileWatcher(cfg, logger) + fw, err := newUpgradeFileWatcher(cfg) if err != nil { return Launcher{}, err } diff --git a/tools/cosmovisor/process_test.go b/tools/cosmovisor/process_test.go index aa587042668c..09658f265cdb 100644 --- a/tools/cosmovisor/process_test.go +++ b/tools/cosmovisor/process_test.go @@ -13,259 +13,244 @@ import ( "time" "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" "cosmossdk.io/log" "cosmossdk.io/tools/cosmovisor" upgradetypes "cosmossdk.io/x/upgrade/types" ) -type processTestSuite struct { - suite.Suite -} - -func TestProcessTestSuite(t *testing.T) { - suite.Run(t, new(processTestSuite)) -} - // TestLaunchProcess will try running the script a few times and watch upgrades work properly // and args are passed through -func (s *processTestSuite) TestLaunchProcess() { +func TestLaunchProcess(t *testing.T) { // binaries from testdata/validate directory - require := s.Require() - home := copyTestData(s.T(), "validate") + home := copyTestData(t, "validate") cfg := &cosmovisor.Config{Home: home, Name: "dummyd", PollInterval: 20, UnsafeSkipBackup: true} - logger := log.NewTestLogger(s.T()).With(log.ModuleKey, "cosmosvisor") + logger := log.NewTestLogger(t).With(log.ModuleKey, "cosmosvisor") // should run the genesis binary and produce expected output stdout, stderr := newBuffer(), newBuffer() currentBin, err := cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.GenesisBin(), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.GenesisBin(), currentBin) launcher, err := cosmovisor.NewLauncher(logger, cfg) - require.NoError(err) + require.NoError(t, err) upgradeFile := cfg.UpgradeInfoFilePath() args := []string{"foo", "bar", "1234", upgradeFile} doUpgrade, err := launcher.Run(args, stdout, stderr) - require.NoError(err) - require.True(doUpgrade) - require.Equal("", stderr.String()) - require.Equal(fmt.Sprintf("Genesis foo bar 1234 %s\nUPGRADE \"chain2\" NEEDED at height: 49: {}\n", upgradeFile), stdout.String()) + require.NoError(t, err) + require.True(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, fmt.Sprintf("Genesis foo bar 1234 %s\nUPGRADE \"chain2\" NEEDED at height: 49: {}\n", upgradeFile), stdout.String()) // ensure this is upgraded now and produces new output currentBin, err = cfg.CurrentBin() - require.NoError(err) + require.NoError(t, err) - require.Equal(cfg.UpgradeBin("chain2"), currentBin) + require.Equal(t, cfg.UpgradeBin("chain2"), currentBin) args = []string{"second", "run", "--verbose"} stdout.Reset() stderr.Reset() doUpgrade, err = launcher.Run(args, stdout, stderr) - require.NoError(err) - require.False(doUpgrade) - require.Equal("", stderr.String()) - require.Equal("Chain 2 is live!\nArgs: second run --verbose\nFinished successfully\n", stdout.String()) + require.NoError(t, err) + require.False(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, "Chain 2 is live!\nArgs: second run --verbose\nFinished successfully\n", stdout.String()) // ended without other upgrade - require.Equal(cfg.UpgradeBin("chain2"), currentBin) + require.Equal(t, cfg.UpgradeBin("chain2"), currentBin) } // TestPlanDisableRecase will test upgrades without lower case plan names -func (s *processTestSuite) TestPlanDisableRecase() { +func TestPlanDisableRecase(t *testing.T) { // binaries from testdata/validate directory - require := s.Require() - home := copyTestData(s.T(), "norecase") + home := copyTestData(t, "norecase") cfg := &cosmovisor.Config{Home: home, Name: "dummyd", PollInterval: 20, UnsafeSkipBackup: true, DisableRecase: true} - logger := log.NewTestLogger(s.T()).With(log.ModuleKey, "cosmosvisor") + logger := log.NewTestLogger(t).With(log.ModuleKey, "cosmosvisor") // should run the genesis binary and produce expected output stdout, stderr := newBuffer(), newBuffer() currentBin, err := cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.GenesisBin(), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.GenesisBin(), currentBin) launcher, err := cosmovisor.NewLauncher(logger, cfg) - require.NoError(err) + require.NoError(t, err) upgradeFile := cfg.UpgradeInfoFilePath() args := []string{"foo", "bar", "1234", upgradeFile} doUpgrade, err := launcher.Run(args, stdout, stderr) - require.NoError(err) - require.True(doUpgrade) - require.Equal("", stderr.String()) - require.Equal(fmt.Sprintf("Genesis foo bar 1234 %s\nUPGRADE \"Chain2\" NEEDED at height: 49: {}\n", upgradeFile), stdout.String()) + require.NoError(t, err) + require.True(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, fmt.Sprintf("Genesis foo bar 1234 %s\nUPGRADE \"Chain2\" NEEDED at height: 49: {}\n", upgradeFile), stdout.String()) // ensure this is upgraded now and produces new output currentBin, err = cfg.CurrentBin() - require.NoError(err) + require.NoError(t, err) - require.Equal(cfg.UpgradeBin("Chain2"), currentBin) + require.Equal(t, cfg.UpgradeBin("Chain2"), currentBin) args = []string{"second", "run", "--verbose"} stdout.Reset() stderr.Reset() doUpgrade, err = launcher.Run(args, stdout, stderr) - require.NoError(err) - require.False(doUpgrade) - require.Equal("", stderr.String()) - require.Equal("Chain 2 is live!\nArgs: second run --verbose\nFinished successfully\n", stdout.String()) + require.NoError(t, err) + require.False(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, "Chain 2 is live!\nArgs: second run --verbose\nFinished successfully\n", stdout.String()) // ended without other upgrade - require.Equal(cfg.UpgradeBin("Chain2"), currentBin) + require.Equal(t, cfg.UpgradeBin("Chain2"), currentBin) } -func (s *processTestSuite) TestLaunchProcessWithRestartDelay() { +func TestLaunchProcessWithRestartDelay(t *testing.T) { // binaries from testdata/validate directory - require := s.Require() - home := copyTestData(s.T(), "validate") + home := copyTestData(t, "validate") cfg := &cosmovisor.Config{Home: home, Name: "dummyd", RestartDelay: 5 * time.Second, PollInterval: 20, UnsafeSkipBackup: true} - logger := log.NewTestLogger(s.T()).With(log.ModuleKey, "cosmosvisor") + logger := log.NewTestLogger(t).With(log.ModuleKey, "cosmosvisor") // should run the genesis binary and produce expected output stdout, stderr := newBuffer(), newBuffer() currentBin, err := cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.GenesisBin(), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.GenesisBin(), currentBin) launcher, err := cosmovisor.NewLauncher(logger, cfg) - require.NoError(err) + require.NoError(t, err) upgradeFile := cfg.UpgradeInfoFilePath() start := time.Now() doUpgrade, err := launcher.Run([]string{"foo", "bar", "1234", upgradeFile}, stdout, stderr) - require.NoError(err) - require.True(doUpgrade) + require.NoError(t, err) + require.True(t, doUpgrade) // may not be the best way but the fastest way to check we meet the delay // in addition to comparing both the runtime of this test and TestLaunchProcess in addition if time.Since(start) < cfg.RestartDelay { - require.FailNow("restart delay not met") + require.FailNow(t, "restart delay not met") } } // TestPlanShutdownGrace will test upgrades without lower case plan names -func (s *processTestSuite) TestPlanShutdownGrace() { +func TestPlanShutdownGrace(t *testing.T) { // binaries from testdata/validate directory - require := s.Require() - home := copyTestData(s.T(), "dontdie") + home := copyTestData(t, "dontdie") cfg := &cosmovisor.Config{Home: home, Name: "dummyd", PollInterval: 20, UnsafeSkipBackup: true, ShutdownGrace: 2 * time.Second} - logger := log.NewTestLogger(s.T()).With(log.ModuleKey, "cosmosvisor") + logger := log.NewTestLogger(t).With(log.ModuleKey, "cosmosvisor") // should run the genesis binary and produce expected output stdout, stderr := newBuffer(), newBuffer() currentBin, err := cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.GenesisBin(), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.GenesisBin(), currentBin) launcher, err := cosmovisor.NewLauncher(logger, cfg) - require.NoError(err) + require.NoError(t, err) upgradeFile := cfg.UpgradeInfoFilePath() args := []string{"foo", "bar", "1234", upgradeFile} doUpgrade, err := launcher.Run(args, stdout, stderr) - require.NoError(err) - require.True(doUpgrade) - require.Equal("", stderr.String()) - require.Equal(fmt.Sprintf("Genesis foo bar 1234 %s\nUPGRADE \"Chain2\" NEEDED at height: 49: {}\nWARN Need Flush\nFlushed\n", upgradeFile), stdout.String()) + require.NoError(t, err) + require.True(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, fmt.Sprintf("Genesis foo bar 1234 %s\nUPGRADE \"Chain2\" NEEDED at height: 49: {}\nWARN Need Flush\nFlushed\n", upgradeFile), stdout.String()) // ensure this is upgraded now and produces new output currentBin, err = cfg.CurrentBin() - require.NoError(err) + require.NoError(t, err) - require.Equal(cfg.UpgradeBin("chain2"), currentBin) + require.Equal(t, cfg.UpgradeBin("chain2"), currentBin) args = []string{"second", "run", "--verbose"} stdout.Reset() stderr.Reset() doUpgrade, err = launcher.Run(args, stdout, stderr) - require.NoError(err) - require.False(doUpgrade) - require.Equal("", stderr.String()) - require.Equal("Chain 2 is live!\nArgs: second run --verbose\nFinished successfully\n", stdout.String()) + require.NoError(t, err) + require.False(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, "Chain 2 is live!\nArgs: second run --verbose\nFinished successfully\n", stdout.String()) // ended without other upgrade - require.Equal(cfg.UpgradeBin("chain2"), currentBin) + require.Equal(t, cfg.UpgradeBin("chain2"), currentBin) } // TestLaunchProcess will try running the script a few times and watch upgrades work properly // and args are passed through -func (s *processTestSuite) TestLaunchProcessWithDownloads() { +func TestLaunchProcessWithDownloads(t *testing.T) { // test case upgrade path (binaries from testdata/download directory): // genesis -> chain2-zip_bin // chain2-zip_bin -> ref_to_chain3-zip_dir.json = (json for the next download instructions) -> chain3-zip_dir // chain3-zip_dir - doesn't upgrade - require := s.Require() - home := copyTestData(s.T(), "download") + home := copyTestData(t, "download") cfg := &cosmovisor.Config{Home: home, Name: "autod", AllowDownloadBinaries: true, PollInterval: 100, UnsafeSkipBackup: true} - logger := log.NewTestLogger(s.T()).With(log.ModuleKey, "cosmovisor") + logger := log.NewTestLogger(t).With(log.ModuleKey, "cosmovisor") upgradeFilename := cfg.UpgradeInfoFilePath() // should run the genesis binary and produce expected output currentBin, err := cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.GenesisBin(), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.GenesisBin(), currentBin) launcher, err := cosmovisor.NewLauncher(logger, cfg) - require.NoError(err) + require.NoError(t, err) stdout, stderr := newBuffer(), newBuffer() args := []string{"some", "args", upgradeFilename} doUpgrade, err := launcher.Run(args, stdout, stderr) - require.NoError(err) - require.True(doUpgrade) - require.Equal("", stderr.String()) - require.Equal("Genesis autod. Args: some args "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain2" NEEDED at height: 49: zip_binary`+"\n", stdout.String()) + require.NoError(t, err) + require.True(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, "Genesis autod. Args: some args "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain2" NEEDED at height: 49: zip_binary`+"\n", stdout.String()) currentBin, err = cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.UpgradeBin("chain2"), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.UpgradeBin("chain2"), currentBin) // start chain2 stdout.Reset() stderr.Reset() args = []string{"run", "--fast", upgradeFilename} doUpgrade, err = launcher.Run(args, stdout, stderr) - require.NoError(err) + require.NoError(t, err) - require.Equal("", stderr.String()) - require.Equal("Chain 2 from zipped binary\nArgs: run --fast "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain3" NEEDED at height: 936: ref_to_chain3-zip_dir.json module=main`+"\n", stdout.String()) + require.Empty(t, stderr.String()) + require.Equal(t, "Chain 2 from zipped binary\nArgs: run --fast "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain3" NEEDED at height: 936: ref_to_chain3-zip_dir.json module=main`+"\n", stdout.String()) // ended with one more upgrade - require.True(doUpgrade) + require.True(t, doUpgrade) currentBin, err = cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.UpgradeBin("chain3"), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.UpgradeBin("chain3"), currentBin) // run the last chain args = []string{"end", "--halt", upgradeFilename} stdout.Reset() stderr.Reset() doUpgrade, err = launcher.Run(args, stdout, stderr) - require.NoError(err) - require.False(doUpgrade) - require.Equal("", stderr.String()) - require.Equal("Chain 3 from zipped directory\nArgs: end --halt "+upgradeFilename+"\n", stdout.String()) + require.NoError(t, err) + require.False(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, "Chain 3 from zipped directory\nArgs: end --halt "+upgradeFilename+"\n", stdout.String()) // and this doesn't upgrade currentBin, err = cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.UpgradeBin("chain3"), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.UpgradeBin("chain3"), currentBin) } // TestLaunchProcessWithDownloadsAndMissingPreupgrade will try running the script a few times and watch upgrades work properly // and args are passed through -func (s *processTestSuite) TestLaunchProcessWithDownloadsAndMissingPreupgrade() { +func TestLaunchProcessWithDownloadsAndMissingPreupgrade(t *testing.T) { // test case upgrade path (binaries from testdata/download directory): // genesis -> chain2-zip_bin // chain2-zip_bin -> ref_to_chain3-zip_dir.json = (json for the next download instructions) -> chain3-zip_dir // chain3-zip_dir - doesn't upgrade - require := s.Require() - home := copyTestData(s.T(), "download") + home := copyTestData(t, "download") cfg := &cosmovisor.Config{ Home: home, Name: "autod", @@ -274,34 +259,33 @@ func (s *processTestSuite) TestLaunchProcessWithDownloadsAndMissingPreupgrade() UnsafeSkipBackup: true, CustomPreUpgrade: "missing.sh", } - logger := log.NewTestLogger(s.T()).With(log.ModuleKey, "cosmovisor") + logger := log.NewTestLogger(t).With(log.ModuleKey, "cosmovisor") upgradeFilename := cfg.UpgradeInfoFilePath() // should run the genesis binary and produce expected output currentBin, err := cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.GenesisBin(), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.GenesisBin(), currentBin) launcher, err := cosmovisor.NewLauncher(logger, cfg) - require.NoError(err) + require.NoError(t, err) // Missing Preupgrade Script stdout, stderr := newBuffer(), newBuffer() args := []string{"some", "args", upgradeFilename} _, err = launcher.Run(args, stdout, stderr) - require.ErrorContains(err, "missing.sh") - require.ErrorIs(err, fs.ErrNotExist) + require.ErrorContains(t, err, "missing.sh") + require.ErrorIs(t, err, fs.ErrNotExist) } // TestLaunchProcessWithDownloadsAndPreupgrade will try running the script a few times and watch upgrades work properly // and args are passed through -func (s *processTestSuite) TestLaunchProcessWithDownloadsAndPreupgrade() { +func TestLaunchProcessWithDownloadsAndPreupgrade(t *testing.T) { // test case upgrade path (binaries from testdata/download directory): // genesis -> chain2-zip_bin // chain2-zip_bin -> ref_to_chain3-zip_dir.json = (json for the next download instructions) -> chain3-zip_dir // chain3-zip_dir - doesn't upgrade - require := s.Require() - home := copyTestData(s.T(), "download") + home := copyTestData(t, "download") cfg := &cosmovisor.Config{ Home: home, Name: "autod", @@ -316,58 +300,58 @@ func (s *processTestSuite) TestLaunchProcessWithDownloadsAndPreupgrade() { // should run the genesis binary and produce expected output currentBin, err := cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.GenesisBin(), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.GenesisBin(), currentBin) launcher, err := cosmovisor.NewLauncher(logger, cfg) - require.NoError(err) + require.NoError(t, err) stdout, stderr := newBuffer(), newBuffer() args := []string{"some", "args", upgradeFilename} doUpgrade, err := launcher.Run(args, stdout, stderr) - require.NoError(err) - require.True(doUpgrade) - require.Equal("", stderr.String()) - require.Equal("Genesis autod. Args: some args "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain2" NEEDED at height: 49: zip_binary`+"\n", stdout.String()) + require.NoError(t, err) + require.True(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, "Genesis autod. Args: some args "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain2" NEEDED at height: 49: zip_binary`+"\n", stdout.String()) currentBin, err = cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.UpgradeBin("chain2"), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.UpgradeBin("chain2"), currentBin) // should have preupgrade.sh results - require.FileExists(filepath.Join(home, "upgrade_name_chain2_height_49")) + require.FileExists(t, filepath.Join(home, "upgrade_name_chain2_height_49")) // start chain2 stdout.Reset() stderr.Reset() args = []string{"run", "--fast", upgradeFilename} doUpgrade, err = launcher.Run(args, stdout, stderr) - require.NoError(err) + require.NoError(t, err) - require.Equal("", stderr.String()) - require.Equal("Chain 2 from zipped binary\nArgs: run --fast "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain3" NEEDED at height: 936: ref_to_chain3-zip_dir.json module=main`+"\n", stdout.String()) + require.Empty(t, stderr.String()) + require.Equal(t, "Chain 2 from zipped binary\nArgs: run --fast "+upgradeFilename+"\n"+`ERROR: UPGRADE "chain3" NEEDED at height: 936: ref_to_chain3-zip_dir.json module=main`+"\n", stdout.String()) // ended with one more upgrade - require.True(doUpgrade) + require.True(t, doUpgrade) currentBin, err = cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.UpgradeBin("chain3"), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.UpgradeBin("chain3"), currentBin) // should have preupgrade.sh results - require.FileExists(filepath.Join(home, "upgrade_name_chain3_height_936")) + require.FileExists(t, filepath.Join(home, "upgrade_name_chain3_height_936")) // run the last chain args = []string{"end", "--halt", upgradeFilename} stdout.Reset() stderr.Reset() doUpgrade, err = launcher.Run(args, stdout, stderr) - require.NoError(err) - require.False(doUpgrade) - require.Equal("", stderr.String()) - require.Equal("Chain 3 from zipped directory\nArgs: end --halt "+upgradeFilename+"\n", stdout.String()) + require.NoError(t, err) + require.False(t, doUpgrade) + require.Empty(t, stderr.String()) + require.Equal(t, "Chain 3 from zipped directory\nArgs: end --halt "+upgradeFilename+"\n", stdout.String()) // and this doesn't upgrade currentBin, err = cfg.CurrentBin() - require.NoError(err) - require.Equal(cfg.UpgradeBin("chain3"), currentBin) + require.NoError(t, err) + require.Equal(t, cfg.UpgradeBin("chain3"), currentBin) } // TestSkipUpgrade tests heights that are identified to be skipped and return if upgrade height matches the skip heights diff --git a/tools/cosmovisor/scanner.go b/tools/cosmovisor/scanner.go index 55f9df6e078e..a5bfdf2cc05c 100644 --- a/tools/cosmovisor/scanner.go +++ b/tools/cosmovisor/scanner.go @@ -9,15 +9,16 @@ import ( "path/filepath" "strconv" "strings" + "testing" "time" - "cosmossdk.io/log" upgradetypes "cosmossdk.io/x/upgrade/types" ) type fileWatcher struct { - filename string // full path to a watched file - interval time.Duration + deamonHome string + filename string // full path to a watched file + interval time.Duration currentBin string currentInfo upgradetypes.Plan @@ -30,7 +31,7 @@ type fileWatcher struct { disableRecase bool } -func newUpgradeFileWatcher(cfg *Config, logger log.Logger) (*fileWatcher, error) { +func newUpgradeFileWatcher(cfg *Config) (*fileWatcher, error) { filename := cfg.UpgradeInfoFilePath() if filename == "" { return nil, errors.New("filename undefined") @@ -52,6 +53,7 @@ func newUpgradeFileWatcher(cfg *Config, logger log.Logger) (*fileWatcher, error) } return &fileWatcher{ + deamonHome: cfg.Home, currentBin: bin, filename: filenameAbs, interval: cfg.PollInterval, @@ -111,10 +113,18 @@ func (fw *fileWatcher) CheckUpdate(currentUpgrade upgradetypes.Plan) bool { return false } + // no update if the file already exists and has not been modified if !stat.ModTime().After(fw.lastModTime) { return false } + // if fw.lastModTime.IsZero() { // check https://github.com/cosmos/cosmos-sdk/issues/21086 + // // first initialization or daemon restart while upgrading-info.json exists. + // // it could be that it was just created and not fully written to disk. + // // wait tiniest bit of time to allow the file to be fully written. + // time.Sleep(2 * time.Millisecond) + // } + info, err := parseUpgradeInfoFile(fw.filename, fw.disableRecase) if err != nil { panic(fmt.Errorf("failed to parse upgrade info file: %w", err)) @@ -153,14 +163,11 @@ func (fw *fileWatcher) CheckUpdate(currentUpgrade upgradetypes.Plan) bool { // checkHeight checks if the current block height func (fw *fileWatcher) checkHeight() (int64, error) { - // TODO(@julienrbrt) use `if !testing.Testing()` from Go 1.22 - // The tests from `process_test.go`, which run only on linux, are failing when using `autod` that is a bash script. - // In production, the binary will always be an application with a status command, but in tests it isn't not. - if strings.HasSuffix(os.Args[0], ".test") { + if testing.Testing() { // we cannot test the command in the test environment return 0, nil } - result, err := exec.Command(fw.currentBin, "status").Output() //nolint:gosec // we want to execute the status command + result, err := exec.Command(fw.currentBin, "status", "--home", fw.deamonHome).CombinedOutput() //nolint:gosec // we want to execute the status command if err != nil { return 0, err } @@ -168,6 +175,9 @@ func (fw *fileWatcher) checkHeight() (int64, error) { type response struct { SyncInfo struct { LatestBlockHeight string `json:"latest_block_height"` + } `json:"sync_info"` + AnotherCasingSyncInfo struct { + LatestBlockHeight string `json:"latest_block_height"` } `json:"SyncInfo"` } @@ -176,11 +186,13 @@ func (fw *fileWatcher) checkHeight() (int64, error) { return 0, err } - if resp.SyncInfo.LatestBlockHeight == "" { - return 0, errors.New("latest block height is empty") + if resp.SyncInfo.LatestBlockHeight != "" { + return strconv.ParseInt(resp.SyncInfo.LatestBlockHeight, 10, 64) + } else if resp.AnotherCasingSyncInfo.LatestBlockHeight != "" { + return strconv.ParseInt(resp.AnotherCasingSyncInfo.LatestBlockHeight, 10, 64) } - return strconv.ParseInt(resp.SyncInfo.LatestBlockHeight, 10, 64) + return 0, errors.New("latest block height is empty") } func parseUpgradeInfoFile(filename string, disableRecase bool) (upgradetypes.Plan, error) {