From f6458f079521ea80e17a01bc96461cebe5aa9e39 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 19 Nov 2021 18:35:56 +0100 Subject: [PATCH] fix(cosmovisor): udpate the help and version command output (#10458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Closes: #10450 * `cosmovisor help` should not print any error. * `cosmovisor version` should print more clear message if it can't run `app version`. ## Before ``` ~/tmp ❯ cosmovisor version Cosmosvisor - A process manager for Cosmos SDK application binaries. Cosmovisor is a wrapper for a Cosmos SDK based App (set using the required DAEMON_NAME env variable). It starts the App by passing all provided arguments and monitors the DAEMON_HOME/data/upgrade-info.json file to perform an update. The upgrade-info.json file is created by the App x/upgrade module when the blockchain height reaches an approved upgrade proposal. The file includes data from the proposal. Cosmovisor interprets that data to perform an update: switch a current binary and restart the App. Configuration of Cosmovisor is done through environment variables, which are documented in: https://github.com/cosmos/cosmos-sdk/tree/master/cosmovisor/README.md 8:03PM ERR multiple configuration errors found: module=cosmovisor 8:03PM ERR 1: error="DAEMON_NAME is not set" module=cosmovisor 8:03PM ERR 2: error="DAEMON_HOME is not set" module=cosmovisor 8:03PM ERR error="2 errors: 1: DAEMON_NAME is not set, 2: DAEMON_HOME is not set" module=cosmovisor ❯ cosmovisor help Cosmosvisor - A process manager for Cosmos SDK application binaries. Cosmovisor is a wrapper for a Cosmos SDK based App (set using the required DAEMON_NAME env variable). It starts the App by passing all provided arguments and monitors the DAEMON_HOME/data/upgrade-info.json file to perform an update. The upgrade-info.json file is created by the App x/upgrade module when the blockchain height reaches an approved upgrade proposal. The file includes data from the proposal. Cosmovisor interprets that data to perform an update: switch a current binary and restart the App. Configuration of Cosmovisor is done through environment variables, which are documented in: https://github.com/cosmos/cosmos-sdk/tree/master/cosmovisor/README.md 8:03PM ERR multiple configuration errors found: module=cosmovisor 8:03PM ERR 1: error="DAEMON_NAME is not set" module=cosmovisor 8:03PM ERR 2: error="DAEMON_HOME is not set" module=cosmovisor 8:03PM ERR error="2 errors: 1: DAEMON_NAME is not set, 2: DAEMON_HOME is not set" module=cosmovisor ``` ## After ``` ❯ ./cosmovisor version Cosmovisor Version: v1.0.0-76-g66bddfb04 8:05PM ERR Can't run APP version 8:05PM ERR 1: error="DAEMON_NAME is not set" 8:05PM ERR 2: error="DAEMON_HOME is not set" ❯ ./cosmovisor help Cosmosvisor - A process manager for Cosmos SDK application binaries. Cosmovisor is a wrapper for a Cosmos SDK based App (set using the required DAEMON_NAME env variable). It starts the App by passing all provided arguments and monitors the DAEMON_HOME/data/upgrade-info.json file to perform an update. The upgrade-info.json file is created by the App x/upgrade module when the blockchain height reaches an approved upgrade proposal. The file includes data from the proposal. Cosmovisor interprets that data to perform an update: switch a current binary and restart the App. Configuration of Cosmovisor is done through environment variables, which are documented in: https://github.com/cosmos/cosmos-sdk/tree/master/cosmovisor/README.md To get help for the configured binary: cosmovisor run help ``` --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [x] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [x] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- cosmovisor/CHANGELOG.md | 4 + cosmovisor/args.go | 20 ++--- cosmovisor/args_test.go | 4 +- cosmovisor/cmd/cosmovisor/cmd/help.go | 18 ++--- cosmovisor/cmd/cosmovisor/cmd/help_test.go | 88 ---------------------- cosmovisor/cmd/cosmovisor/cmd/root.go | 5 +- cosmovisor/cmd/cosmovisor/cmd/run.go | 7 +- cosmovisor/cmd/cosmovisor/cmd/version.go | 17 ++++- cosmovisor/cmd/cosmovisor/main.go | 3 +- cosmovisor/errors/multi.go | 17 +++++ 10 files changed, 60 insertions(+), 123 deletions(-) diff --git a/cosmovisor/CHANGELOG.md b/cosmovisor/CHANGELOG.md index 91d0e9ffda..0504d4be12 100644 --- a/cosmovisor/CHANGELOG.md +++ b/cosmovisor/CHANGELOG.md @@ -44,6 +44,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ + [\#10285](https://github.com/cosmos/cosmos-sdk/pull/10316) Running `cosmovisor` without the `run` argument. +### Bug Fixes + ++ [\#10458](https://github.com/cosmos/cosmos-sdk/pull/10458) Fix version when using 'go install github.com/cosmos/cosmos-sdk/cosmovisor/cmd/cosmovisor@v1.0.0' to install cosmovisor. + ## v1.0.0 2021-09-30 ### Features diff --git a/cosmovisor/args.go b/cosmovisor/args.go index fa5ef20361..b58e4f7c14 100644 --- a/cosmovisor/args.go +++ b/cosmovisor/args.go @@ -177,20 +177,16 @@ func GetConfigFromEnv() (*Config, error) { } // LogConfigOrError logs either the config details or the error. -func LogConfigOrError(logger zerolog.Logger, cfg *Config, cerr error) { +func LogConfigOrError(logger zerolog.Logger, cfg *Config, err error) { + if cfg == nil && err == nil { + return + } + logger.Info().Msg("Configuration:") switch { - case cerr != nil: - switch err := cerr.(type) { - case *cverrors.MultiError: - logger.Error().Msg("multiple configuration errors found:") - for i, e := range err.GetErrors() { - logger.Error().Err(e).Msg(fmt.Sprintf(" %d:", i+1)) - } - default: - logger.Error().Err(cerr).Msg("configuration error:") - } + case err != nil: + cverrors.LogErrors(logger, "configuration errors found", err) case cfg != nil: - logger.Info().Msg("Configuration is valid:\n" + cfg.DetailString()) + logger.Info().Msg(cfg.DetailString()) } } diff --git a/cosmovisor/args_test.go b/cosmovisor/args_test.go index 86c0d0df1c..069050a1aa 100644 --- a/cosmovisor/args_test.go +++ b/cosmovisor/args_test.go @@ -554,14 +554,14 @@ func (s *argsTestSuite) TestLogConfigOrError() { name: "multi error", cfg: nil, err: errMulti, - contains: []string{"multiple configuration errors found", errs[0].Error(), errs[1].Error(), errs[2].Error()}, + contains: []string{"configuration errors found", errs[0].Error(), errs[1].Error(), errs[2].Error()}, notcontains: nil, }, { name: "config", cfg: cfg, err: nil, - contains: []string{"Configuration is valid", cfg.DetailString()}, + contains: []string{"Configurable Values", cfg.DetailString()}, notcontains: nil, }, { diff --git a/cosmovisor/cmd/cosmovisor/cmd/help.go b/cosmovisor/cmd/cosmovisor/cmd/help.go index abb757dd91..e25df4ee36 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/help.go +++ b/cosmovisor/cmd/cosmovisor/cmd/help.go @@ -2,10 +2,6 @@ package cmd import ( "fmt" - "os" - "time" - - "github.com/rs/zerolog" "github.com/cosmos/cosmos-sdk/cosmovisor" ) @@ -17,20 +13,13 @@ var HelpArgs = []string{"help", "--help", "-h"} // Help is needed if either cosmovisor.EnvName and/or cosmovisor.EnvHome env vars aren't set. // Help is requested if the first arg is "help", "--help", or "-h". func ShouldGiveHelp(arg string) bool { - return isOneOf(arg, HelpArgs) || len(os.Getenv(cosmovisor.EnvName)) == 0 || len(os.Getenv(cosmovisor.EnvHome)) == 0 + return isOneOf(arg, HelpArgs) } // DoHelp outputs help text func DoHelp() { // Not using the logger for this output because the header and footer look weird for help text. fmt.Println(GetHelpText()) - // Check the config and output details or any errors. - // Not using the cosmovisor.Logger in order to ignore any level it might have set, - // and also to not have any of the extra parameters in the output. - output := zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.Kitchen} - logger := zerolog.New(output).With().Timestamp().Logger() - cfg, err := cosmovisor.GetConfigFromEnv() - cosmovisor.LogConfigOrError(logger, cfg, err) } // GetHelpText creates the help text multi-line string. @@ -49,5 +38,10 @@ documented in: https://github.com/cosmos/cosmos-sdk/tree/master/cosmovisor/READM To get help for the configured binary: cosmovisor run help + +Available Commands: + help This help message + run Runs app passing all subsequent parameters + version Prints version of cosmovisor and the associated app. `, cosmovisor.EnvName, cosmovisor.EnvHome) } diff --git a/cosmovisor/cmd/cosmovisor/cmd/help_test.go b/cosmovisor/cmd/cosmovisor/cmd/help_test.go index 6366957019..ebcc252d57 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/help_test.go +++ b/cosmovisor/cmd/cosmovisor/cmd/help_test.go @@ -88,94 +88,6 @@ func (s *HelpTestSuite) setEnv(t *testing.T, env *cosmovisorHelpEnv) { } } -func (s *HelpTestSuite) TestShouldGiveHelpEnvVars() { - initialEnv := s.clearEnv() - defer s.setEnv(nil, initialEnv) - - emptyVal := "" - homeVal := "/somehome" - nameVal := "somename" - - tests := []struct { - name string - envHome *string - envName *string - expected bool - }{ - { - name: "home set name set", - envHome: &homeVal, - envName: &nameVal, - expected: false, - }, - { - name: "home not set name not set", - envHome: nil, - envName: nil, - expected: true, - }, - { - name: "home empty name not set", - envHome: &emptyVal, - envName: nil, - expected: true, - }, - { - name: "home set name not set", - envHome: &homeVal, - envName: nil, - expected: true, - }, - { - name: "home not set name empty", - envHome: nil, - envName: &emptyVal, - expected: true, - }, - { - name: "home empty name empty", - envHome: &emptyVal, - envName: &emptyVal, - expected: true, - }, - { - name: "home set name empty", - envHome: &homeVal, - envName: &emptyVal, - expected: true, - }, - { - name: "home not set name set", - envHome: nil, - envName: &nameVal, - expected: true, - }, - { - name: "home empty name set", - envHome: &emptyVal, - envName: &nameVal, - expected: true, - }, - } - - prepEnv := func(t *testing.T, envVar string, envVal *string) { - if envVal == nil { - require.NoError(t, os.Unsetenv(cosmovisor.EnvHome)) - } else { - require.NoError(t, os.Setenv(envVar, *envVal)) - } - } - - for _, tc := range tests { - s.T().Run(tc.name, func(t *testing.T) { - prepEnv(t, cosmovisor.EnvHome, tc.envHome) - prepEnv(t, cosmovisor.EnvName, tc.envName) - actual := ShouldGiveHelp("not-a-help-arg") - assert.Equal(t, tc.expected, actual) - }) - } -} - func (s HelpTestSuite) TestShouldGiveHelpArg() { initialEnv := s.clearEnv() defer s.setEnv(nil, initialEnv) diff --git a/cosmovisor/cmd/cosmovisor/cmd/root.go b/cosmovisor/cmd/cosmovisor/cmd/root.go index a4d3f2b2f1..2b1921ca56 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/root.go +++ b/cosmovisor/cmd/cosmovisor/cmd/root.go @@ -13,12 +13,11 @@ func RunCosmovisorCommand(args []string) error { arg0 = strings.TrimSpace(args[0]) } switch { + case IsVersionCommand(arg0): + return PrintVersion() case ShouldGiveHelp(arg0): DoHelp() return nil - case IsVersionCommand(arg0): - PrintVersion() - return Run([]string{"version"}) case IsRunCommand(arg0): return Run(args[1:]) } diff --git a/cosmovisor/cmd/cosmovisor/cmd/run.go b/cosmovisor/cmd/cosmovisor/cmd/run.go index 428aa53917..8912e1a977 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/run.go +++ b/cosmovisor/cmd/cosmovisor/cmd/run.go @@ -16,10 +16,9 @@ func IsRunCommand(arg string) bool { // Run runs the configured program with the given args and monitors it for upgrades. func Run(args []string) error { - cfg, cerr := cosmovisor.GetConfigFromEnv() - cosmovisor.LogConfigOrError(cosmovisor.Logger, cfg, cerr) - if cerr != nil { - return cerr + cfg, err := cosmovisor.GetConfigFromEnv() + if err != nil { + return err } launcher, err := cosmovisor.NewLauncher(cfg) if err != nil { diff --git a/cosmovisor/cmd/cosmovisor/cmd/version.go b/cosmovisor/cmd/cosmovisor/cmd/version.go index 11c6ee75e5..774c64b9b5 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/version.go +++ b/cosmovisor/cmd/cosmovisor/cmd/version.go @@ -2,6 +2,11 @@ package cmd import ( "fmt" + "os" + "time" + + cverrors "github.com/cosmos/cosmos-sdk/cosmovisor/errors" + "github.com/rs/zerolog" ) // Version represents Cosmovisor version value. Set during build @@ -16,6 +21,16 @@ func IsVersionCommand(arg string) bool { } // PrintVersion prints the cosmovisor version. -func PrintVersion() { +func PrintVersion() error { fmt.Println("Cosmovisor Version: ", Version) + + if err := Run([]string{"version"}); err != nil { + // Check the config and output details or any errors. + // Not using the cosmovisor.Logger in order to ignore any level it might have set, + // and also to not have any of the extra parameters in the output. + output := zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.Kitchen} + logger := zerolog.New(output).With().Timestamp().Logger() + cverrors.LogErrors(logger, "Can't run APP version", err) + } + return nil } diff --git a/cosmovisor/cmd/cosmovisor/main.go b/cosmovisor/cmd/cosmovisor/main.go index 4d82805be2..8cd8623c7e 100644 --- a/cosmovisor/cmd/cosmovisor/main.go +++ b/cosmovisor/cmd/cosmovisor/main.go @@ -5,12 +5,13 @@ import ( "github.com/cosmos/cosmos-sdk/cosmovisor" "github.com/cosmos/cosmos-sdk/cosmovisor/cmd/cosmovisor/cmd" + cverrors "github.com/cosmos/cosmos-sdk/cosmovisor/errors" ) func main() { cosmovisor.SetupLogging() if err := cmd.RunCosmovisorCommand(os.Args[1:]); err != nil { - cosmovisor.Logger.Error().Err(err).Msg("") + cverrors.LogErrors(cosmovisor.Logger, "", err) os.Exit(1) } } diff --git a/cosmovisor/errors/multi.go b/cosmovisor/errors/multi.go index 3b1d419ea2..bb7885a9bb 100644 --- a/cosmovisor/errors/multi.go +++ b/cosmovisor/errors/multi.go @@ -3,6 +3,8 @@ package errors import ( "fmt" "strings" + + "github.com/rs/zerolog" ) // MultiError is an error combining multiple other errors. @@ -67,3 +69,18 @@ func (e *MultiError) Error() string { func (e MultiError) String() string { return e.Error() } + +func LogErrors(logger zerolog.Logger, msg string, err error) { + switch err := err.(type) { + case *MultiError: + if msg != "" { + logger.Error().Msg(msg) + } + for i, e := range err.GetErrors() { + logger.Error().Err(e).Msg(fmt.Sprintf(" %d:", i+1)) + } + default: + logger.Error().Err(err).Msg(msg) + } + +}