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 <[email protected]>

* The option.Parse method should return failure or success without other logic

---------

Co-authored-by: Hao Chen <[email protected]>

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)

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)

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))
  }
 

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)

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,25 +244,30 @@ 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)
 
  if opt.ShowConfig {
  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.