From 2760499201ed1935178aa2e21e1f34bb3aca3c3b Mon Sep 17 00:00:00 2001 From: LianBo Date: Wed, 15 Feb 2023 17:51:45 +0800 Subject: [PATCH] Refactoring the option.Parse method (#930) * Fix some issues with install.sh * remove the rm command * Update scripts/install.sh Co-authored-by: Hao Chen * The option.Parse method should return failure or success without other logic --------- Co-authored-by: Hao Chen --- cmd/server/main.go | 13 ++++++++----- pkg/cluster/cluster_test.go | 4 ++-- pkg/cluster/member_test.go | 2 +- pkg/cluster/test_util.go | 2 +- pkg/option/option.go | 39 +++++++++++++++++++------------------ 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index 57a215b5a0..8d62b6ceee 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -42,15 +42,18 @@ func main() { rand.Seed(time.Now().UnixNano()) opt := option.New() - msg, err := opt.Parse() - if err != nil { + if err := opt.Parse(); err != nil { common.Exit(1, err.Error()) } - if msg != "" { - common.Exit(0, msg) + + if opt.ShowVersion { + common.Exit(0, version.Short) + } + if opt.ShowHelp { + common.Exit(0, opt.FlagUsages()) } - err = env.InitServerDir(opt) + err := env.InitServerDir(opt) if err != nil { log.Printf("failed to init env: %v", err) os.Exit(1) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 16b139fc9a..2f4fa19634 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -95,7 +95,7 @@ func mockStaticClusterMembers(count int) ([]*option.Options, membersSlice, []*pb opt.LogDir = "log" opt.MemberDir = "member" opt.Debug = false - _, err = opt.Parse() // create directories + err = opt.Parse() // create directories if err != nil { panic(fmt.Errorf("parse option failed: %v", err)) } @@ -184,7 +184,7 @@ func createSecondaryNode(clusterName string, primaryListenPeerURLs []string) *cl opt.Cluster.PrimaryListenPeerURLs = primaryListenPeerURLs opt.APIAddr = fmt.Sprintf("localhost:%d", ports[0]) - _, err = opt.Parse() + err = opt.Parse() check(err) env.InitServerDir(opt) diff --git a/pkg/cluster/member_test.go b/pkg/cluster/member_test.go index 0dbcf297a8..dc883a999d 100644 --- a/pkg/cluster/member_test.go +++ b/pkg/cluster/member_test.go @@ -53,7 +53,7 @@ func mockTestOpt(ports []int) *option.Options { opt.MemberDir = "member" opt.Debug = false - if _, err := opt.Parse(); err != nil { + if err := opt.Parse(); err != nil { panic(fmt.Errorf("parse option failed: %v", err)) } diff --git a/pkg/cluster/test_util.go b/pkg/cluster/test_util.go index f3e6b16ed5..a72c5d1222 100644 --- a/pkg/cluster/test_util.go +++ b/pkg/cluster/test_util.go @@ -53,7 +53,7 @@ func CreateOptionsForTest(tempDir string) *option.Options { opt.LogDir = fmt.Sprintf("%s/log", tempDir) opt.MemberDir = fmt.Sprintf("%s/member", tempDir) - _, err = opt.Parse() + err = opt.Parse() check(err) env.InitServerDir(opt) diff --git a/pkg/option/option.go b/pkg/option/option.go index 562356009c..3f7941f477 100644 --- a/pkg/option/option.go +++ b/pkg/option/option.go @@ -32,7 +32,6 @@ import ( "github.com/megaease/easegress/pkg/common" "github.com/megaease/easegress/pkg/util/codectool" - "github.com/megaease/easegress/pkg/version" ) // ClusterOptions defines the cluster members. @@ -159,7 +158,7 @@ func New() *Options { opt.flags.IntVar(&opt.StatusUpdateMaxBatchSize, "status-update-max-batch-size", 20, "Number of object statuses to update at maximum in one transaction.") - opt.viper.BindPFlags(opt.flags) + _ = opt.viper.BindPFlags(opt.flags) return opt } @@ -181,27 +180,24 @@ func (opt *Options) renameLegacyClusterRoles() { fmtLogger := fmt.Printf // Importing logger here is an import cycle, so use fmt instead. if opt.ClusterRole == "writer" { opt.ClusterRole = "primary" - fmtLogger(warning, "writer", "primary") + _, _ = fmtLogger(warning, "writer", "primary") } if opt.ClusterRole == "reader" { opt.ClusterRole = "secondary" - fmtLogger(warning, "reader", "secondary") + _, _ = fmtLogger(warning, "reader", "secondary") } } -// Parse parses all arguments, returns normal message without error if --help/--version set. -func (opt *Options) Parse() (string, error) { +// Parse parses all arguments, when the user wants to display version information or view help, +// we do not execute subsequent logic and return directly. +func (opt *Options) Parse() error { err := opt.flags.Parse(os.Args[1:]) if err != nil { - return "", err - } - - if opt.ShowVersion { - return version.Short, nil + return err } - if opt.ShowHelp { - return opt.flags.FlagUsages(), nil + if opt.ShowVersion || opt.ShowHelp { + return nil } opt.viper.AutomaticEnv() @@ -213,7 +209,7 @@ func (opt *Options) Parse() (string, error) { opt.viper.SetConfigType("yaml") err := opt.viper.ReadInConfig() if err != nil { - return "", fmt.Errorf("read config file %s failed: %v", + return fmt.Errorf("read config file %s failed: %v", opt.ConfigFile, err) } } @@ -234,7 +230,7 @@ func (opt *Options) Parse() (string, error) { c.TagName = "yaml" }) if err != nil { - return "", fmt.Errorf("yaml file unmarshal failed, please make sure you provide valid yaml file, %v", err) + return fmt.Errorf("yaml file unmarshal failed, please make sure you provide valid yaml file, %v", err) } opt.renameLegacyClusterRoles() @@ -248,17 +244,17 @@ func (opt *Options) Parse() (string, error) { err = opt.validate() if err != nil { - return "", err + return err } err = opt.prepare() if err != nil { - return "", err + return err } buff, err := codectool.MarshalYAML(opt) if err != nil { - return "", fmt.Errorf("marshal config to yaml failed: %v", err) + return fmt.Errorf("marshal config to yaml failed: %v", err) } opt.yamlStr = string(buff) @@ -266,7 +262,12 @@ func (opt *Options) Parse() (string, error) { fmt.Printf("%s", opt.yamlStr) } - return "", nil + return nil +} + +// FlagUsages export original flag usages, see FlagSet.FlagUsages. +func (opt *Options) FlagUsages() string { + return opt.flags.FlagUsages() } // ParseURLs parses list of strings to url.URL objects.