From 1bd15658f8b808625032d29363bef6e8430d6773 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?= Date: Tue, 29 Jan 2019 20:59:06 +0100 Subject: [PATCH] kubeadm: cleanup of phases arguments * Return `nil` instead of a pointer to an empty struct when possible, before the pointer was introduced the empty struct was required. * Explicitly accept only one argument maximum for `kubeadm join` as in `kubeadm join `. * Accept no arguments for `kubeadm init`. * Make phases leafs accept arguments, whereas intermediate phases automatically gets set that they accept no arguments. --- cmd/kubeadm/app/cmd/init.go | 19 ++++++++++--------- cmd/kubeadm/app/cmd/join.go | 10 ++++++---- cmd/kubeadm/app/cmd/phases/workflow/runner.go | 7 +++++-- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index 0a0f941374..4d12ef2a79 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -148,6 +148,7 @@ func NewCmdInit(out io.Writer, initOptions *initOptions) *cobra.Command { err = showJoinCommand(data, out) kubeadmutil.CheckErr(err) }, + Args: cobra.NoArgs, } // adds flags to the init command @@ -286,27 +287,27 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io // validated values to the public kubeadm config API when applicable var err error if options.externalcfg.FeatureGates, err = features.NewFeatureGate(&features.InitFeatureGates, options.featureGatesString); err != nil { - return &initData{}, err + return nil, err } ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(options.ignorePreflightErrors) if err != nil { - return &initData{}, err + return nil, err } if err = validation.ValidateMixedArguments(cmd.Flags()); err != nil { - return &initData{}, err + return nil, err } if err = options.bto.ApplyTo(options.externalcfg); err != nil { - return &initData{}, err + return nil, err } // Either use the config file if specified, or convert public kubeadm API to the internal InitConfiguration // and validates InitConfiguration cfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(options.cfgPath, options.externalcfg) if err != nil { - return &initData{}, err + return nil, err } // override node name and CRI socket from the command line options @@ -318,17 +319,17 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io } if err := configutil.VerifyAPIServerBindAddress(cfg.LocalAPIEndpoint.AdvertiseAddress); err != nil { - return &initData{}, err + return nil, err } if err := features.ValidateVersion(features.InitFeatureGates, cfg.FeatureGates, cfg.KubernetesVersion); err != nil { - return &initData{}, err + return nil, err } // if dry running creates a temporary folder for saving kubeadm generated files dryRunDir := "" if options.dryRun { if dryRunDir, err = ioutil.TempDir("", "kubeadm-init-dryrun"); err != nil { - return &initData{}, errors.Wrap(err, "couldn't create a temporary directory") + return nil, errors.Wrap(err, "couldn't create a temporary directory") } } @@ -340,7 +341,7 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io kubeconfigDir = dryRunDir } if err := kubeconfigphase.ValidateKubeconfigsForExternalCA(kubeconfigDir, cfg); err != nil { - return &initData{}, err + return nil, err } } diff --git a/cmd/kubeadm/app/cmd/join.go b/cmd/kubeadm/app/cmd/join.go index 2706484f14..ff5f91a438 100644 --- a/cmd/kubeadm/app/cmd/join.go +++ b/cmd/kubeadm/app/cmd/join.go @@ -203,6 +203,8 @@ func NewCmdJoin(out io.Writer, joinOptions *joinOptions) *cobra.Command { err = data.Run() kubeadmutil.CheckErr(err) }, + // We accept the master location as an optional positional argument + Args: cobra.MaximumNArgs(1), } AddJoinConfigFlags(cmd.Flags(), joinOptions.externalcfg) @@ -358,11 +360,11 @@ func newJoinData(cmd *cobra.Command, args []string, options *joinOptions, out io ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(options.ignorePreflightErrors) if err != nil { - return &joinData{}, err + return nil, err } if err = validation.ValidateMixedArguments(cmd.Flags()); err != nil { - return &joinData{}, err + return nil, err } // Either use the config file if specified, or convert public kubeadm API to the internal JoinConfiguration @@ -377,7 +379,7 @@ func newJoinData(cmd *cobra.Command, args []string, options *joinOptions, out io cfg, err := configutil.JoinConfigFileAndDefaultsToInternalConfig(options.cfgPath, options.externalcfg) if err != nil { - return &joinData{}, err + return nil, err } // override node name and CRI socket from the command line options @@ -390,7 +392,7 @@ func newJoinData(cmd *cobra.Command, args []string, options *joinOptions, out io if cfg.ControlPlane != nil { if err := configutil.VerifyAPIServerBindAddress(cfg.ControlPlane.LocalAPIEndpoint.AdvertiseAddress); err != nil { - return &joinData{}, err + return nil, err } } diff --git a/cmd/kubeadm/app/cmd/phases/workflow/runner.go b/cmd/kubeadm/app/cmd/phases/workflow/runner.go index cd4026d5d0..183c50212b 100644 --- a/cmd/kubeadm/app/cmd/phases/workflow/runner.go +++ b/cmd/kubeadm/app/cmd/phases/workflow/runner.go @@ -309,7 +309,6 @@ func (e *Runner) BindToCommand(cmd *cobra.Command) { phaseCommand := &cobra.Command{ Use: "phase", Short: fmt.Sprintf("use this command to invoke single phase of the %s workflow", cmd.Name()), - // TODO: this logic is currently lacking verification if a suphase name is valid! } cmd.AddCommand(phaseCommand) @@ -352,7 +351,6 @@ func (e *Runner) BindToCommand(cmd *cobra.Command) { os.Exit(1) } }, - Args: cobra.NoArgs, // this forces cobra to fail if a wrong phase name is passed } // makes the new command inherits local flags from the parent command @@ -371,6 +369,11 @@ func (e *Runner) BindToCommand(cmd *cobra.Command) { }) } + // if this phase has children (not a leaf) it doesn't accept any args + if len(p.Phases) > 0 { + phaseCmd.Args = cobra.NoArgs + } + // adds the command to parent if p.level == 0 { phaseCommand.AddCommand(phaseCmd)