Merge pull request #111277 from chymy/improve-kubeadm-subcommand-tips

kubeadm: improve tips of incorrect input of kubedm subcommand

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

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

cmd/kubeadm/app/cmd/reset.go

@@ -98,7 +98,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)

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
 			}

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

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

cmd/kubeadm/app/cmd/util/cmdutil.go

@@ -34,23 +34,27 @@ 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, " ")))
 		}
+		c.Help()
+		kubeadmutil.CheckErr(kubeadmutil.ErrExit)
+	}
+}
 
-		return errors.Errorf("invalid subcommand: %q", args[0])
-	}
+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
@@ -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 {

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,9 +92,16 @@ func checkErr(err error, handleErr func(string, int)) {
 		}
 	}
 
-	switch err.(type) {
-	case nil:
+	if err == nil {
 		return
+	}
+	switch {
+	case err == ErrExit:
+		handleErr("", DefaultErrorExitCode)
+	case strings.Contains(err.Error(), ErrInvalidSubCommandMsg):
+		handleErr(err.Error(), DefaultErrorExitCode)
+	default:
+		switch err.(type) {
 		case preflightError:
 			handleErr(msg, PreFlightExitCode)
 		case errorsutil.Aggregate:
@@ -96,6 +110,7 @@ func checkErr(err error, handleErr func(string, int)) {
 		default:
 			handleErr(msg, DefaultErrorExitCode)
 		}
+	}
 }
 
 // FormatErrMsg returns a human-readable string describing the slice of errors passed to the function