From 71b692ad6bbe4f2029956e7437fed2a4c0938b7e Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Wed, 10 Apr 2024 11:31:21 +0530 Subject: [PATCH 1/9] feat: load cosmovisor configuration from toml file --- tools/cosmovisor/args.go | 119 ++++++++++++++---- tools/cosmovisor/args_test.go | 8 +- .../cosmovisor/cmd/cosmovisor/add_upgrade.go | 8 +- tools/cosmovisor/cmd/cosmovisor/init.go | 41 ++++-- tools/cosmovisor/cmd/cosmovisor/init_test.go | 85 +++++++++++-- tools/cosmovisor/cmd/cosmovisor/root.go | 8 +- tools/cosmovisor/cmd/cosmovisor/run.go | 16 ++- .../cosmovisor/cmd/cosmovisor/version_test.go | 3 +- tools/cosmovisor/flags.go | 2 + tools/cosmovisor/process.go | 8 +- tools/cosmovisor/process_test.go | 4 +- 11 files changed, 246 insertions(+), 56 deletions(-) diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 4501e8e7d880..5ea3fd27f2d4 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -15,6 +15,8 @@ import ( "cosmossdk.io/log" "cosmossdk.io/x/upgrade/plan" upgradetypes "cosmossdk.io/x/upgrade/types" + "github.com/pelletier/go-toml/v2" + "github.com/spf13/viper" ) // environment variable names @@ -42,26 +44,33 @@ const ( genesisDir = "genesis" upgradesDir = "upgrades" currentLink = "current" + + cfgFileName = "config" + cfgExtension = "toml" +) + +var ( + ErrEmptyConfigENV = errors.New("config env variable not set or empty") ) // Config is the information passed in to control the daemon type Config struct { - Home string - Name string - AllowDownloadBinaries bool - DownloadMustHaveChecksum bool - RestartAfterUpgrade bool - RestartDelay time.Duration - ShutdownGrace time.Duration - PollInterval time.Duration - UnsafeSkipBackup bool - DataBackupPath string - PreupgradeMaxRetries int - DisableLogs bool - ColorLogs bool - TimeFormatLogs string - CustomPreupgrade string - DisableRecase bool + Home string `toml:"DAEMON_HOME" mapstructure:"DAEMON_HOME"` + Name string `toml:"DAEMON_NAME" mapstructure:"DAEMON_NAME"` + AllowDownloadBinaries bool `toml:"DAEMON_ALLOW_DOWNLOAD_BINARIES" mapstructure:"DAEMON_ALLOW_DOWNLOAD_BINARIES" default:"false"` + DownloadMustHaveChecksum bool `toml:"DAEMON_DOWNLOAD_MUST_HAVE_CHECKSUM" mapstructure:"DAEMON_DOWNLOAD_MUST_HAVE_CHECKSUM" default:"false"` + RestartAfterUpgrade bool `toml:"DAEMON_RESTART_AFTER_UPGRADE" mapstructure:"DAEMON_RESTART_AFTER_UPGRADE" default:"true"` + RestartDelay time.Duration `toml:"DAEMON_RESTART_DELAY" mapstructure:"DAEMON_RESTART_DELAY"` + ShutdownGrace time.Duration `toml:"DAEMON_SHUTDOWN_GRACE" mapstructure:"DAEMON_SHUTDOWN_GRACE"` + PollInterval time.Duration `toml:"DAEMON_POLL_INTERVAL" mapstructure:"DAEMON_POLL_INTERVAL" default:"300ms"` + UnsafeSkipBackup bool `toml:"UNSAFE_SKIP_BACKUP" mapstructure:"UNSAFE_SKIP_BACKUP" default:"false"` + DataBackupPath string `toml:"DAEMON_DATA_BACKUP_DIR" mapstructure:"DAEMON_DATA_BACKUP_DIR"` + PreUpgradeMaxRetries int `toml:"DAEMON_PREUPGRADE_MAX_RETRIES" mapstructure:"DAEMON_PREUPGRADE_MAX_RETRIES" default:"0"` + DisableLogs bool `toml:"COSMOVISOR_DISABLE_LOGS" mapstructure:"COSMOVISOR_DISABLE_LOGS" default:"false"` + ColorLogs bool `toml:"COSMOVISOR_COLOR_LOGS" mapstructure:"COSMOVISOR_COLOR_LOGS" default:"true"` + TimeFormatLogs string `toml:"COSMOVISOR_TIMEFORMAT_LOGS" mapstructure:"COSMOVISOR_TIMEFORMAT_LOGS" default:"kitchen"` + CustomPreUpgrade string `toml:"COSMOVISOR_CUSTOM_PREUPGRADE" mapstructure:"COSMOVISOR_CUSTOM_PREUPGRADE" default:""` + DisableRecase bool `toml:"COSMOVISOR_DISABLE_RECASE" mapstructure:"COSMOVISOR_DISABLE_RECASE" default:"false"` // currently running upgrade currentUpgrade upgradetypes.Plan @@ -145,6 +154,42 @@ func (cfg *Config) CurrentBin() (string, error) { return binpath, nil } +// GetConfigFromFile will read the configuration from the file at the given path. +// It will return an error if the file does not exist or if the configuration is invalid. +// If ENV variables are set, they will override the values in the file. +func GetConfigFromFile(filePath string) (*Config, error) { + if filePath == "" { + return nil, ErrEmptyConfigENV + } + + // ensure the file exist + if _, err := os.Stat(filePath); err != nil { + return nil, fmt.Errorf("config file does not exist: at %s : %w", filePath, err) + } + + // read the configuration from the file + viper.SetConfigFile(filePath) + // load the env variables + // if the env variable is set, it will override the value provided by the config + viper.AutomaticEnv() + + if err := viper.ReadInConfig(); err != nil { + return nil, fmt.Errorf("failed to read config file: %w", err) + } + + cfg := &Config{} + if err := viper.Unmarshal(cfg); err != nil { + return nil, fmt.Errorf("failed to unmarshal configuration: %w", err) + } + + errs := cfg.validate() + if len(errs) > 0 { + return nil, errors.Join(errs...) + } + + return cfg, nil +} + // GetConfigFromEnv will read the environmental variables into a config // and then validate it is reasonable func GetConfigFromEnv() (*Config, error) { @@ -153,7 +198,7 @@ func GetConfigFromEnv() (*Config, error) { Home: os.Getenv(EnvHome), Name: os.Getenv(EnvName), DataBackupPath: os.Getenv(EnvDataBackupPath), - CustomPreupgrade: os.Getenv(EnvCustomPreupgrade), + CustomPreUpgrade: os.Getenv(EnvCustomPreupgrade), } if cfg.DataBackupPath == "" { @@ -220,8 +265,8 @@ func GetConfigFromEnv() (*Config, error) { } } - envPreupgradeMaxRetriesVal := os.Getenv(EnvPreupgradeMaxRetries) - if cfg.PreupgradeMaxRetries, err = strconv.Atoi(envPreupgradeMaxRetriesVal); err != nil && envPreupgradeMaxRetriesVal != "" { + envPreUpgradeMaxRetriesVal := os.Getenv(EnvPreupgradeMaxRetries) + if cfg.PreUpgradeMaxRetries, err = strconv.Atoi(envPreUpgradeMaxRetriesVal); err != nil && envPreUpgradeMaxRetriesVal != "" { errs = append(errs, fmt.Errorf("%s could not be parsed to int: %w", EnvPreupgradeMaxRetries, err)) } @@ -395,12 +440,13 @@ func BooleanOption(name string, defaultVal bool) (bool, error) { return false, fmt.Errorf("env variable %q must have a boolean value (\"true\" or \"false\"), got %q", name, p) } -// checks and validates env option +// TimeFormatOptionFromEnv checks and validates the time format option func TimeFormatOptionFromEnv(env, defaultVal string) (string, error) { val, set := os.LookupEnv(env) if !set { return defaultVal, nil } + switch val { case "layout": return time.Layout, nil @@ -445,11 +491,11 @@ func (cfg Config) DetailString() string { {EnvInterval, cfg.PollInterval.String()}, {EnvSkipBackup, fmt.Sprintf("%t", cfg.UnsafeSkipBackup)}, {EnvDataBackupPath, cfg.DataBackupPath}, - {EnvPreupgradeMaxRetries, fmt.Sprintf("%d", cfg.PreupgradeMaxRetries)}, + {EnvPreupgradeMaxRetries, fmt.Sprintf("%d", cfg.PreUpgradeMaxRetries)}, {EnvDisableLogs, fmt.Sprintf("%t", cfg.DisableLogs)}, {EnvColorLogs, fmt.Sprintf("%t", cfg.ColorLogs)}, {EnvTimeFormatLogs, cfg.TimeFormatLogs}, - {EnvCustomPreupgrade, cfg.CustomPreupgrade}, + {EnvCustomPreupgrade, cfg.CustomPreUpgrade}, {EnvDisableRecase, fmt.Sprintf("%t", cfg.DisableRecase)}, } @@ -479,3 +525,32 @@ func (cfg Config) DetailString() string { } return sb.String() } + +// Export exports the configuration to a file at the given path. +func (cfg Config) Export(path string) (string, error) { + // if path is empty, use the default path + if path == "" { + path = filepath.Join(cfg.Root(), filepath.Join(cfgFileName+"."+cfgExtension)) + } + + // ensure the path has proper extension + if !strings.HasSuffix(path, cfgExtension) { + return "", fmt.Errorf("provided config file must have %s extension", cfgExtension) + } + + // create the file + file, err := os.Create(path) + if err != nil { + return "", fmt.Errorf("failed to create configuration file: %w", err) + } + + defer file.Close() + + // write the configuration to the file + err = toml.NewEncoder(file).Encode(cfg) + if err != nil { + return "", fmt.Errorf("failed to encode configuration: %w", err) + } + + return path, nil +} diff --git a/tools/cosmovisor/args_test.go b/tools/cosmovisor/args_test.go index e161d229e7db..018dd36d48a5 100644 --- a/tools/cosmovisor/args_test.go +++ b/tools/cosmovisor/args_test.go @@ -412,7 +412,7 @@ func (s *argsTestSuite) TestDetailString() { PollInterval: pollInterval, UnsafeSkipBackup: unsafeSkipBackup, DataBackupPath: dataBackupPath, - PreupgradeMaxRetries: preupgradeMaxRetries, + PreUpgradeMaxRetries: preupgradeMaxRetries, } expectedPieces := []string{ @@ -477,11 +477,11 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { PollInterval: time.Millisecond * time.Duration(interval), UnsafeSkipBackup: skipBackup, DataBackupPath: dataBackupPath, - PreupgradeMaxRetries: preupgradeMaxRetries, + PreUpgradeMaxRetries: preupgradeMaxRetries, DisableLogs: disableLogs, ColorLogs: colorLogs, TimeFormatLogs: timeFormatLogs, - CustomPreupgrade: customPreUpgrade, + CustomPreUpgrade: customPreUpgrade, DisableRecase: disableRecase, ShutdownGrace: time.Duration(shutdownGrace), } @@ -791,7 +791,7 @@ func BenchmarkDetailString(b *testing.B) { AllowDownloadBinaries: true, UnsafeSkipBackup: true, PollInterval: 450 * time.Second, - PreupgradeMaxRetries: 1e7, + PreUpgradeMaxRetries: 1e7, } b.ReportAllocs() diff --git a/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go b/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go index 24de3cd4899a..c2e4ba5cadf8 100644 --- a/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go +++ b/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go @@ -25,13 +25,19 @@ func NewAddUpgradeCmd() *cobra.Command { addUpgrade.Flags().Bool(cosmovisor.FlagForce, false, "overwrite existing upgrade binary / upgrade-info.json file") addUpgrade.Flags().Int64(cosmovisor.FlagUpgradeHeight, 0, "define a height at which to upgrade the binary automatically (without governance proposal)") + addUpgrade.Flags().String(cosmovisor.FlagConfig, "", "path to cosmovisor config file") return addUpgrade } // AddUpgrade adds upgrade info to manifest func AddUpgrade(cmd *cobra.Command, args []string) error { - cfg, err := cosmovisor.GetConfigFromEnv() + configPath, err := cmd.Flags().GetString(cosmovisor.FlagConfig) + if err != nil { + return fmt.Errorf("failed to get config flag: %w", err) + } + + cfg, err := cosmovisor.GetConfigFromFile(configPath) if err != nil { return err } diff --git a/tools/cosmovisor/cmd/cosmovisor/init.go b/tools/cosmovisor/cmd/cosmovisor/init.go index 9b08529e647e..de00162160ce 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init.go +++ b/tools/cosmovisor/cmd/cosmovisor/init.go @@ -8,6 +8,8 @@ import ( "path/filepath" "time" + "github.com/spf13/viper" + "github.com/spf13/cobra" "cosmossdk.io/log" @@ -15,14 +17,21 @@ import ( "cosmossdk.io/x/upgrade/plan" ) -var initCmd = &cobra.Command{ - Use: "init ", - Short: "Initialize a cosmovisor daemon home directory.", - Args: cobra.ExactArgs(1), - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - return InitializeCosmovisor(nil, args) - }, +func NewIntCmd() *cobra.Command { + var initCmd = &cobra.Command{ + Use: "init ", + Short: "Initialize a cosmovisor daemon home directory.", + Args: cobra.ExactArgs(1), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + mustNoError(viper.BindPFlags(cmd.Flags())) + return InitializeCosmovisor(nil, args) + }, + } + + initCmd.Flags().String(cosmovisor.FlagExportConfig, "", "path to export the configuration file to (default is <-home_path->/cosmovisor/config.toml") + + return initCmd } // InitializeCosmovisor initializes the cosmovisor directories, current link, and initial executable. @@ -88,12 +97,20 @@ func InitializeCosmovisor(logger log.Logger, args []string) error { } logger.Info(fmt.Sprintf("the current symlink points to: %q", cur)) + filePath, err := cfg.Export(viper.GetString(cosmovisor.FlagExportConfig)) + if err != nil { + return fmt.Errorf("failed to export configuration: %w", err) + } + + logger.Info(fmt.Sprintf("exported configuration to: %s", filePath)) + return nil } // getConfigForInitCmd gets just the configuration elements needed to initialize cosmovisor. func getConfigForInitCmd() (*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{ @@ -105,19 +122,27 @@ func getConfigForInitCmd() (*cosmovisor.Config, 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)) } + switch { case len(cfg.Home) == 0: errs = append(errs, fmt.Errorf("%s is not set", cosmovisor.EnvHome)) case !filepath.IsAbs(cfg.Home): errs = append(errs, fmt.Errorf("%s must be an absolute path", cosmovisor.EnvHome)) } + if len(errs) > 0 { return cfg, errors.Join(errs...) } diff --git a/tools/cosmovisor/cmd/cosmovisor/init_test.go b/tools/cosmovisor/cmd/cosmovisor/init_test.go index c872675e280d..15cc3a927374 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/init_test.go @@ -9,6 +9,8 @@ import ( "testing" "time" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -18,7 +20,10 @@ import ( ) const ( - notset = " is not set" + notset = " is not set" + cosmovisorDirName = "cosmovisor" + + cfgFileWithExt = "config.toml" ) type InitTestSuite struct { @@ -79,6 +84,7 @@ func (s *InitTestSuite) clearEnv() *cosmovisorInitEnv { for envVar := range rv.ToMap() { rv.Set(envVar, os.Getenv(envVar)) s.Require().NoError(os.Unsetenv(envVar)) + viper.Reset() } return &rv } @@ -247,7 +253,7 @@ func (p *BufferedPipe) panicIfStarted(msg string) { func (s *InitTestSuite) NewCapturingLogger() (*BufferedPipe, log.Logger) { bufferedStdOut, err := StartNewBufferedPipe("stdout", os.Stdout) s.Require().NoError(err, "creating stdout buffered pipe") - logger := log.NewLogger(bufferedStdOut, log.ColorOption(false), log.TimeFormatOption(time.RFC3339Nano)).With(log.ModuleKey, "cosmovisor") + logger := log.NewLogger(bufferedStdOut, log.ColorOption(false), log.TimeFormatOption(time.RFC3339Nano)).With(log.ModuleKey, cosmovisorDirName) return &bufferedStdOut, logger } @@ -360,7 +366,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorInvalidExisting() { Home: filepath.Join(testDir, "home"), Name: "pear", } - genDir := filepath.Join(env.Home, "cosmovisor", "genesis") + genDir := filepath.Join(env.Home, cosmovisorDirName, "genesis") genBin := filepath.Join(genDir, "bin") require.NoError(t, os.MkdirAll(genDir, 0o755), "creating genesis directory") require.NoError(t, copyFile(hwExe, genBin), "copying exe to genesis/bin") @@ -380,7 +386,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorInvalidExisting() { } // Create the genesis bin executable path fully as a directory (instead of a file). // That should get through all the other stuff, but error when EnsureBinary is called. - genBinExe := filepath.Join(env.Home, "cosmovisor", "genesis", "bin", env.Name) + genBinExe := filepath.Join(env.Home, cosmovisorDirName, "genesis", "bin", env.Name) require.NoError(t, os.MkdirAll(genBinExe, 0o755)) expErr := fmt.Sprintf("%s is not a regular file", env.Name) // Check the log messages just to make sure it's erroring where expecting. @@ -416,7 +422,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorInvalidExisting() { Home: filepath.Join(testDir, "home"), Name: "orange", } - rootDir := filepath.Join(env.Home, "cosmovisor") + rootDir := filepath.Join(env.Home, cosmovisorDirName) require.NoError(t, os.MkdirAll(rootDir, 0o755)) curLn := filepath.Join(rootDir, "current") genDir := filepath.Join(rootDir, "genesis") @@ -433,6 +439,22 @@ func (s *InitTestSuite) TestInitializeCosmovisorInvalidExisting() { assert.Contains(t, bufferStr, "checking on the current symlink and creating it if needed") }) + s.T().Run("flag export config file invalid extension", func(t *testing.T) { + testDir := t.TempDir() + env := &cosmovisorInitEnv{ + Home: filepath.Join(testDir, "home"), + Name: "pineapple", + } + + s.setEnv(t, env) + // setting an invalid extension for config file + viper.Set(cosmovisor.FlagExportConfig, filepath.Join(env.Home, cosmovisorDirName, "config.json")) + _, logger := s.NewCapturingLogger() + logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) + err := InitializeCosmovisor(logger, []string{hwExe}) + require.ErrorContains(t, err, "config file must have toml extension") + }) + // Failure cases not tested: // Cannot create genesis bin directory // I had a test for this that created the `genesis` directory with permissions 0o555. @@ -465,8 +487,8 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { Home: filepath.Join(testDir, "home"), Name: "blank", } - curLn := filepath.Join(env.Home, "cosmovisor", "current") - genBinDir := filepath.Join(env.Home, "cosmovisor", "genesis", "bin") + curLn := filepath.Join(env.Home, cosmovisorDirName, "current") + genBinDir := filepath.Join(env.Home, cosmovisorDirName, "genesis", "bin") genBinExe := filepath.Join(genBinDir, env.Name) expInLog := []string{ "checking on the genesis/bin directory", @@ -476,6 +498,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("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(s.T(), env) @@ -508,7 +531,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { Home: filepath.Join(testDir, "home"), Name: "nocur", } - rootDir := filepath.Join(env.Home, "cosmovisor") + rootDir := filepath.Join(env.Home, cosmovisorDirName) genBinDir := filepath.Join(rootDir, "genesis", "bin") genBinDirExe := filepath.Join(genBinDir, env.Name) require.NoError(t, os.MkdirAll(genBinDir, 0o755), "making genesis bin dir") @@ -528,6 +551,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("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(t, env) @@ -548,7 +572,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { Home: filepath.Join(testDir, "home"), Name: "emptygen", } - rootDir := filepath.Join(env.Home, "cosmovisor") + rootDir := filepath.Join(env.Home, cosmovisorDirName) genBinDir := filepath.Join(rootDir, "genesis", "bin") genBinExe := filepath.Join(genBinDir, env.Name) require.NoError(t, os.MkdirAll(genBinDir, 0o755), "making genesis bin dir") @@ -560,6 +584,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("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(t, env) @@ -573,4 +598,46 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { assert.Contains(t, bufferStr, exp) } }) + + s.T().Run("flag export config use provide path", func(t *testing.T) { + testDir := s.T().TempDir() + env := &cosmovisorInitEnv{ + Home: filepath.Join(testDir, "home"), + Name: "emptygen", + } + + s.setEnv(t, env) + buffer, logger := s.NewCapturingLogger() + + expectedPath := filepath.Join(env.Home, cfgFileWithExt) + // setting a path to export flag + viper.Set(cosmovisor.FlagExportConfig, expectedPath) + expectedInLog := fmt.Sprintf("exported configuration to: %s", expectedPath) + + logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) + err := InitializeCosmovisor(logger, []string{hwExe}) + require.NoError(t, err, "calling InitializeCosmovisor") + bufferBz := buffer.Collect() + bufferStr := string(bufferBz) + assert.Contains(t, bufferStr, expectedInLog) + }) + + s.T().Run("flag export config use default path if not provided", func(t *testing.T) { + testDir := s.T().TempDir() + env := &cosmovisorInitEnv{ + Home: filepath.Join(testDir, "home"), + Name: "emptygen", + } + + s.setEnv(t, env) + buffer, logger := s.NewCapturingLogger() + // setting empty path to export flag + viper.Set(cosmovisor.FlagExportConfig, "") + logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) + err := InitializeCosmovisor(logger, []string{hwExe}) + require.NoError(t, err, "calling InitializeCosmovisor") + bufferBz := buffer.Collect() + bufferStr := string(bufferBz) + assert.Contains(t, bufferStr, fmt.Sprintf("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt))) + }) } diff --git a/tools/cosmovisor/cmd/cosmovisor/root.go b/tools/cosmovisor/cmd/cosmovisor/root.go index 4d87d9867c2d..41c283144365 100644 --- a/tools/cosmovisor/cmd/cosmovisor/root.go +++ b/tools/cosmovisor/cmd/cosmovisor/root.go @@ -12,7 +12,7 @@ func NewRootCmd() *cobra.Command { } rootCmd.AddCommand( - initCmd, + NewIntCmd(), runCmd, configCmd, NewVersionCmd(), @@ -21,3 +21,9 @@ func NewRootCmd() *cobra.Command { return rootCmd } + +func mustNoError(err error) { + if err != nil { + panic(err) + } +} diff --git a/tools/cosmovisor/cmd/cosmovisor/run.go b/tools/cosmovisor/cmd/cosmovisor/run.go index 24aa9b6f8991..98fc0c26cbcb 100644 --- a/tools/cosmovisor/cmd/cosmovisor/run.go +++ b/tools/cosmovisor/cmd/cosmovisor/run.go @@ -1,14 +1,22 @@ package main import ( - "github.com/spf13/cobra" + "os" "cosmossdk.io/tools/cosmovisor" + "github.com/spf13/cobra" +) + +const ( + cfgENV = "CONFIG" ) var runCmd = &cobra.Command{ - Use: "run", - Short: "Run an APP command.", + Use: "run", + Short: "Run an APP command.", + Long: `Run an APP command. This command is intended to be used by the cosmovisor process manager. +It will run the configured program and monitor it for upgrades. +Initialise the cosmovisor configuration with the 'init' command.`, SilenceUsage: true, DisableFlagParsing: true, RunE: func(_ *cobra.Command, args []string) error { @@ -18,7 +26,7 @@ var runCmd = &cobra.Command{ // run runs the configured program with the given args and monitors it for upgrades. func run(args []string, options ...RunOption) error { - cfg, err := cosmovisor.GetConfigFromEnv() + cfg, err := cosmovisor.GetConfigFromFile(os.Getenv(cfgENV)) if err != nil { return err } diff --git a/tools/cosmovisor/cmd/cosmovisor/version_test.go b/tools/cosmovisor/cmd/cosmovisor/version_test.go index 8f51ea47dafb..18eff057fc11 100644 --- a/tools/cosmovisor/cmd/cosmovisor/version_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/version_test.go @@ -5,6 +5,7 @@ import ( "context" "testing" + "cosmossdk.io/tools/cosmovisor" "github.com/stretchr/testify/require" "cosmossdk.io/log" @@ -23,5 +24,5 @@ func TestVersionCommand_Error(t *testing.T) { ctx := context.WithValue(context.Background(), log.ContextKey, logger) require.Error(t, rootCmd.ExecuteContext(ctx)) - require.Contains(t, out.String(), "DAEMON_NAME is not set") + require.Contains(t, out.String(), cosmovisor.ErrEmptyConfigENV.Error()) } diff --git a/tools/cosmovisor/flags.go b/tools/cosmovisor/flags.go index 73c17b9247dd..04a970eaa5f0 100644 --- a/tools/cosmovisor/flags.go +++ b/tools/cosmovisor/flags.go @@ -6,4 +6,6 @@ const ( FlagCosmovisorOnly = "cosmovisor-only" FlagForce = "force" FlagUpgradeHeight = "upgrade-height" + FlagConfig = "config" + FlagExportConfig = "export-config" ) diff --git a/tools/cosmovisor/process.go b/tools/cosmovisor/process.go index 3de7a942faec..923995a51cd4 100644 --- a/tools/cosmovisor/process.go +++ b/tools/cosmovisor/process.go @@ -200,7 +200,7 @@ func (l Launcher) doBackup() error { // doCustomPreUpgrade executes the custom preupgrade script if provided. func (l Launcher) doCustomPreUpgrade() error { - if l.cfg.CustomPreupgrade == "" { + if l.cfg.CustomPreUpgrade == "" { return nil } @@ -220,7 +220,7 @@ func (l Launcher) doCustomPreUpgrade() error { } // check if preupgradeFile is executable file - preupgradeFile := filepath.Join(l.cfg.Home, "cosmovisor", l.cfg.CustomPreupgrade) + preupgradeFile := filepath.Join(l.cfg.Home, "cosmovisor", l.cfg.CustomPreUpgrade) l.logger.Info("looking for COSMOVISOR_CUSTOM_PREUPGRADE file", "file", preupgradeFile) info, err := os.Stat(preupgradeFile) if err != nil { @@ -263,8 +263,8 @@ func (l Launcher) doCustomPreUpgrade() error { func (l *Launcher) doPreUpgrade() error { counter := 0 for { - if counter > l.cfg.PreupgradeMaxRetries { - return fmt.Errorf("pre-upgrade command failed. reached max attempt of retries - %d", l.cfg.PreupgradeMaxRetries) + if counter > l.cfg.PreUpgradeMaxRetries { + return fmt.Errorf("pre-upgrade command failed. reached max attempt of retries - %d", l.cfg.PreUpgradeMaxRetries) } if err := l.executePreUpgradeCmd(); err != nil { diff --git a/tools/cosmovisor/process_test.go b/tools/cosmovisor/process_test.go index 2ecea4cb780f..1cd862af0166 100644 --- a/tools/cosmovisor/process_test.go +++ b/tools/cosmovisor/process_test.go @@ -272,7 +272,7 @@ func (s *processTestSuite) TestLaunchProcessWithDownloadsAndMissingPreupgrade() AllowDownloadBinaries: true, PollInterval: 100, UnsafeSkipBackup: true, - CustomPreupgrade: "missing.sh", + CustomPreUpgrade: "missing.sh", } logger := log.NewTestLogger(s.T()).With(log.ModuleKey, "cosmovisor") upgradeFilename := cfg.UpgradeInfoFilePath() @@ -308,7 +308,7 @@ func (s *processTestSuite) TestLaunchProcessWithDownloadsAndPreupgrade() { AllowDownloadBinaries: true, PollInterval: 100, UnsafeSkipBackup: true, - CustomPreupgrade: "preupgrade.sh", + CustomPreUpgrade: "preupgrade.sh", } buf := newBuffer() // inspect output using buf.String() logger := log.NewLogger(buf).With(log.ModuleKey, "cosmovisor") From af650257f4f274a4cd0faf69b5c3b0cebda9c7d4 Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Wed, 10 Apr 2024 11:52:33 +0530 Subject: [PATCH 2/9] fix: linter issues --- tools/cosmovisor/args.go | 16 ++++++++++------ tools/cosmovisor/cmd/cosmovisor/init.go | 5 ++--- tools/cosmovisor/cmd/cosmovisor/init_test.go | 1 - tools/cosmovisor/cmd/cosmovisor/run.go | 5 +++-- tools/cosmovisor/cmd/cosmovisor/version_test.go | 2 +- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 5ea3fd27f2d4..20c57475ff4d 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -12,11 +12,12 @@ import ( "strings" "time" + "github.com/pelletier/go-toml/v2" + "github.com/spf13/viper" + "cosmossdk.io/log" "cosmossdk.io/x/upgrade/plan" upgradetypes "cosmossdk.io/x/upgrade/types" - "github.com/pelletier/go-toml/v2" - "github.com/spf13/viper" ) // environment variable names @@ -49,9 +50,7 @@ const ( cfgExtension = "toml" ) -var ( - ErrEmptyConfigENV = errors.New("config env variable not set or empty") -) +var ErrEmptyConfigENV = errors.New("config env variable not set or empty") // Config is the information passed in to control the daemon type Config struct { @@ -81,6 +80,11 @@ func (cfg *Config) Root() string { return filepath.Join(cfg.Home, rootName) } +// DefaultCfgPath returns the default path to the configuration file. +func (cfg *Config) DefaultCfgPath() string { + return filepath.Join(cfg.Root(), cfgFileName+"."+cfgExtension) +} + // GenesisBin is the path to the genesis binary - must be in place to start manager func (cfg *Config) GenesisBin() string { return filepath.Join(cfg.Root(), genesisDir, "bin", cfg.Name) @@ -530,7 +534,7 @@ func (cfg Config) DetailString() string { func (cfg Config) Export(path string) (string, error) { // if path is empty, use the default path if path == "" { - path = filepath.Join(cfg.Root(), filepath.Join(cfgFileName+"."+cfgExtension)) + path = cfg.DefaultCfgPath() } // ensure the path has proper extension diff --git a/tools/cosmovisor/cmd/cosmovisor/init.go b/tools/cosmovisor/cmd/cosmovisor/init.go index de00162160ce..5f35e309b7ac 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init.go +++ b/tools/cosmovisor/cmd/cosmovisor/init.go @@ -8,9 +8,8 @@ import ( "path/filepath" "time" - "github.com/spf13/viper" - "github.com/spf13/cobra" + "github.com/spf13/viper" "cosmossdk.io/log" "cosmossdk.io/tools/cosmovisor" @@ -18,7 +17,7 @@ import ( ) func NewIntCmd() *cobra.Command { - var initCmd = &cobra.Command{ + initCmd := &cobra.Command{ Use: "init ", Short: "Initialize a cosmovisor daemon home directory.", Args: cobra.ExactArgs(1), diff --git a/tools/cosmovisor/cmd/cosmovisor/init_test.go b/tools/cosmovisor/cmd/cosmovisor/init_test.go index 15cc3a927374..502fcc24ea1f 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/init_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/spf13/viper" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" diff --git a/tools/cosmovisor/cmd/cosmovisor/run.go b/tools/cosmovisor/cmd/cosmovisor/run.go index 98fc0c26cbcb..1c46cb707281 100644 --- a/tools/cosmovisor/cmd/cosmovisor/run.go +++ b/tools/cosmovisor/cmd/cosmovisor/run.go @@ -3,8 +3,9 @@ package main import ( "os" - "cosmossdk.io/tools/cosmovisor" "github.com/spf13/cobra" + + "cosmossdk.io/tools/cosmovisor" ) const ( @@ -16,7 +17,7 @@ var runCmd = &cobra.Command{ Short: "Run an APP command.", Long: `Run an APP command. This command is intended to be used by the cosmovisor process manager. It will run the configured program and monitor it for upgrades. -Initialise the cosmovisor configuration with the 'init' command.`, +Initialize the cosmovisor configuration with the 'init' command.`, SilenceUsage: true, DisableFlagParsing: true, RunE: func(_ *cobra.Command, args []string) error { diff --git a/tools/cosmovisor/cmd/cosmovisor/version_test.go b/tools/cosmovisor/cmd/cosmovisor/version_test.go index 18eff057fc11..f71f8cdd283f 100644 --- a/tools/cosmovisor/cmd/cosmovisor/version_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/version_test.go @@ -5,10 +5,10 @@ import ( "context" "testing" - "cosmossdk.io/tools/cosmovisor" "github.com/stretchr/testify/require" "cosmossdk.io/log" + "cosmossdk.io/tools/cosmovisor" ) func TestVersionCommand_Error(t *testing.T) { From 3351c7bccfa9720d008c4d4ffb5bf23db4105f89 Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Thu, 11 Apr 2024 16:21:54 +0530 Subject: [PATCH 3/9] test: add test cases --- tools/cosmovisor/args.go | 65 ++++++++-- tools/cosmovisor/args_test.go | 225 ++++++++++++++++++++++++++++------ 2 files changed, 248 insertions(+), 42 deletions(-) diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 20c57475ff4d..ea8df69ef075 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -20,6 +20,8 @@ import ( upgradetypes "cosmossdk.io/x/upgrade/types" ) +var ErrEmptyConfigENV = errors.New("config env variable not set or empty") + // environment variable names const ( EnvHome = "DAEMON_HOME" @@ -50,8 +52,6 @@ const ( cfgExtension = "toml" ) -var ErrEmptyConfigENV = errors.New("config env variable not set or empty") - // Config is the information passed in to control the daemon type Config struct { Home string `toml:"DAEMON_HOME" mapstructure:"DAEMON_HOME"` @@ -168,7 +168,7 @@ func GetConfigFromFile(filePath string) (*Config, error) { // ensure the file exist if _, err := os.Stat(filePath); err != nil { - return nil, fmt.Errorf("config file does not exist: at %s : %w", filePath, err) + return nil, fmt.Errorf("config not found: at %s : %w", filePath, err) } // read the configuration from the file @@ -186,7 +186,16 @@ func GetConfigFromFile(filePath string) (*Config, error) { return nil, fmt.Errorf("failed to unmarshal configuration: %w", err) } - errs := cfg.validate() + var ( + err error + errs []error + ) + + if cfg.TimeFormatLogs, err = getTimeFormatOption(cfg.TimeFormatLogs); err != nil { + errs = append(errs, err) + } + + errs = append(errs, cfg.validate()...) if len(errs) > 0 { return nil, errors.Join(errs...) } @@ -404,6 +413,7 @@ func (cfg *Config) SetCurrentUpgrade(u upgradetypes.Plan) (rerr error) { return err } +// UpgradeInfo returns the current upgrade info func (cfg *Config) UpgradeInfo() (upgradetypes.Plan, error) { if cfg.currentUpgrade.Name != "" { return cfg.currentUpgrade, nil @@ -430,7 +440,7 @@ returnError: return cfg.currentUpgrade, fmt.Errorf("failed to read %q: %w", filename, err) } -// checks and validates env option +// BooleanOption checks and validate env option func BooleanOption(name string, defaultVal bool) (bool, error) { p := strings.ToLower(os.Getenv(name)) switch p { @@ -451,6 +461,10 @@ func TimeFormatOptionFromEnv(env, defaultVal string) (string, error) { return defaultVal, nil } + return getTimeFormatOption(val) +} + +func getTimeFormatOption(val string) (string, error) { switch val { case "layout": return time.Layout, nil @@ -480,6 +494,40 @@ func TimeFormatOptionFromEnv(env, defaultVal string) (string, error) { return "", nil } return "", fmt.Errorf("env variable %q must have a timeformat value (\"layout|ansic|unixdate|rubydate|rfc822|rfc822z|rfc850|rfc1123|rfc1123z|rfc3339|rfc3339nano|kitchen\"), got %q", EnvTimeFormatLogs, val) + +} + +// ValueToTimeFormatOption converts the time format option to the env value +func ValueToTimeFormatOption(format string) string { + switch format { + case time.Layout: + return "layout" + case time.ANSIC: + return "ansic" + case time.UnixDate: + return "unixdate" + case time.RubyDate: + return "rubydate" + case time.RFC822: + return "rfc822" + case time.RFC822Z: + return "rfc822z" + case time.RFC850: + return "rfc850" + case time.RFC1123: + return "rfc1123" + case time.RFC1123Z: + return "rfc1123z" + case time.RFC3339: + return "rfc3339" + case time.RFC3339Nano: + return "rfc3339nano" + case time.Kitchen: + return "kitchen" + default: + return "" + } + } // DetailString returns a multi-line string with details about this config. @@ -539,15 +587,18 @@ func (cfg Config) Export(path string) (string, error) { // ensure the path has proper extension if !strings.HasSuffix(path, cfgExtension) { - return "", fmt.Errorf("provided config file must have %s extension", cfgExtension) + return "", fmt.Errorf("invalid file extension must have %s extension", cfgExtension) } // create the file - file, err := os.Create(path) + file, err := os.Create(filepath.Clean(path)) if err != nil { return "", fmt.Errorf("failed to create configuration file: %w", err) } + // convert the time value to its format option + cfg.TimeFormatLogs = ValueToTimeFormatOption(cfg.TimeFormatLogs) + defer file.Close() // write the configuration to the file diff --git a/tools/cosmovisor/args_test.go b/tools/cosmovisor/args_test.go index 018dd36d48a5..f020bfa8ae1c 100644 --- a/tools/cosmovisor/args_test.go +++ b/tools/cosmovisor/args_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/pelletier/go-toml/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -444,6 +445,41 @@ func (s *argsTestSuite) TestDetailString() { } } +var newConfig = func( + home, name string, + downloadBin bool, + downloadMustHaveChecksum bool, + restartUpgrade bool, + restartDelay int, + skipBackup bool, + dataBackupPath string, + interval, preupgradeMaxRetries int, + disableLogs, colorLogs bool, + timeFormatLogs string, + customPreUpgrade string, + disableRecase bool, + shutdownGrace int, +) *Config { + return &Config{ + Home: home, + Name: name, + AllowDownloadBinaries: downloadBin, + DownloadMustHaveChecksum: downloadMustHaveChecksum, + RestartAfterUpgrade: restartUpgrade, + RestartDelay: time.Millisecond * time.Duration(restartDelay), + PollInterval: time.Millisecond * time.Duration(interval), + UnsafeSkipBackup: skipBackup, + DataBackupPath: dataBackupPath, + PreUpgradeMaxRetries: preupgradeMaxRetries, + DisableLogs: disableLogs, + ColorLogs: colorLogs, + TimeFormatLogs: timeFormatLogs, + CustomPreUpgrade: customPreUpgrade, + DisableRecase: disableRecase, + ShutdownGrace: time.Duration(shutdownGrace), + } +} + func (s *argsTestSuite) TestGetConfigFromEnv() { initialEnv := s.clearEnv() defer s.setEnv(nil, initialEnv) @@ -452,41 +488,6 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { absPath, perr := filepath.Abs(relPath) s.Require().NoError(perr) - newConfig := func( - home, name string, - downloadBin bool, - downloadMustHaveChecksum bool, - restartUpgrade bool, - restartDelay int, - skipBackup bool, - dataBackupPath string, - interval, preupgradeMaxRetries int, - disableLogs, colorLogs bool, - timeFormatLogs string, - customPreUpgrade string, - disableRecase bool, - shutdownGrace int, - ) *Config { - return &Config{ - Home: home, - Name: name, - AllowDownloadBinaries: downloadBin, - DownloadMustHaveChecksum: downloadMustHaveChecksum, - RestartAfterUpgrade: restartUpgrade, - RestartDelay: time.Millisecond * time.Duration(restartDelay), - PollInterval: time.Millisecond * time.Duration(interval), - UnsafeSkipBackup: skipBackup, - DataBackupPath: dataBackupPath, - PreUpgradeMaxRetries: preupgradeMaxRetries, - DisableLogs: disableLogs, - ColorLogs: colorLogs, - TimeFormatLogs: timeFormatLogs, - CustomPreUpgrade: customPreUpgrade, - DisableRecase: disableRecase, - ShutdownGrace: time.Duration(shutdownGrace), - } - } - tests := []struct { name string envVals cosmovisorEnv @@ -783,6 +784,160 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { } } +func setUpDir(t *testing.T) string { + t.Helper() + + home := t.TempDir() + err := os.MkdirAll(filepath.Join(home, rootName), 0o755) + require.NoError(t, err) + return home +} + +func setupConfig(t *testing.T, home string) string { + cfg := newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, "kitchen", "", true, 10) + path := filepath.Join(home, rootName, "config.toml") + f, err := os.Create(path) + require.NoError(t, err) + + enc := toml.NewEncoder(f) + require.NoError(t, enc.Encode(&cfg)) + + err = f.Close() + require.NoError(t, err) + + return path +} + +func TestConfigFromFile(t *testing.T) { + home := setUpDir(t) + // create a config file + cfgFilePath := setupConfig(t, home) + + testCases := []struct { + name string + config *Config + expectedCfg func() *Config + filePath string + expectedError string + malleate func() + }{ + { + name: "valid config", + expectedCfg: func() *Config { + return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) + }, + filePath: cfgFilePath, + expectedError: "", + malleate: func() {}, + }, + + { + name: "empty config file path", + expectedCfg: func() *Config { + return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) + }, + filePath: "", + expectedError: ErrEmptyConfigENV.Error(), + malleate: func() {}, + }, + { + name: "invalid config file path", + expectedCfg: func() *Config { + return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) + }, + filePath: filepath.Join(home, "invalid.toml"), + expectedError: "config not found", + malleate: func() {}, + }, + { + name: "env variable will override config file fields", + filePath: cfgFilePath, + expectedError: "", + malleate: func() { + // set env variable different from the config file + os.Setenv(EnvName, "env-name") + }, + expectedCfg: func() *Config { + return newConfig(home, "env-name", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) + + }, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + tc.malleate() + actualCfg, err := GetConfigFromFile(tc.filePath) + if tc.expectedError != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedError) + return + } + + require.NoError(t, err) + require.EqualValues(t, tc.expectedCfg(), actualCfg) + + }) + } +} + +func TestConfigExport(t *testing.T) { + home := setUpDir(t) + // create a config file + cfgFilePath := setupConfig(t, home) + // load file + expectedCfg, err := GetConfigFromFile(cfgFilePath) + require.NoError(t, err) + + testCases := []struct { + name string + filePath string + expectedPath string + expectedError string + }{ + { + name: "valid config", + filePath: cfgFilePath, + expectedPath: filepath.Join(expectedCfg.Root(), "config.toml"), + expectedError: "", + }, + { + name: "empty config file path will use default path", + filePath: "", + expectedPath: expectedCfg.DefaultCfgPath(), + expectedError: "", + }, + { + name: "invalid file extension", + filePath: filepath.Join(home, rootName, "config.json"), + expectedPath: "", + expectedError: "invalid file extension must have toml extension", + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + actualPath, err := expectedCfg.Export(tc.filePath) + if tc.expectedError != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedError) + return + } + require.NoError(t, err) + require.Equal(t, actualPath, tc.expectedPath) + + // load file + actualCfg, err := GetConfigFromFile(actualPath) + require.NoError(t, err) + + require.EqualValues(t, expectedCfg, actualCfg) + }) + + } +} + var sink interface{} func BenchmarkDetailString(b *testing.B) { From 2d2da1b1734fe4e4c71f9d400ca15934a9429a4c Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Thu, 11 Apr 2024 18:33:29 +0530 Subject: [PATCH 4/9] fix: linter errors --- tools/cosmovisor/args.go | 2 -- tools/cosmovisor/args_test.go | 4 ++-- tools/cosmovisor/cmd/cosmovisor/init_test.go | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index ea8df69ef075..5b56e04b6939 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -494,7 +494,6 @@ func getTimeFormatOption(val string) (string, error) { return "", nil } return "", fmt.Errorf("env variable %q must have a timeformat value (\"layout|ansic|unixdate|rubydate|rfc822|rfc822z|rfc850|rfc1123|rfc1123z|rfc3339|rfc3339nano|kitchen\"), got %q", EnvTimeFormatLogs, val) - } // ValueToTimeFormatOption converts the time format option to the env value @@ -527,7 +526,6 @@ func ValueToTimeFormatOption(format string) string { default: return "" } - } // DetailString returns a multi-line string with details about this config. diff --git a/tools/cosmovisor/args_test.go b/tools/cosmovisor/args_test.go index f020bfa8ae1c..4dc0eb278d21 100644 --- a/tools/cosmovisor/args_test.go +++ b/tools/cosmovisor/args_test.go @@ -794,6 +794,8 @@ func setUpDir(t *testing.T) string { } func setupConfig(t *testing.T, home string) string { + t.Helper() + cfg := newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, "kitchen", "", true, 10) path := filepath.Join(home, rootName, "config.toml") f, err := os.Create(path) @@ -859,7 +861,6 @@ func TestConfigFromFile(t *testing.T) { }, expectedCfg: func() *Config { return newConfig(home, "env-name", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) - }, }, } @@ -877,7 +878,6 @@ func TestConfigFromFile(t *testing.T) { require.NoError(t, err) require.EqualValues(t, tc.expectedCfg(), actualCfg) - }) } } diff --git a/tools/cosmovisor/cmd/cosmovisor/init_test.go b/tools/cosmovisor/cmd/cosmovisor/init_test.go index 502fcc24ea1f..754a8a312c0c 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/init_test.go @@ -451,7 +451,7 @@ func (s *InitTestSuite) TestInitializeCosmovisorInvalidExisting() { _, logger := s.NewCapturingLogger() logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) err := InitializeCosmovisor(logger, []string{hwExe}) - require.ErrorContains(t, err, "config file must have toml extension") + require.ErrorContains(t, err, "invalid file extension") }) // Failure cases not tested: From 44a724ee1cb52300bb380bef080a47c9cf17efff Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Fri, 12 Apr 2024 08:40:56 +0530 Subject: [PATCH 5/9] docs: update changelog --- tools/cosmovisor/CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/cosmovisor/CHANGELOG.md b/tools/cosmovisor/CHANGELOG.md index e2d51a1106de..33b40061e9b0 100644 --- a/tools/cosmovisor/CHANGELOG.md +++ b/tools/cosmovisor/CHANGELOG.md @@ -36,6 +36,15 @@ 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. + +## Improvements +* [#19995](https://github.com/cosmos/cosmos-sdk/pull/19995): + * Add `--export-config` flag to provide `config.toml` path to initialise configuration in `cosmovisor init`. + * Set `CONFIG` environment variable to provide `config.toml` path to load the configuration in `cosmovisor run` + * Add `--config` flag to provide `config.toml` path to the configuration file in `cosmovisor add-upgrade`. + ## v1.5.0 - 2023-07-17 ## Features From 74c8b24dfafd903bffb3e687f65a7803d3b48736 Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Fri, 12 Apr 2024 09:18:49 +0530 Subject: [PATCH 6/9] fix: changelog indentation --- tools/cosmovisor/CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/cosmovisor/CHANGELOG.md b/tools/cosmovisor/CHANGELOG.md index 33b40061e9b0..d8df9876c13a 100644 --- a/tools/cosmovisor/CHANGELOG.md +++ b/tools/cosmovisor/CHANGELOG.md @@ -37,13 +37,15 @@ 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. ## Improvements + * [#19995](https://github.com/cosmos/cosmos-sdk/pull/19995): - * Add `--export-config` flag to provide `config.toml` path to initialise configuration in `cosmovisor init`. - * Set `CONFIG` environment variable to provide `config.toml` path to load the configuration in `cosmovisor run` - * Add `--config` flag to provide `config.toml` path to the configuration file in `cosmovisor add-upgrade`. + * Add `--export-config` flag to provide `config.toml` path to initialise configuration in `cosmovisor init`. + * Set `CONFIG` environment variable to provide `config.toml` path to load the configuration in `cosmovisor run` + * Add `--config` flag to provide `config.toml` path to the configuration file in `cosmovisor add-upgrade`. ## v1.5.0 - 2023-07-17 From fa973a59f9e83f8980d5ce767ddb3ceabfcba2b3 Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Sun, 14 Apr 2024 13:18:30 +0530 Subject: [PATCH 7/9] address comments --- tools/cosmovisor/CHANGELOG.md | 7 +- tools/cosmovisor/args.go | 69 ++++---- tools/cosmovisor/args_test.go | 123 ++++---------- .../cosmovisor/cmd/cosmovisor/add_upgrade.go | 1 - tools/cosmovisor/cmd/cosmovisor/config.go | 8 +- tools/cosmovisor/cmd/cosmovisor/init.go | 14 +- tools/cosmovisor/cmd/cosmovisor/init_test.go | 158 +++++++++++++----- tools/cosmovisor/cmd/cosmovisor/root.go | 9 +- tools/cosmovisor/cmd/cosmovisor/run.go | 6 +- .../cosmovisor/cmd/cosmovisor/version_test.go | 3 +- tools/cosmovisor/flags.go | 1 - 11 files changed, 206 insertions(+), 193 deletions(-) diff --git a/tools/cosmovisor/CHANGELOG.md b/tools/cosmovisor/CHANGELOG.md index d8df9876c13a..8c6f7192881a 100644 --- a/tools/cosmovisor/CHANGELOG.md +++ b/tools/cosmovisor/CHANGELOG.md @@ -43,9 +43,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Improvements * [#19995](https://github.com/cosmos/cosmos-sdk/pull/19995): - * Add `--export-config` flag to provide `config.toml` path to initialise configuration in `cosmovisor init`. - * Set `CONFIG` environment variable to provide `config.toml` path to load the configuration in `cosmovisor run` - * Add `--config` flag to provide `config.toml` path to the configuration file in `cosmovisor add-upgrade`. + * `init command` writes the configuration to the config file only at the default path `DAEMON_HOME/cosmovisor/config.toml`. + * Set `CONFIG` environment variable to provide `config.toml` path to load the configuration in `cosmovisor run`. + * Add `--config` flag to provide `config.toml` path to the configuration file in root command used by `add-upgrade` and `config` subcommands. + * `config command` now displays the configuration from the config file if it is provided. If the config file is not provided, it will display the configuration from the environment variables. ## v1.5.0 - 2023-07-17 diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 5b56e04b6939..7567af547808 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -54,22 +54,22 @@ const ( // Config is the information passed in to control the daemon type Config struct { - Home string `toml:"DAEMON_HOME" mapstructure:"DAEMON_HOME"` - Name string `toml:"DAEMON_NAME" mapstructure:"DAEMON_NAME"` - AllowDownloadBinaries bool `toml:"DAEMON_ALLOW_DOWNLOAD_BINARIES" mapstructure:"DAEMON_ALLOW_DOWNLOAD_BINARIES" default:"false"` - DownloadMustHaveChecksum bool `toml:"DAEMON_DOWNLOAD_MUST_HAVE_CHECKSUM" mapstructure:"DAEMON_DOWNLOAD_MUST_HAVE_CHECKSUM" default:"false"` - RestartAfterUpgrade bool `toml:"DAEMON_RESTART_AFTER_UPGRADE" mapstructure:"DAEMON_RESTART_AFTER_UPGRADE" default:"true"` - RestartDelay time.Duration `toml:"DAEMON_RESTART_DELAY" mapstructure:"DAEMON_RESTART_DELAY"` - ShutdownGrace time.Duration `toml:"DAEMON_SHUTDOWN_GRACE" mapstructure:"DAEMON_SHUTDOWN_GRACE"` - PollInterval time.Duration `toml:"DAEMON_POLL_INTERVAL" mapstructure:"DAEMON_POLL_INTERVAL" default:"300ms"` - UnsafeSkipBackup bool `toml:"UNSAFE_SKIP_BACKUP" mapstructure:"UNSAFE_SKIP_BACKUP" default:"false"` - DataBackupPath string `toml:"DAEMON_DATA_BACKUP_DIR" mapstructure:"DAEMON_DATA_BACKUP_DIR"` - PreUpgradeMaxRetries int `toml:"DAEMON_PREUPGRADE_MAX_RETRIES" mapstructure:"DAEMON_PREUPGRADE_MAX_RETRIES" default:"0"` - DisableLogs bool `toml:"COSMOVISOR_DISABLE_LOGS" mapstructure:"COSMOVISOR_DISABLE_LOGS" default:"false"` - ColorLogs bool `toml:"COSMOVISOR_COLOR_LOGS" mapstructure:"COSMOVISOR_COLOR_LOGS" default:"true"` - TimeFormatLogs string `toml:"COSMOVISOR_TIMEFORMAT_LOGS" mapstructure:"COSMOVISOR_TIMEFORMAT_LOGS" default:"kitchen"` - CustomPreUpgrade string `toml:"COSMOVISOR_CUSTOM_PREUPGRADE" mapstructure:"COSMOVISOR_CUSTOM_PREUPGRADE" default:""` - DisableRecase bool `toml:"COSMOVISOR_DISABLE_RECASE" mapstructure:"COSMOVISOR_DISABLE_RECASE" default:"false"` + Home string `toml:"daemon_home" mapstructure:"daemon_home"` + Name string `toml:"daemon_name" mapstructure:"daemon_name"` + AllowDownloadBinaries bool `toml:"daemon_allow_download_binaries" mapstructure:"daemon_allow_download_binaries" default:"false"` + DownloadMustHaveChecksum bool `toml:"daemon_download_must_have_checksum" mapstructure:"daemon_download_must_have_checksum" default:"false"` + RestartAfterUpgrade bool `toml:"daemon_restart_after_upgrade" mapstructure:"daemon_restart_after_upgrade" default:"true"` + RestartDelay time.Duration `toml:"daemon_restart_delay" mapstructure:"daemon_restart_delay"` + ShutdownGrace time.Duration `toml:"daemon_shutdown_grace" mapstructure:"daemon_shutdown_grace"` + PollInterval time.Duration `toml:"daemon_poll_interval" mapstructure:"daemon_poll_interval" default:"300ms"` + UnsafeSkipBackup bool `toml:"unsafe_skip_backup" mapstructure:"unsafe_skip_backup" default:"false"` + DataBackupPath string `toml:"daemon_data_backup_dir" mapstructure:"daemon_data_backup_dir"` + PreUpgradeMaxRetries int `toml:"daemon_preupgrade_max_retries" mapstructure:"daemon_preupgrade_max_retries" default:"0"` + DisableLogs bool `toml:"cosmovisor_disable_logs" mapstructure:"cosmovisor_disable_logs" default:"false"` + ColorLogs bool `toml:"cosmovisor_color_logs" mapstructure:"cosmovisor_color_logs" default:"true"` + TimeFormatLogs string `toml:"cosmovisor_timeforamt_logs" mapstructure:"cosmovisor_timeforamt_logs" default:"kitchen"` + CustomPreUpgrade string `toml:"cosmovisor_custom_preupgrade" mapstructure:"cosmovisor_custom_preupgrade" default:""` + DisableRecase bool `toml:"cosmovisor_disable_recase" mapstructure:"cosmovisor_disable_recase" default:"false"` // currently running upgrade currentUpgrade upgradetypes.Plan @@ -159,11 +159,11 @@ func (cfg *Config) CurrentBin() (string, error) { } // GetConfigFromFile will read the configuration from the file at the given path. -// It will return an error if the file does not exist or if the configuration is invalid. -// If ENV variables are set, they will override the values in the file. +// If the file path is not provided, it will try to 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 nil, ErrEmptyConfigENV + return GetConfigFromEnv() } // ensure the file exist @@ -577,15 +577,17 @@ func (cfg Config) DetailString() string { } // Export exports the configuration to a file at the given path. -func (cfg Config) Export(path string) (string, error) { - // if path is empty, use the default path - if path == "" { - path = cfg.DefaultCfgPath() - } - - // ensure the path has proper extension - if !strings.HasSuffix(path, cfgExtension) { - return "", fmt.Errorf("invalid file extension must have %s extension", cfgExtension) +func (cfg Config) Export() (string, error) { + // always use the default path + path := filepath.Clean(cfg.DefaultCfgPath()) + + // check if config file already exists ask user if they want to overwrite it + if _, err := os.Stat(path); err == nil { + // ask user if they want to overwrite the file + if !askForConfirmation(fmt.Sprintf("file %s already exists, do you want to overwrite it?", path)) { + cfg.Logger(os.Stdout).Info("file already exists, not overriding") + return path, nil + } } // create the file @@ -607,3 +609,14 @@ func (cfg Config) Export(path string) (string, error) { return path, nil } + +func askForConfirmation(str string) bool { + var response string + fmt.Printf("%s [y/n]: ", str) + _, err := fmt.Scanln(&response) + if err != nil { + return false + } + + return strings.ToLower(response) == "y" +} diff --git a/tools/cosmovisor/args_test.go b/tools/cosmovisor/args_test.go index 4dc0eb278d21..b379720763f0 100644 --- a/tools/cosmovisor/args_test.go +++ b/tools/cosmovisor/args_test.go @@ -784,36 +784,36 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { } } -func setUpDir(t *testing.T) string { - t.Helper() +func (s *argsTestSuite) setUpDir() string { + s.T().Helper() - home := t.TempDir() + home := s.T().TempDir() err := os.MkdirAll(filepath.Join(home, rootName), 0o755) - require.NoError(t, err) + s.Require().NoError(err) return home } -func setupConfig(t *testing.T, home string) string { - t.Helper() +func (s *argsTestSuite) setupConfig(home string) string { + s.T().Helper() - cfg := newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, "kitchen", "", true, 10) + cfg := newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, "kitchen", "", true, 10000000000) path := filepath.Join(home, rootName, "config.toml") f, err := os.Create(path) - require.NoError(t, err) + s.Require().NoError(err) enc := toml.NewEncoder(f) - require.NoError(t, enc.Encode(&cfg)) + s.Require().NoError(enc.Encode(&cfg)) err = f.Close() - require.NoError(t, err) + s.Require().NoError(err) return path } -func TestConfigFromFile(t *testing.T) { - home := setUpDir(t) +func (s *argsTestSuite) TestConfigFromFile() { + home := s.setUpDir() // create a config file - cfgFilePath := setupConfig(t, home) + cfgFilePath := s.setupConfig(home) testCases := []struct { name string @@ -826,31 +826,12 @@ func TestConfigFromFile(t *testing.T) { { name: "valid config", expectedCfg: func() *Config { - return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) + return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10000000000) }, filePath: cfgFilePath, expectedError: "", malleate: func() {}, }, - - { - name: "empty config file path", - expectedCfg: func() *Config { - return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) - }, - filePath: "", - expectedError: ErrEmptyConfigENV.Error(), - malleate: func() {}, - }, - { - name: "invalid config file path", - expectedCfg: func() *Config { - return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) - }, - filePath: filepath.Join(home, "invalid.toml"), - expectedError: "config not found", - malleate: func() {}, - }, { name: "env variable will override config file fields", filePath: cfgFilePath, @@ -860,81 +841,35 @@ func TestConfigFromFile(t *testing.T) { os.Setenv(EnvName, "env-name") }, expectedCfg: func() *Config { - return newConfig(home, "env-name", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10) + return newConfig(home, "env-name", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10000000000) }, }, - } - - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - tc.malleate() - actualCfg, err := GetConfigFromFile(tc.filePath) - if tc.expectedError != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tc.expectedError) - return - } - - require.NoError(t, err) - require.EqualValues(t, tc.expectedCfg(), actualCfg) - }) - } -} - -func TestConfigExport(t *testing.T) { - home := setUpDir(t) - // create a config file - cfgFilePath := setupConfig(t, home) - // load file - expectedCfg, err := GetConfigFromFile(cfgFilePath) - require.NoError(t, err) - - testCases := []struct { - name string - filePath string - expectedPath string - expectedError string - }{ - { - name: "valid config", - filePath: cfgFilePath, - expectedPath: filepath.Join(expectedCfg.Root(), "config.toml"), - expectedError: "", - }, { - name: "empty config file path will use default path", + name: "empty config file path will load config from ENV variables", + expectedCfg: func() *Config { + return newConfig(home, "test", true, true, true, 406, false, home, 8, 0, false, true, time.Kitchen, "", true, 10000000000) + }, filePath: "", - expectedPath: expectedCfg.DefaultCfgPath(), expectedError: "", - }, - { - name: "invalid file extension", - filePath: filepath.Join(home, rootName, "config.json"), - expectedPath: "", - expectedError: "invalid file extension must have toml extension", + malleate: func() { + s.setEnv(s.T(), &cosmovisorEnv{home, "test", "true", "true", "true", "406ms", "false", home, "8ms", "0", "false", "true", "kitchen", "", "true", "10s"}) + }, }, } for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - actualPath, err := expectedCfg.Export(tc.filePath) + s.T().Run(tc.name, func(t *testing.T) { + tc.malleate() + actualCfg, err := GetConfigFromFile(tc.filePath) if tc.expectedError != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tc.expectedError) + s.Require().NoError(err) + s.Require().Contains(err.Error(), tc.expectedError) return } - require.NoError(t, err) - require.Equal(t, actualPath, tc.expectedPath) - // load file - actualCfg, err := GetConfigFromFile(actualPath) - require.NoError(t, err) - - require.EqualValues(t, expectedCfg, actualCfg) + s.Require().NoError(err) + s.Require().EqualValues(tc.expectedCfg(), actualCfg) }) - } } diff --git a/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go b/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go index c2e4ba5cadf8..1b7a09ac7399 100644 --- a/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go +++ b/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go @@ -25,7 +25,6 @@ func NewAddUpgradeCmd() *cobra.Command { addUpgrade.Flags().Bool(cosmovisor.FlagForce, false, "overwrite existing upgrade binary / upgrade-info.json file") addUpgrade.Flags().Int64(cosmovisor.FlagUpgradeHeight, 0, "define a height at which to upgrade the binary automatically (without governance proposal)") - addUpgrade.Flags().String(cosmovisor.FlagConfig, "", "path to cosmovisor config file") return addUpgrade } diff --git a/tools/cosmovisor/cmd/cosmovisor/config.go b/tools/cosmovisor/cmd/cosmovisor/config.go index 927e005ebf6a..70bdcb67ccd3 100644 --- a/tools/cosmovisor/cmd/cosmovisor/config.go +++ b/tools/cosmovisor/cmd/cosmovisor/config.go @@ -7,11 +7,13 @@ import ( ) var configCmd = &cobra.Command{ - Use: "config", - Short: "Display cosmovisor config (prints environment variables used by cosmovisor).", + Use: "config", + Short: "Display cosmovisor config.", + Long: `Display cosmovisor config. If a config file is provided, it will display the config from the file, +otherwise it will display the config from the environment variables.`, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - cfg, err := cosmovisor.GetConfigFromEnv() + cfg, err := cosmovisor.GetConfigFromFile(cmd.Flag(cosmovisor.FlagConfig).Value.String()) if err != nil { return err } diff --git a/tools/cosmovisor/cmd/cosmovisor/init.go b/tools/cosmovisor/cmd/cosmovisor/init.go index 5f35e309b7ac..27bf7ae1e035 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init.go +++ b/tools/cosmovisor/cmd/cosmovisor/init.go @@ -9,7 +9,6 @@ import ( "time" "github.com/spf13/cobra" - "github.com/spf13/viper" "cosmossdk.io/log" "cosmossdk.io/tools/cosmovisor" @@ -18,18 +17,17 @@ import ( func NewIntCmd() *cobra.Command { initCmd := &cobra.Command{ - Use: "init ", - Short: "Initialize a cosmovisor daemon home directory.", + Use: "init ", + Short: "Initialize a cosmovisor daemon home directory.", + Long: `Initialize a cosmovisor daemon home directory with the provided executable +Configuration file uses default path (<-home->/cosmovisor/config.toml) to export the config.`, Args: cobra.ExactArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - mustNoError(viper.BindPFlags(cmd.Flags())) return InitializeCosmovisor(nil, args) }, } - initCmd.Flags().String(cosmovisor.FlagExportConfig, "", "path to export the configuration file to (default is <-home_path->/cosmovisor/config.toml") - return initCmd } @@ -96,12 +94,12 @@ func InitializeCosmovisor(logger log.Logger, args []string) error { } logger.Info(fmt.Sprintf("the current symlink points to: %q", cur)) - filePath, err := cfg.Export(viper.GetString(cosmovisor.FlagExportConfig)) + filePath, err := cfg.Export() if err != nil { return fmt.Errorf("failed to export configuration: %w", err) } - logger.Info(fmt.Sprintf("exported configuration to: %s", filePath)) + logger.Info(fmt.Sprintf("config file present at: %s", filePath)) return nil } diff --git a/tools/cosmovisor/cmd/cosmovisor/init_test.go b/tools/cosmovisor/cmd/cosmovisor/init_test.go index 754a8a312c0c..95d971ab08ed 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/init_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/pelletier/go-toml/v2" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -116,6 +117,26 @@ func (s *InitTestSuite) setEnv(t *testing.T, env *cosmovisorInitEnv) { //nolint: } } +// readStdInpFromFile reads the provided data as if it were a standard input. +func (s *InitTestSuite) readStdInpFromFile(data []byte) { + // Create a temporary file and write the test input into it + tmpfile, err := os.CreateTemp("", "test") + if err != nil { + s.T().Fatal(err) + } + + // write the test input into the temporary file + if _, err := tmpfile.Write(data); err != nil { + s.T().Fatal(err) + } + + if _, err := tmpfile.Seek(0, 0); err != nil { + s.T().Fatal(err) + } + + os.Stdin = tmpfile +} + var ( _ io.Reader = BufferedPipe{} _ io.Writer = BufferedPipe{} @@ -438,22 +459,6 @@ func (s *InitTestSuite) TestInitializeCosmovisorInvalidExisting() { assert.Contains(t, bufferStr, "checking on the current symlink and creating it if needed") }) - s.T().Run("flag export config file invalid extension", func(t *testing.T) { - testDir := t.TempDir() - env := &cosmovisorInitEnv{ - Home: filepath.Join(testDir, "home"), - Name: "pineapple", - } - - s.setEnv(t, env) - // setting an invalid extension for config file - viper.Set(cosmovisor.FlagExportConfig, filepath.Join(env.Home, cosmovisorDirName, "config.json")) - _, logger := s.NewCapturingLogger() - logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) - err := InitializeCosmovisor(logger, []string{hwExe}) - require.ErrorContains(t, err, "invalid file extension") - }) - // Failure cases not tested: // Cannot create genesis bin directory // I had a test for this that created the `genesis` directory with permissions 0o555. @@ -497,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("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), + fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(s.T(), env) @@ -550,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("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), + fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(t, env) @@ -583,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("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), + fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt)), } s.setEnv(t, env) @@ -598,30 +603,10 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { } }) - s.T().Run("flag export config use provide path", func(t *testing.T) { - testDir := s.T().TempDir() - env := &cosmovisorInitEnv{ - Home: filepath.Join(testDir, "home"), - Name: "emptygen", - } - - s.setEnv(t, env) - buffer, logger := s.NewCapturingLogger() - - expectedPath := filepath.Join(env.Home, cfgFileWithExt) - // setting a path to export flag - viper.Set(cosmovisor.FlagExportConfig, expectedPath) - expectedInLog := fmt.Sprintf("exported configuration to: %s", expectedPath) - - logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) - err := InitializeCosmovisor(logger, []string{hwExe}) - require.NoError(t, err, "calling InitializeCosmovisor") - bufferBz := buffer.Collect() - bufferStr := string(bufferBz) - assert.Contains(t, bufferStr, expectedInLog) + s.T().Run("ask to override (y/n) the existing config file", func(t *testing.T) { }) - s.T().Run("flag export config use default path if not provided", func(t *testing.T) { + s.T().Run("init command exports configs to default path", func(t *testing.T) { testDir := s.T().TempDir() env := &cosmovisorInitEnv{ Home: filepath.Join(testDir, "home"), @@ -630,13 +615,98 @@ func (s *InitTestSuite) TestInitializeCosmovisorValid() { s.setEnv(t, env) buffer, logger := s.NewCapturingLogger() - // setting empty path to export flag - viper.Set(cosmovisor.FlagExportConfig, "") logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) err := InitializeCosmovisor(logger, []string{hwExe}) require.NoError(t, err, "calling InitializeCosmovisor") bufferBz := buffer.Collect() bufferStr := string(bufferBz) - assert.Contains(t, bufferStr, fmt.Sprintf("exported configuration to: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt))) + assert.Contains(t, bufferStr, fmt.Sprintf("config file present at: %s", filepath.Join(env.Home, cosmovisorDirName, cfgFileWithExt))) }) } + +func (s *InitTestSuite) TestInitializeCosmovisorWithOverrideCfg() { + initEnv := s.clearEnv() + defer s.setEnv(nil, initEnv) + + tmpExe := s.CreateHelloWorld(0o755) + testDir := s.T().TempDir() + homePath := filepath.Join(testDir, "backup") + testCases := []struct { + name string + input string + cfg *cosmovisor.Config + override bool + }{ + { + name: "yes override", + input: "y\n", + cfg: &cosmovisor.Config{ + Home: homePath, + Name: "old_test", + DataBackupPath: homePath, + }, + override: true, + }, + { + name: "no override", + input: "n\n", + cfg: &cosmovisor.Config{ + Home: homePath, + Name: "old_test", + DataBackupPath: homePath, + }, + override: false, + }, + } + + for _, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + // create a root cosmovisor directory + require.NoError(t, os.MkdirAll(tc.cfg.Root(), 0o755), "making root dir") + + // create a config file in the default location + file, err := os.Create(tc.cfg.DefaultCfgPath()) + require.NoError(t, err) + + // write the config to the file + err = toml.NewEncoder(file).Encode(tc.cfg) + require.NoError(t, err) + + err = file.Close() + require.NoError(t, err) + + s.readStdInpFromFile([]byte(tc.input)) + + _, logger := s.NewCapturingLogger() + logger.Info(fmt.Sprintf("Calling InitializeCosmovisor: %s", t.Name())) + + // override the daemon name in environment file + // if override is true (y), then the name should be updated in the config file + // otherwise (n), the name should not be updated in the config file + s.setEnv(t, &cosmovisorInitEnv{ + Home: tc.cfg.Home, + Name: "update_name", + }) + + err = InitializeCosmovisor(logger, []string{tmpExe}) + require.NoError(t, err, "calling InitializeCosmovisor") + + cfg := &cosmovisor.Config{} + // read the config file + cfgFile, err := os.Open(tc.cfg.DefaultCfgPath()) + require.NoError(t, err) + defer cfgFile.Close() + + err = toml.NewDecoder(cfgFile).Decode(cfg) + require.NoError(t, err) + if tc.override { + // check if the name is updated + // basically, override the existing config file + assert.Equal(t, "update_name", cfg.Name) + } else { + // daemon name should not be updated + assert.Equal(t, tc.cfg.Name, cfg.Name) + } + }) + } +} diff --git a/tools/cosmovisor/cmd/cosmovisor/root.go b/tools/cosmovisor/cmd/cosmovisor/root.go index 41c283144365..e176ddb822b6 100644 --- a/tools/cosmovisor/cmd/cosmovisor/root.go +++ b/tools/cosmovisor/cmd/cosmovisor/root.go @@ -2,6 +2,8 @@ package main import ( "github.com/spf13/cobra" + + "cosmossdk.io/tools/cosmovisor" ) func NewRootCmd() *cobra.Command { @@ -19,11 +21,6 @@ func NewRootCmd() *cobra.Command { NewAddUpgradeCmd(), ) + rootCmd.PersistentFlags().String(cosmovisor.FlagConfig, "", "path to cosmovisor config file") return rootCmd } - -func mustNoError(err error) { - if err != nil { - panic(err) - } -} diff --git a/tools/cosmovisor/cmd/cosmovisor/run.go b/tools/cosmovisor/cmd/cosmovisor/run.go index 1c46cb707281..da3acc521ad7 100644 --- a/tools/cosmovisor/cmd/cosmovisor/run.go +++ b/tools/cosmovisor/cmd/cosmovisor/run.go @@ -15,9 +15,9 @@ const ( 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 process manager. -It will run the configured program and monitor it for upgrades. -Initialize the cosmovisor configuration with the 'init' command.`, + Long: `Run an APP command. This command is intended to be used by the cosmovisor binary. +Provide config file path via CONFIG environment variable. +`, SilenceUsage: true, DisableFlagParsing: true, RunE: func(_ *cobra.Command, args []string) error { diff --git a/tools/cosmovisor/cmd/cosmovisor/version_test.go b/tools/cosmovisor/cmd/cosmovisor/version_test.go index f71f8cdd283f..8f51ea47dafb 100644 --- a/tools/cosmovisor/cmd/cosmovisor/version_test.go +++ b/tools/cosmovisor/cmd/cosmovisor/version_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/require" "cosmossdk.io/log" - "cosmossdk.io/tools/cosmovisor" ) func TestVersionCommand_Error(t *testing.T) { @@ -24,5 +23,5 @@ func TestVersionCommand_Error(t *testing.T) { ctx := context.WithValue(context.Background(), log.ContextKey, logger) require.Error(t, rootCmd.ExecuteContext(ctx)) - require.Contains(t, out.String(), cosmovisor.ErrEmptyConfigENV.Error()) + require.Contains(t, out.String(), "DAEMON_NAME is not set") } diff --git a/tools/cosmovisor/flags.go b/tools/cosmovisor/flags.go index 04a970eaa5f0..d86c920b8c31 100644 --- a/tools/cosmovisor/flags.go +++ b/tools/cosmovisor/flags.go @@ -7,5 +7,4 @@ const ( FlagForce = "force" FlagUpgradeHeight = "upgrade-height" FlagConfig = "config" - FlagExportConfig = "export-config" ) From 03aa6d458949464e54453f775eb4d202554392e5 Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Sun, 14 Apr 2024 14:05:50 +0530 Subject: [PATCH 8/9] nit: fix typo --- tools/cosmovisor/args.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 7567af547808..9417f5256728 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -67,7 +67,7 @@ type Config struct { PreUpgradeMaxRetries int `toml:"daemon_preupgrade_max_retries" mapstructure:"daemon_preupgrade_max_retries" default:"0"` DisableLogs bool `toml:"cosmovisor_disable_logs" mapstructure:"cosmovisor_disable_logs" default:"false"` ColorLogs bool `toml:"cosmovisor_color_logs" mapstructure:"cosmovisor_color_logs" default:"true"` - TimeFormatLogs string `toml:"cosmovisor_timeforamt_logs" mapstructure:"cosmovisor_timeforamt_logs" default:"kitchen"` + TimeFormatLogs string `toml:"cosmovisor_timeformat_logs" mapstructure:"cosmovisor_timeformat_logs" default:"kitchen"` CustomPreUpgrade string `toml:"cosmovisor_custom_preupgrade" mapstructure:"cosmovisor_custom_preupgrade" default:""` DisableRecase bool `toml:"cosmovisor_disable_recase" mapstructure:"cosmovisor_disable_recase" default:"false"` From 9a9a729bbe1c083428d51b5b12bace9255e2a194 Mon Sep 17 00:00:00 2001 From: Aryan Tikarya Date: Tue, 23 Apr 2024 14:57:25 +0530 Subject: [PATCH 9/9] address comments --- tools/cosmovisor/CHANGELOG.md | 4 +- tools/cosmovisor/args.go | 2 - .../cosmovisor/cmd/cosmovisor/add_upgrade.go | 2 +- tools/cosmovisor/cmd/cosmovisor/config.go | 2 +- tools/cosmovisor/cmd/cosmovisor/init.go | 4 +- tools/cosmovisor/cmd/cosmovisor/root.go | 2 +- tools/cosmovisor/cmd/cosmovisor/run.go | 41 +++++++++++++++---- tools/cosmovisor/cmd/cosmovisor/version.go | 3 +- tools/cosmovisor/flags.go | 2 +- 9 files changed, 42 insertions(+), 20 deletions(-) diff --git a/tools/cosmovisor/CHANGELOG.md b/tools/cosmovisor/CHANGELOG.md index 8c6f7192881a..ff0bd081995b 100644 --- a/tools/cosmovisor/CHANGELOG.md +++ b/tools/cosmovisor/CHANGELOG.md @@ -44,8 +44,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#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`. - * Set `CONFIG` environment variable to provide `config.toml` path to load the configuration in `cosmovisor run`. - * Add `--config` flag to provide `config.toml` path to the configuration file in root command used by `add-upgrade` and `config` subcommands. + * 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) ...`. + * Add `--cosmovisor-config` flag to provide `config.toml` path to the configuration file in root command used by `add-upgrade` and `config` subcommands. * `config command` now displays the configuration from the config file if it is provided. If the config file is not provided, it will display the configuration from the environment variables. ## v1.5.0 - 2023-07-17 diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 9417f5256728..e89112749b30 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -20,8 +20,6 @@ import ( upgradetypes "cosmossdk.io/x/upgrade/types" ) -var ErrEmptyConfigENV = errors.New("config env variable not set or empty") - // environment variable names const ( EnvHome = "DAEMON_HOME" diff --git a/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go b/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go index 1b7a09ac7399..b64550f6447e 100644 --- a/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go +++ b/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go @@ -31,7 +31,7 @@ func NewAddUpgradeCmd() *cobra.Command { // AddUpgrade adds upgrade info to manifest func AddUpgrade(cmd *cobra.Command, args []string) error { - configPath, err := cmd.Flags().GetString(cosmovisor.FlagConfig) + configPath, err := cmd.Flags().GetString(cosmovisor.FlagCosmovisorConfig) if err != nil { return fmt.Errorf("failed to get config flag: %w", err) } diff --git a/tools/cosmovisor/cmd/cosmovisor/config.go b/tools/cosmovisor/cmd/cosmovisor/config.go index 70bdcb67ccd3..d651e5fb3839 100644 --- a/tools/cosmovisor/cmd/cosmovisor/config.go +++ b/tools/cosmovisor/cmd/cosmovisor/config.go @@ -13,7 +13,7 @@ var configCmd = &cobra.Command{ otherwise it will display the config from the environment variables.`, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - cfg, err := cosmovisor.GetConfigFromFile(cmd.Flag(cosmovisor.FlagConfig).Value.String()) + cfg, err := cosmovisor.GetConfigFromFile(cmd.Flag(cosmovisor.FlagCosmovisorConfig).Value.String()) if err != nil { return err } diff --git a/tools/cosmovisor/cmd/cosmovisor/init.go b/tools/cosmovisor/cmd/cosmovisor/init.go index 27bf7ae1e035..66d99b68032e 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init.go +++ b/tools/cosmovisor/cmd/cosmovisor/init.go @@ -19,8 +19,8 @@ func NewIntCmd() *cobra.Command { initCmd := &cobra.Command{ Use: "init ", Short: "Initialize a cosmovisor daemon home directory.", - Long: `Initialize a cosmovisor daemon home directory with the provided executable -Configuration file uses default path (<-home->/cosmovisor/config.toml) to export the config.`, + Long: `Initialize a cosmovisor daemon home directory with the provided executable. +Configuration file is initialized at the default path (<-home->/cosmovisor/config.toml).`, Args: cobra.ExactArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/tools/cosmovisor/cmd/cosmovisor/root.go b/tools/cosmovisor/cmd/cosmovisor/root.go index e176ddb822b6..d9f6094d593c 100644 --- a/tools/cosmovisor/cmd/cosmovisor/root.go +++ b/tools/cosmovisor/cmd/cosmovisor/root.go @@ -21,6 +21,6 @@ func NewRootCmd() *cobra.Command { NewAddUpgradeCmd(), ) - rootCmd.PersistentFlags().String(cosmovisor.FlagConfig, "", "path to cosmovisor config file") + rootCmd.PersistentFlags().StringP(cosmovisor.FlagCosmovisorConfig, "c", "", "path to cosmovisor config file") return rootCmd } diff --git a/tools/cosmovisor/cmd/cosmovisor/run.go b/tools/cosmovisor/cmd/cosmovisor/run.go index da3acc521ad7..6aa938c7d780 100644 --- a/tools/cosmovisor/cmd/cosmovisor/run.go +++ b/tools/cosmovisor/cmd/cosmovisor/run.go @@ -1,33 +1,35 @@ package main import ( - "os" + "fmt" + "strings" "github.com/spf13/cobra" "cosmossdk.io/tools/cosmovisor" ) -const ( - cfgENV = "CONFIG" -) - 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 config file path via CONFIG environment variable. +Provide cosmovisor config file path in command args or set env variables to load configuration. `, SilenceUsage: true, DisableFlagParsing: true, RunE: func(_ *cobra.Command, args []string) error { - return run(args) + cfgPath, args, err := parseCosmovisorConfig(args) + if err != nil { + return fmt.Errorf("failed to parse cosmovisor config: %w", err) + } + + return run(cfgPath, args) }, } // run runs the configured program with the given args and monitors it for upgrades. -func run(args []string, options ...RunOption) error { - cfg, err := cosmovisor.GetConfigFromFile(os.Getenv(cfgENV)) +func run(cfgPath string, args []string, options ...RunOption) error { + cfg, err := cosmovisor.GetConfigFromFile(cfgPath) if err != nil { return err } @@ -56,3 +58,24 @@ func run(args []string, options ...RunOption) error { return err } + +func parseCosmovisorConfig(args []string) (string, []string, error) { + var configFilePath string + for i, arg := range args { + // Check if the argument is the config flag + if strings.EqualFold(arg, fmt.Sprintf("--%s", cosmovisor.FlagCosmovisorConfig)) || + strings.EqualFold(arg, fmt.Sprintf("-%s", cosmovisor.FlagCosmovisorConfig)) { + // Check if there is an argument after the flag which should be the config file path + if i+1 >= len(args) { + return "", nil, fmt.Errorf("--%s requires an argument", cosmovisor.FlagCosmovisorConfig) + } + + configFilePath = args[i+1] + // Remove the flag and its value from the arguments + args = append(args[:i], args[i+2:]...) + break + } + } + + return configFilePath, args, nil +} diff --git a/tools/cosmovisor/cmd/cosmovisor/version.go b/tools/cosmovisor/cmd/cosmovisor/version.go index fa9778ea54d8..a51b376355af 100644 --- a/tools/cosmovisor/cmd/cosmovisor/version.go +++ b/tools/cosmovisor/cmd/cosmovisor/version.go @@ -47,7 +47,7 @@ func printVersion(cmd *cobra.Command, args []string, noAppVersion bool) error { return nil } - if err := run(append([]string{"version"}, args...)); err != nil { + if err := run("", append([]string{"version"}, args...)); err != nil { return fmt.Errorf("failed to run version command: %w", err) } @@ -62,6 +62,7 @@ func printVersionJSON(cmd *cobra.Command, args []string, noAppVersion bool) erro buf := new(strings.Builder) if err := run( + "", []string{"version", "--long", "--output", "json"}, StdOutRunOption(buf), ); err != nil { diff --git a/tools/cosmovisor/flags.go b/tools/cosmovisor/flags.go index d86c920b8c31..7c0db36bee66 100644 --- a/tools/cosmovisor/flags.go +++ b/tools/cosmovisor/flags.go @@ -6,5 +6,5 @@ const ( FlagCosmovisorOnly = "cosmovisor-only" FlagForce = "force" FlagUpgradeHeight = "upgrade-height" - FlagConfig = "config" + FlagCosmovisorConfig = "cosmovisor-config" )