From 9e7f29c3fbe3798a04c50a788a1739eb0a460237 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 18 May 2017 15:34:04 -0700 Subject: [PATCH] cmd: fix deprecation warning bug Some kubectl commands were deprecated but would fail to print the correct warning message when a flag was given before the command name. # Correctly prints the warning that "resize" is deprecated and # "scale" is now preferred. kubectl scale [...] # Should print the same warning but no warning is printed. kubectl --v=1 scale [...] This was due to a fragile check on os.Args[1]. This commit implements a new function deprecatedCmd() that is used to construct new "passthrough" commands which are marked as deprecated and hidden. Note that there is an existing "filters" system that may be preferable to the system created in this commit. I'm not sure why the "filters" array was not used for all deprecated commands in the first place. --- pkg/kubectl/cmd/apiversions.go | 9 +-------- pkg/kubectl/cmd/clusterinfo.go | 11 ++--------- pkg/kubectl/cmd/cmd.go | 19 +++++++++++++++++++ pkg/kubectl/cmd/replace.go | 7 +------ pkg/kubectl/cmd/rollingupdate.go | 8 +------- pkg/kubectl/cmd/run.go | 9 +-------- pkg/kubectl/cmd/scale.go | 9 +-------- 7 files changed, 26 insertions(+), 46 deletions(-) diff --git a/pkg/kubectl/cmd/apiversions.go b/pkg/kubectl/cmd/apiversions.go index a255c3789fb..885dece34ba 100644 --- a/pkg/kubectl/cmd/apiversions.go +++ b/pkg/kubectl/cmd/apiversions.go @@ -19,7 +19,6 @@ package cmd import ( "fmt" "io" - "os" "sort" "github.com/spf13/cobra" @@ -38,9 +37,7 @@ var ( func NewCmdApiVersions(f cmdutil.Factory, out io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "api-versions", - // apiversions is deprecated. - Aliases: []string{"apiversions"}, + Use: "api-versions", Short: "Print the supported API versions on the server, in the form of \"group/version\"", Long: "Print the supported API versions on the server, in the form of \"group/version\"", Example: apiversionsExample, @@ -53,10 +50,6 @@ func NewCmdApiVersions(f cmdutil.Factory, out io.Writer) *cobra.Command { } func RunApiVersions(f cmdutil.Factory, w io.Writer) error { - if len(os.Args) > 1 && os.Args[1] == "apiversions" { - printDeprecationWarning("api-versions", "apiversions") - } - discoveryclient, err := f.DiscoveryClient() if err != nil { return err diff --git a/pkg/kubectl/cmd/clusterinfo.go b/pkg/kubectl/cmd/clusterinfo.go index d28d7e15930..e03c34708c7 100644 --- a/pkg/kubectl/cmd/clusterinfo.go +++ b/pkg/kubectl/cmd/clusterinfo.go @@ -19,7 +19,6 @@ package cmd import ( "fmt" "io" - "os" "strconv" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,7 +28,7 @@ import ( "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/util/i18n" - "github.com/daviddengcn/go-colortext" + ct "github.com/daviddengcn/go-colortext" "github.com/spf13/cobra" ) @@ -45,9 +44,7 @@ var ( func NewCmdClusterInfo(f cmdutil.Factory, out io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "cluster-info", - // clusterinfo is deprecated. - Aliases: []string{"clusterinfo"}, + Use: "cluster-info", Short: i18n.T("Display cluster info"), Long: longDescr, Example: clusterinfoExample, @@ -62,10 +59,6 @@ func NewCmdClusterInfo(f cmdutil.Factory, out io.Writer) *cobra.Command { } func RunClusterInfo(f cmdutil.Factory, out io.Writer, cmd *cobra.Command) error { - if len(os.Args) > 1 && os.Args[1] == "clusterinfo" { - printDeprecationWarning("cluster-info", "clusterinfo") - } - client, err := f.ClientConfig() if err != nil { return err diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go index 21087c8a456..67f5ab5d342 100644 --- a/pkg/kubectl/cmd/cmd.go +++ b/pkg/kubectl/cmd/cmd.go @@ -284,6 +284,7 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob NewCmdCreate(f, out, err), NewCmdExposeService(f, out), NewCmdRun(f, in, out, err), + deprecatedCmd("run-container", NewCmdRun(f, in, out, err)), set.NewCmdSet(f, out, err), }, }, @@ -301,7 +302,9 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob Commands: []*cobra.Command{ rollout.NewCmdRollout(f, out, err), NewCmdRollingUpdate(f, out), + deprecatedCmd("rollingupdate", NewCmdRollingUpdate(f, out)), NewCmdScale(f, out), + deprecatedCmd("resize", NewCmdScale(f, out)), NewCmdAutoscale(f, out), }, }, @@ -310,6 +313,7 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob Commands: []*cobra.Command{ NewCmdCertificate(f, out), NewCmdClusterInfo(f, out), + deprecatedCmd("clusterinfo", NewCmdClusterInfo(f, out)), NewCmdTop(f, out, err), NewCmdCordon(f, out), NewCmdUncordon(f, out), @@ -336,6 +340,7 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob NewCmdApply(f, out, err), NewCmdPatch(f, out), NewCmdReplace(f, out), + deprecatedCmd("update", NewCmdReplace(f, out)), NewCmdConvert(f, out), }, }, @@ -372,6 +377,7 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob cmds.AddCommand(NewCmdPlugin(f, in, out, err)) cmds.AddCommand(NewCmdVersion(f, out)) cmds.AddCommand(NewCmdApiVersions(f, out)) + cmds.AddCommand(deprecatedCmd("apiversions", NewCmdApiVersions(f, out))) cmds.AddCommand(NewCmdOptions()) return cmds @@ -385,6 +391,19 @@ func printDeprecationWarning(command, alias string) { glog.Warningf("%s is DEPRECATED and will be removed in a future version. Use %s instead.", alias, command) } +// Create a constructor for a command that redirects to another command, but +// first prints a deprecation message. +func deprecatedCmd(deprecatedVersion string, cmd *cobra.Command) *cobra.Command { + // Have to be careful here because Cobra automatically extracts the name + // of the command from the .Use field. + originalName := cmd.Name() + + cmd.Use = deprecatedVersion + cmd.Deprecated = fmt.Sprintf("use %q instead", originalName) + cmd.Hidden = true + return cmd +} + func Deprecated(baseName, to string, parent, cmd *cobra.Command) string { cmd.Long = fmt.Sprintf("Deprecated: This command is deprecated, all its functionalities are covered by \"%s %s\"", baseName, to) cmd.Short = fmt.Sprintf("Deprecated: %s", to) diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index b46968d9d7f..47be6f3f12b 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/pkg/kubectl/cmd/replace.go @@ -65,9 +65,7 @@ func NewCmdReplace(f cmdutil.Factory, out io.Writer) *cobra.Command { options := &resource.FilenameOptions{} cmd := &cobra.Command{ - Use: "replace -f FILENAME", - // update is deprecated. - Aliases: []string{"update"}, + Use: "replace -f FILENAME", Short: i18n.T("Replace a resource by filename or stdin"), Long: replaceLong, Example: replaceExample, @@ -94,9 +92,6 @@ func NewCmdReplace(f cmdutil.Factory, out io.Writer) *cobra.Command { } func RunReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, options *resource.FilenameOptions) error { - if len(os.Args) > 1 && os.Args[1] == "update" { - printDeprecationWarning("replace", "update") - } schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir")) if err != nil { return err diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index 4ec53cbdeef..230430e5689 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -20,7 +20,6 @@ import ( "bytes" "fmt" "io" - "os" "time" "github.com/golang/glog" @@ -79,9 +78,7 @@ func NewCmdRollingUpdate(f cmdutil.Factory, out io.Writer) *cobra.Command { options := &resource.FilenameOptions{} cmd := &cobra.Command{ - Use: "rolling-update OLD_CONTROLLER_NAME ([NEW_CONTROLLER_NAME] --image=NEW_CONTAINER_IMAGE | -f NEW_CONTROLLER_SPEC)", - // rollingupdate is deprecated. - Aliases: []string{"rollingupdate"}, + Use: "rolling-update OLD_CONTROLLER_NAME ([NEW_CONTROLLER_NAME] --image=NEW_CONTAINER_IMAGE | -f NEW_CONTROLLER_SPEC)", Short: i18n.T("Perform a rolling update of the given ReplicationController"), Long: rollingUpdateLong, Example: rollingUpdateExample, @@ -143,9 +140,6 @@ func validateArguments(cmd *cobra.Command, filenames, args []string) error { } func RunRollingUpdate(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, options *resource.FilenameOptions) error { - if len(os.Args) > 1 && os.Args[1] == "rollingupdate" { - printDeprecationWarning("rolling-update", "rollingupdate") - } err := validateArguments(cmd, options.Filenames, args) if err != nil { return err diff --git a/pkg/kubectl/cmd/run.go b/pkg/kubectl/cmd/run.go index 691d5376adc..831fea073dd 100644 --- a/pkg/kubectl/cmd/run.go +++ b/pkg/kubectl/cmd/run.go @@ -19,7 +19,6 @@ package cmd import ( "fmt" "io" - "os" "github.com/docker/distribution/reference" "github.com/spf13/cobra" @@ -90,9 +89,7 @@ var ( func NewCmdRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "run NAME --image=image [--env=\"key=value\"] [--port=port] [--replicas=replicas] [--dry-run=bool] [--overrides=inline-json] [--command] -- [COMMAND] [args...]", - // run-container is deprecated - Aliases: []string{"run-container"}, + Use: "run NAME --image=image [--env=\"key=value\"] [--port=port] [--replicas=replicas] [--dry-run=bool] [--overrides=inline-json] [--command] -- [COMMAND] [args...]", Short: i18n.T("Run a particular image on the cluster"), Long: runLong, Example: runExample, @@ -140,10 +137,6 @@ func addRunFlags(cmd *cobra.Command) { } func Run(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cobra.Command, args []string, argsLenAtDash int) error { - if len(os.Args) > 1 && os.Args[1] == "run-container" { - printDeprecationWarning("run", "run-container") - } - // Let kubectl run follow rules for `--`, see #13004 issue if len(args) == 0 || argsLenAtDash == 0 { return cmdutil.UsageError(cmd, "NAME is required for run") diff --git a/pkg/kubectl/cmd/scale.go b/pkg/kubectl/cmd/scale.go index 2be894328cd..20aae2fe8e0 100644 --- a/pkg/kubectl/cmd/scale.go +++ b/pkg/kubectl/cmd/scale.go @@ -19,7 +19,6 @@ package cmd import ( "fmt" "io" - "os" "github.com/spf13/cobra" @@ -65,9 +64,7 @@ func NewCmdScale(f cmdutil.Factory, out io.Writer) *cobra.Command { argAliases := kubectl.ResourceAliases(validArgs) cmd := &cobra.Command{ - Use: "scale [--resource-version=version] [--current-replicas=count] --replicas=COUNT (-f FILENAME | TYPE NAME)", - // resize is deprecated - Aliases: []string{"resize"}, + Use: "scale [--resource-version=version] [--current-replicas=count] --replicas=COUNT (-f FILENAME | TYPE NAME)", Short: i18n.T("Set a new size for a Deployment, ReplicaSet, Replication Controller, or Job"), Long: scaleLong, Example: scaleExample, @@ -96,10 +93,6 @@ func NewCmdScale(f cmdutil.Factory, out io.Writer) *cobra.Command { // RunScale executes the scaling func RunScale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, shortOutput bool, options *resource.FilenameOptions) error { - if len(os.Args) > 1 && os.Args[1] == "resize" { - printDeprecationWarning("scale", "resize") - } - cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err