From 749256e9c26f21257668694e5b3503e482d3668b Mon Sep 17 00:00:00 2001 From: chymy Date: Wed, 20 Jul 2022 06:29:45 +0000 Subject: [PATCH] Improve tips of incorrect input of kubedm some subcommand Signed-off-by: chymy --- cmd/kubeadm/app/cmd/certs.go | 3 ++- cmd/kubeadm/app/cmd/config.go | 6 ++--- cmd/kubeadm/app/cmd/reset.go | 2 +- cmd/kubeadm/app/cmd/token.go | 8 +++---- cmd/kubeadm/app/cmd/token_test.go | 18 +++++++-------- cmd/kubeadm/app/cmd/upgrade/upgrade.go | 2 +- cmd/kubeadm/app/cmd/util/cmdutil.go | 24 +++++++++++--------- cmd/kubeadm/app/util/error.go | 31 +++++++++++++++++++------- 8 files changed, 57 insertions(+), 37 deletions(-) diff --git a/cmd/kubeadm/app/cmd/certs.go b/cmd/kubeadm/app/cmd/certs.go index 94b74760726..084d472bab5 100644 --- a/cmd/kubeadm/app/cmd/certs.go +++ b/cmd/kubeadm/app/cmd/certs.go @@ -96,6 +96,7 @@ func newCmdCertsUtility(out io.Writer) *cobra.Command { Use: "certs", Aliases: []string{"certificates"}, Short: "Commands related to handling kubernetes certificates", + Run: cmdutil.SubCmdRun(), } cmd.AddCommand(newCmdCertsRenewal(out)) @@ -203,7 +204,7 @@ func newCmdCertsRenewal(out io.Writer) *cobra.Command { Use: "renew", Short: "Renew certificates for a Kubernetes cluster", Long: cmdutil.MacroCommandLongDescription, - RunE: cmdutil.SubCmdRunE("renew"), + Run: cmdutil.SubCmdRun(), } cmd.AddCommand(getRenewSubCommands(out, kubeadmconstants.KubernetesDir)...) diff --git a/cmd/kubeadm/app/cmd/config.go b/cmd/kubeadm/app/cmd/config.go index 1edb54298dc..c02f5806d45 100644 --- a/cmd/kubeadm/app/cmd/config.go +++ b/cmd/kubeadm/app/cmd/config.go @@ -68,7 +68,7 @@ func newCmdConfig(out io.Writer) *cobra.Command { // cobra will print usage information, but still exit cleanly. // We want to return an error code in these cases so that the // user knows that their command was invalid. - RunE: cmdutil.SubCmdRunE("config"), + Run: cmdutil.SubCmdRun(), } options.AddKubeConfigFlag(cmd.PersistentFlags(), &kubeConfigFile) @@ -88,7 +88,7 @@ func newCmdConfigPrint(out io.Writer) *cobra.Command { Long: dedent.Dedent(` This command prints configurations for subcommands provided. For details, see: https://pkg.go.dev/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm#section-directories`), - RunE: cmdutil.SubCmdRunE("print"), + Run: cmdutil.SubCmdRun(), } cmd.AddCommand(newCmdConfigPrintInitDefaults(out)) cmd.AddCommand(newCmdConfigPrintJoinDefaults(out)) @@ -277,7 +277,7 @@ func newCmdConfigImages(out io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "images", Short: "Interact with container images used by kubeadm", - RunE: cmdutil.SubCmdRunE("images"), + Run: cmdutil.SubCmdRun(), } cmd.AddCommand(newCmdConfigImagesList(out, nil)) cmd.AddCommand(newCmdConfigImagesPull()) diff --git a/cmd/kubeadm/app/cmd/reset.go b/cmd/kubeadm/app/cmd/reset.go index 6ee9d02794f..faf5fd67f16 100644 --- a/cmd/kubeadm/app/cmd/reset.go +++ b/cmd/kubeadm/app/cmd/reset.go @@ -95,7 +95,7 @@ func newResetOptions() *resetOptions { func newResetData(cmd *cobra.Command, options *resetOptions, in io.Reader, out io.Writer) (*resetData, error) { var cfg *kubeadmapi.InitConfiguration - client, err := cmdutil.GetClientset(options.kubeconfigPath, false) + client, err := cmdutil.GetClientSet(options.kubeconfigPath, false) if err == nil { klog.V(1).Infof("[reset] Loaded client set from kubeconfig file: %s", options.kubeconfigPath) cfg, err = configutil.FetchInitConfigurationFromCluster(client, nil, "reset", false, false) diff --git a/cmd/kubeadm/app/cmd/token.go b/cmd/kubeadm/app/cmd/token.go index 30e47ab519d..b53b2b6ae86 100644 --- a/cmd/kubeadm/app/cmd/token.go +++ b/cmd/kubeadm/app/cmd/token.go @@ -83,7 +83,7 @@ func newCmdToken(out io.Writer, errW io.Writer) *cobra.Command { // cobra will print usage information, but still exit cleanly. // We want to return an error code in these cases so that the // user knows that their command was invalid. - RunE: cmdutil.SubCmdRunE("token"), + Run: cmdutil.SubCmdRun(), } options.AddKubeConfigFlag(tokenCmd.PersistentFlags(), &kubeConfigFile) @@ -127,7 +127,7 @@ func newCmdToken(out io.Writer, errW io.Writer) *cobra.Command { klog.V(1).Infoln("[token] getting Clientsets from kubeconfig file") kubeConfigFile = cmdutil.GetKubeConfigPath(kubeConfigFile) - client, err := cmdutil.GetClientset(kubeConfigFile, dryRun) + client, err := cmdutil.GetClientSet(kubeConfigFile, dryRun) if err != nil { return err } @@ -159,7 +159,7 @@ func newCmdToken(out io.Writer, errW io.Writer) *cobra.Command { `), RunE: func(tokenCmd *cobra.Command, args []string) error { kubeConfigFile = cmdutil.GetKubeConfigPath(kubeConfigFile) - client, err := cmdutil.GetClientset(kubeConfigFile, dryRun) + client, err := cmdutil.GetClientSet(kubeConfigFile, dryRun) if err != nil { return err } @@ -193,7 +193,7 @@ func newCmdToken(out io.Writer, errW io.Writer) *cobra.Command { return errors.Errorf("missing subcommand; 'token delete' is missing token of form %q", bootstrapapi.BootstrapTokenIDPattern) } kubeConfigFile = cmdutil.GetKubeConfigPath(kubeConfigFile) - client, err := cmdutil.GetClientset(kubeConfigFile, dryRun) + client, err := cmdutil.GetClientSet(kubeConfigFile, dryRun) if err != nil { return err } diff --git a/cmd/kubeadm/app/cmd/token_test.go b/cmd/kubeadm/app/cmd/token_test.go index c7a2cab5879..ebbba192c57 100644 --- a/cmd/kubeadm/app/cmd/token_test.go +++ b/cmd/kubeadm/app/cmd/token_test.go @@ -265,7 +265,7 @@ func TestNewCmdToken(t *testing.T) { } } -func TestGetClientset(t *testing.T) { +func TestGetClientSet(t *testing.T) { testConfigTokenFile := "test-config-file" tmpDir, err := os.MkdirTemp("", "kubeadm-token-test") @@ -276,13 +276,13 @@ func TestGetClientset(t *testing.T) { fullPath := filepath.Join(tmpDir, testConfigTokenFile) // test dryRun = false on a non-exisiting file - if _, err = cmdutil.GetClientset(fullPath, false); err == nil { - t.Errorf("getClientset(); dry-run: false; did no fail for test file %q: %v", fullPath, err) + if _, err = cmdutil.GetClientSet(fullPath, false); err == nil { + t.Errorf("GetClientSet(); dry-run: false; did no fail for test file %q: %v", fullPath, err) } // test dryRun = true on a non-exisiting file - if _, err = cmdutil.GetClientset(fullPath, true); err == nil { - t.Errorf("getClientset(); dry-run: true; did no fail for test file %q: %v", fullPath, err) + if _, err = cmdutil.GetClientSet(fullPath, true); err == nil { + t.Errorf("GetClientSet(); dry-run: true; did no fail for test file %q: %v", fullPath, err) } f, err := os.Create(fullPath) @@ -296,8 +296,8 @@ func TestGetClientset(t *testing.T) { } // test dryRun = true on an exisiting file - if _, err = cmdutil.GetClientset(fullPath, true); err != nil { - t.Errorf("getClientset(); dry-run: true; failed for test file %q: %v", fullPath, err) + if _, err = cmdutil.GetClientSet(fullPath, true); err != nil { + t.Errorf("GetClientSet(); dry-run: true; failed for test file %q: %v", fullPath, err) } } @@ -321,9 +321,9 @@ func TestRunDeleteTokens(t *testing.T) { t.Errorf("Unable to write test file %q: %v", fullPath, err) } - client, err := cmdutil.GetClientset(fullPath, true) + client, err := cmdutil.GetClientSet(fullPath, true) if err != nil { - t.Errorf("Unable to run getClientset() for test file %q: %v", fullPath, err) + t.Errorf("Unable to run GetClientSet() for test file %q: %v", fullPath, err) } // test valid; should not fail diff --git a/cmd/kubeadm/app/cmd/upgrade/upgrade.go b/cmd/kubeadm/app/cmd/upgrade/upgrade.go index 32499f1d4ac..902e2cbccb9 100644 --- a/cmd/kubeadm/app/cmd/upgrade/upgrade.go +++ b/cmd/kubeadm/app/cmd/upgrade/upgrade.go @@ -54,7 +54,7 @@ func NewCmdUpgrade(out io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "upgrade", Short: "Upgrade your cluster smoothly to a newer version with this command", - RunE: cmdutil.SubCmdRunE("upgrade"), + Run: cmdutil.SubCmdRun(), } cmd.AddCommand(newCmdApply(flags)) diff --git a/cmd/kubeadm/app/cmd/util/cmdutil.go b/cmd/kubeadm/app/cmd/util/cmdutil.go index 26c2f55be72..a76faf3bd1e 100644 --- a/cmd/kubeadm/app/cmd/util/cmdutil.go +++ b/cmd/kubeadm/app/cmd/util/cmdutil.go @@ -34,25 +34,29 @@ import ( kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig" ) -// SubCmdRunE returns a function that handles a case where a subcommand must be specified +// SubCmdRun returns a function that handles a case where a subcommand must be specified // Without this callback, if a user runs just the command without a subcommand, // or with an invalid subcommand, cobra will print usage information, but still exit cleanly. -// We want to return an error code in these cases so that the -// user knows that their command was invalid. -func SubCmdRunE(name string) func(*cobra.Command, []string) error { - return func(_ *cobra.Command, args []string) error { - if len(args) < 1 { - return errors.Errorf("missing subcommand; %q is not meant to be run on its own", name) +func SubCmdRun() func(c *cobra.Command, args []string) { + return func(c *cobra.Command, args []string) { + if len(args) > 0 { + kubeadmutil.CheckErr(usageErrorf(c, "invalid subcommand %q", strings.Join(args, " "))) } - - return errors.Errorf("invalid subcommand: %q", args[0]) + c.Help() + kubeadmutil.CheckErr(kubeadmutil.ErrExit) } } +func usageErrorf(c *cobra.Command, format string, args ...interface{}) error { + msg := fmt.Sprintf(format, args...) + return errors.Errorf("%s\nSee '%s -h' for help and examples", msg, c.CommandPath()) +} + // ValidateExactArgNumber validates that the required top-level arguments are specified func ValidateExactArgNumber(args []string, supportedArgs []string) error { lenSupported := len(supportedArgs) @@ -127,7 +131,7 @@ func InteractivelyConfirmAction(action, question string, r io.Reader) error { } // GetClientSet gets a real or fake client depending on whether the user is dry-running or not -func GetClientset(file string, dryRun bool) (clientset.Interface, error) { +func GetClientSet(file string, dryRun bool) (clientset.Interface, error) { if dryRun { dryRunGetter, err := apiclient.NewClientBackedDryRunGetterFromKubeconfig(file) if err != nil { diff --git a/cmd/kubeadm/app/util/error.go b/cmd/kubeadm/app/util/error.go index 131fbf01929..b67b395da41 100644 --- a/cmd/kubeadm/app/util/error.go +++ b/cmd/kubeadm/app/util/error.go @@ -23,6 +23,8 @@ import ( "strconv" "strings" + "github.com/pkg/errors" + errorsutil "k8s.io/apimachinery/pkg/util/errors" ) @@ -35,6 +37,11 @@ const ( ValidationExitCode = 3 ) +var ( + ErrInvalidSubCommandMsg = "invalid subcommand" + ErrExit = errors.New("exit") +) + // fatal prints the message if set and then exits. func fatal(msg string, code int) { if len(msg) > 0 { @@ -85,16 +92,24 @@ func checkErr(err error, handleErr func(string, int)) { } } - switch err.(type) { - case nil: + if err == nil { return - case preflightError: - handleErr(msg, PreFlightExitCode) - case errorsutil.Aggregate: - handleErr(msg, ValidationExitCode) - + } + switch { + case err == ErrExit: + handleErr("", DefaultErrorExitCode) + case strings.Contains(err.Error(), ErrInvalidSubCommandMsg): + handleErr(err.Error(), DefaultErrorExitCode) default: - handleErr(msg, DefaultErrorExitCode) + switch err.(type) { + case preflightError: + handleErr(msg, PreFlightExitCode) + case errorsutil.Aggregate: + handleErr(msg, ValidationExitCode) + + default: + handleErr(msg, DefaultErrorExitCode) + } } }