From 9e7f29c3fbe3798a04c50a788a1739eb0a460237 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 18 May 2017 15:34:04 -0700 Subject: [PATCH 01/10] 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 From b3fc6556e4d59e00d360f7fd8035e606a032603b Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Fri, 19 May 2017 07:42:15 -0700 Subject: [PATCH 02/10] kubectl: improve docstring on deprecatedCmd --- pkg/kubectl/cmd/cmd.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go index 67f5ab5d342..4d41dc7af49 100644 --- a/pkg/kubectl/cmd/cmd.go +++ b/pkg/kubectl/cmd/cmd.go @@ -391,8 +391,9 @@ 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. +// deprecatedCmd is intended to be used to create a "wrapper" command around an +// existing command. The wrapper works the same but prints a deprecation +// message before running. 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. From 2acc5f18b5644f457ad5f4663ff00aca85266a9f Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Fri, 19 May 2017 08:49:43 -0700 Subject: [PATCH 03/10] test-cmd-util.sh: add test for deprecated commands --- hack/make-rules/test-cmd-util.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 234d9aa02fa..2febf034f95 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -1077,6 +1077,25 @@ run_save_config_tests() { kubectl delete rc frontend "${kube_flags[@]}" } +run_kubectl_using_deprecated_commands_test() { + ## `kubectl run-container` should function identical to `kubectl run`, but it + ## should also print a deprecation warning. + # Pre-Condition: no Job exists + kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" '' + # Command + output_message=$(kubectl run-container pi --generator=job/v1 "--image=$IMAGE_PERL" --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(20)' "${kube_flags[@]}") + # Ensure that the user is warned their command is deprecated. + kube::test::if_has_string "${output_message}" 'Deprecated' + # Post-Condition: Job "pi" is created + kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" 'pi:' + # Clean up + kubectl delete jobs pi "${kube_flags[@]}" + # Post-condition: no pods exist. + kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' + # Clean up + kubectl delete deployment nginx-apps "${kube_flags[@]}" +} + run_kubectl_run_tests() { ## kubectl run should create deployments or jobs # Pre-Condition: no Job exists @@ -3140,6 +3159,7 @@ runTests() { # run for federation apiserver as well. run_kubectl_apply_tests run_kubectl_run_tests + run_kubectl_using_deprecated_commands_test run_kubectl_create_filter_tests fi From 699a3e20ff3f567090ea8c5d209b6e5e7b8d72de Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Fri, 19 May 2017 08:56:53 -0700 Subject: [PATCH 04/10] .generated_docs: update to include "new" commands --- docs/.generated_docs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/.generated_docs b/docs/.generated_docs index 6068315a6e9..a288f3c4082 100644 --- a/docs/.generated_docs +++ b/docs/.generated_docs @@ -12,6 +12,7 @@ docs/man/man1/kube-proxy.1 docs/man/man1/kube-scheduler.1 docs/man/man1/kubectl-annotate.1 docs/man/man1/kubectl-api-versions.1 +docs/man/man1/kubectl-apiversions.1 docs/man/man1/kubectl-apply-set-last-applied.1 docs/man/man1/kubectl-apply-view-last-applied.1 docs/man/man1/kubectl-apply.1 @@ -24,6 +25,8 @@ docs/man/man1/kubectl-certificate-deny.1 docs/man/man1/kubectl-certificate.1 docs/man/man1/kubectl-cluster-info-dump.1 docs/man/man1/kubectl-cluster-info.1 +docs/man/man1/kubectl-clusterinfo-dump.1 +docs/man/man1/kubectl-clusterinfo.1 docs/man/man1/kubectl-completion.1 docs/man/man1/kubectl-config-current-context.1 docs/man/man1/kubectl-config-delete-cluster.1 @@ -77,13 +80,16 @@ docs/man/man1/kubectl-plugin.1 docs/man/man1/kubectl-port-forward.1 docs/man/man1/kubectl-proxy.1 docs/man/man1/kubectl-replace.1 +docs/man/man1/kubectl-resize.1 docs/man/man1/kubectl-rolling-update.1 +docs/man/man1/kubectl-rollingupdate.1 docs/man/man1/kubectl-rollout-history.1 docs/man/man1/kubectl-rollout-pause.1 docs/man/man1/kubectl-rollout-resume.1 docs/man/man1/kubectl-rollout-status.1 docs/man/man1/kubectl-rollout-undo.1 docs/man/man1/kubectl-rollout.1 +docs/man/man1/kubectl-run-container.1 docs/man/man1/kubectl-run.1 docs/man/man1/kubectl-scale.1 docs/man/man1/kubectl-set-image.1 @@ -97,6 +103,7 @@ docs/man/man1/kubectl-top-node.1 docs/man/man1/kubectl-top-pod.1 docs/man/man1/kubectl-top.1 docs/man/man1/kubectl-uncordon.1 +docs/man/man1/kubectl-update.1 docs/man/man1/kubectl-version.1 docs/man/man1/kubectl.1 docs/man/man1/kubelet.1 @@ -191,12 +198,14 @@ docs/user-guide/kubectl/kubectl_version.md docs/yaml/kubectl/kubectl.yaml docs/yaml/kubectl/kubectl_annotate.yaml docs/yaml/kubectl/kubectl_api-versions.yaml +docs/yaml/kubectl/kubectl_apiversions.yaml docs/yaml/kubectl/kubectl_apply.yaml docs/yaml/kubectl/kubectl_attach.yaml docs/yaml/kubectl/kubectl_auth.yaml docs/yaml/kubectl/kubectl_autoscale.yaml docs/yaml/kubectl/kubectl_certificate.yaml docs/yaml/kubectl/kubectl_cluster-info.yaml +docs/yaml/kubectl/kubectl_clusterinfo.yaml docs/yaml/kubectl/kubectl_completion.yaml docs/yaml/kubectl/kubectl_config.yaml docs/yaml/kubectl/kubectl_convert.yaml @@ -219,8 +228,11 @@ docs/yaml/kubectl/kubectl_plugin.yaml docs/yaml/kubectl/kubectl_port-forward.yaml docs/yaml/kubectl/kubectl_proxy.yaml docs/yaml/kubectl/kubectl_replace.yaml +docs/yaml/kubectl/kubectl_resize.yaml docs/yaml/kubectl/kubectl_rolling-update.yaml +docs/yaml/kubectl/kubectl_rollingupdate.yaml docs/yaml/kubectl/kubectl_rollout.yaml +docs/yaml/kubectl/kubectl_run-container.yaml docs/yaml/kubectl/kubectl_run.yaml docs/yaml/kubectl/kubectl_scale.yaml docs/yaml/kubectl/kubectl_set.yaml @@ -228,4 +240,5 @@ docs/yaml/kubectl/kubectl_stop.yaml docs/yaml/kubectl/kubectl_taint.yaml docs/yaml/kubectl/kubectl_top.yaml docs/yaml/kubectl/kubectl_uncordon.yaml +docs/yaml/kubectl/kubectl_update.yaml docs/yaml/kubectl/kubectl_version.yaml From bf2fc6214486fe401bc2dcdb3ed8442a7362c5ec Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Fri, 19 May 2017 10:40:21 -0700 Subject: [PATCH 05/10] kubectl: renmae deprecatedCmd -> deprecatedAlias This will clear up some of the confusion around the deprecatedCmd / Deprecated function. --- pkg/kubectl/cmd/cmd.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go index 4d41dc7af49..a01a23852c4 100644 --- a/pkg/kubectl/cmd/cmd.go +++ b/pkg/kubectl/cmd/cmd.go @@ -284,7 +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)), + deprecatedAlias("run-container", NewCmdRun(f, in, out, err)), set.NewCmdSet(f, out, err), }, }, @@ -302,9 +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)), + deprecatedAlias("rollingupdate", NewCmdRollingUpdate(f, out)), NewCmdScale(f, out), - deprecatedCmd("resize", NewCmdScale(f, out)), + deprecatedAlias("resize", NewCmdScale(f, out)), NewCmdAutoscale(f, out), }, }, @@ -313,7 +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)), + deprecatedAlias("clusterinfo", NewCmdClusterInfo(f, out)), NewCmdTop(f, out, err), NewCmdCordon(f, out), NewCmdUncordon(f, out), @@ -340,7 +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)), + deprecatedAlias("update", NewCmdReplace(f, out)), NewCmdConvert(f, out), }, }, @@ -377,7 +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(deprecatedAlias("apiversions", NewCmdApiVersions(f, out))) cmds.AddCommand(NewCmdOptions()) return cmds @@ -391,10 +391,10 @@ func printDeprecationWarning(command, alias string) { glog.Warningf("%s is DEPRECATED and will be removed in a future version. Use %s instead.", alias, command) } -// deprecatedCmd is intended to be used to create a "wrapper" command around an -// existing command. The wrapper works the same but prints a deprecation -// message before running. -func deprecatedCmd(deprecatedVersion string, cmd *cobra.Command) *cobra.Command { +// deprecatedAlias is intended to be used to create a "wrapper" command around +// an existing command. The wrapper works the same but prints a deprecation +// message before running. This command is identical functionality. +func deprecatedAlias(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() @@ -405,6 +405,9 @@ func deprecatedCmd(deprecatedVersion string, cmd *cobra.Command) *cobra.Command return cmd } +// Deprecated is similar to deprecatedAlias, but it is used for deprecations +// that are not simple aliases; this command is actually a different +// (deprecated) codepath. 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) From 2f0faedbee55e775e4e116c587b0867e5eb1cacd Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Fri, 19 May 2017 10:45:49 -0700 Subject: [PATCH 06/10] kubectl: make "Deprecated" a private function There's no reason to export this function, so I've made it private. --- pkg/kubectl/cmd/cmd.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go index a01a23852c4..91639dddfda 100644 --- a/pkg/kubectl/cmd/cmd.go +++ b/pkg/kubectl/cmd/cmd.go @@ -357,7 +357,7 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob filters := []string{ "options", - Deprecated("kubectl", "delete", cmds, NewCmdStop(f, out)), + deprecated("kubectl", "delete", cmds, NewCmdStop(f, out)), } templates.ActsAsRootCommand(cmds, filters, groups...) @@ -405,12 +405,12 @@ func deprecatedAlias(deprecatedVersion string, cmd *cobra.Command) *cobra.Comman return cmd } -// Deprecated is similar to deprecatedAlias, but it is used for deprecations +// deprecated is similar to deprecatedAlias, but it is used for deprecations // that are not simple aliases; this command is actually a different // (deprecated) codepath. -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) +func deprecated(baseName, to string, parent, cmd *cobra.Command) string { + cmd.Long = fmt.Sprintf("Deprecated: all functionality can be found in \"%s %s\"", baseName, to) + cmd.Short = fmt.Sprintf("Deprecated: use %s", to) parent.AddCommand(cmd) return cmd.Name() } From ca899ec04882df9a148bc6c15f48e76af3cf7223 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Fri, 19 May 2017 11:24:53 -0700 Subject: [PATCH 07/10] kubectl: write unit test for deprecatedAlias() --- pkg/kubectl/cmd/cmd_test.go | 38 +++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 19514f2fa1f..efa359bc1b6 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -25,9 +25,12 @@ import ( "net/http" "os" "reflect" + stdstrings "strings" "testing" "time" + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -659,3 +662,38 @@ func genResponseWithJsonEncodedBody(bodyStruct interface{}) (*http.Response, err } return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bytesBody(jsonBytes)}, nil } + +func Test_deprecatedAlias(t *testing.T) { + makeCobraCommand := func() *cobra.Command { + cobraCmd := new(cobra.Command) + cobraCmd.Use = "print five lines" + return cobraCmd + } + + original := makeCobraCommand() + alias := deprecatedAlias("echo", makeCobraCommand()) + + if len(alias.Deprecated) == 0 { + t.Error("deprecatedAlias should always have a non-empty .Deprecated") + } + if !stdstrings.Contains(alias.Deprecated, "print") { + t.Error("deprecatedAlias should give the name of the new function in its .Deprecated field") + } + if !alias.Hidden { + t.Error("deprecatedAlias should never have .Hidden == false (deprecated aliases should be hidden)") + } + + if alias.Name() != "echo" { + t.Errorf("deprecatedAlias has name %q, expected %q", + alias.Name(), "echo") + } + if original.Name() != "print" { + t.Errorf("original command has name %q, expected %q", + original.Name(), "print") + } + + // It would be nice to test to see that original.Run == alias.Run + // Unfortunately Golang does not allow comparing functions. I could do + // this with reflect, but that's technically invoking undefined + // behavior. +} From 5e1a3fd020a864ff065f997c873e5c4197f65e8d Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Fri, 19 May 2017 14:49:03 -0700 Subject: [PATCH 08/10] kubectl: improve deprecatedAlias unit test Two new behaviors are tested: 1. The output message that deprecatedAlias gives when it is called must include the word "deprecatated" and the name of the new function that the user should use instead. 2. The correct function must be called by the alias (alias should "fall back" to the functionality of the original. --- pkg/kubectl/cmd/cmd_test.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index efa359bc1b6..6fec9f56fa4 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -664,9 +664,13 @@ func genResponseWithJsonEncodedBody(bodyStruct interface{}) (*http.Response, err } func Test_deprecatedAlias(t *testing.T) { + var correctCommandCalled bool makeCobraCommand := func() *cobra.Command { cobraCmd := new(cobra.Command) cobraCmd.Use = "print five lines" + cobraCmd.Run = func(*cobra.Command, []string) { + correctCommandCalled = true + } return cobraCmd } @@ -692,8 +696,19 @@ func Test_deprecatedAlias(t *testing.T) { original.Name(), "print") } + buffer := new(bytes.Buffer) + alias.SetOutput(buffer) + alias.Execute() + str := buffer.String() + if !stdstrings.Contains(str, "deprecated") || !stdstrings.Contains(str, "print") { + t.Errorf("deprecation warning %q does not include enough information", str) + } + // It would be nice to test to see that original.Run == alias.Run // Unfortunately Golang does not allow comparing functions. I could do // this with reflect, but that's technically invoking undefined - // behavior. + // behavior. Best we can do is make sure that the function is called. + if !correctCommandCalled { + t.Errorf("original function doesn't appear to have been called by alias") + } } From 46cdb3966c3f306a4efea15d30527ce4a9ec4cd2 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Mon, 22 May 2017 10:49:44 -0700 Subject: [PATCH 09/10] test-cmd-util: fix deprecated commands test Additionally, move the test down to ensure definition order matches run order. --- hack/make-rules/test-cmd-util.sh | 36 +++++++++++++++----------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 6b4b1a14fd8..3d9f77e4eeb 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -1077,25 +1077,6 @@ run_save_config_tests() { kubectl delete rc frontend "${kube_flags[@]}" } -run_kubectl_using_deprecated_commands_test() { - ## `kubectl run-container` should function identical to `kubectl run`, but it - ## should also print a deprecation warning. - # Pre-Condition: no Job exists - kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" '' - # Command - output_message=$(kubectl run-container pi --generator=job/v1 "--image=$IMAGE_PERL" --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(20)' "${kube_flags[@]}") - # Ensure that the user is warned their command is deprecated. - kube::test::if_has_string "${output_message}" 'Deprecated' - # Post-Condition: Job "pi" is created - kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" 'pi:' - # Clean up - kubectl delete jobs pi "${kube_flags[@]}" - # Post-condition: no pods exist. - kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' - # Clean up - kubectl delete deployment nginx-apps "${kube_flags[@]}" -} - run_kubectl_run_tests() { ## kubectl run should create deployments or jobs # Pre-Condition: no Job exists @@ -1132,6 +1113,23 @@ run_kubectl_run_tests() { kubectl delete deployment nginx-apps "${kube_flags[@]}" } +run_kubectl_using_deprecated_commands_test() { + ## `kubectl run-container` should function identical to `kubectl run`, but it + ## should also print a deprecation warning. + # Pre-Condition: no Job exists + kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" '' + # Command + output_message=$(kubectl 2>&1 run-container pi --generator=job/v1 "--image=$IMAGE_PERL" --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(15)' "${kube_flags[@]}") + # Ensure that the user is warned their command is deprecated. + kube::test::if_has_string "${output_message}" 'deprecated' + # Post-Condition: Job "pi" is created + kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" 'pi:' + # Clean up + kubectl delete jobs pi "${kube_flags[@]}" + # Post-condition: no pods exist. + kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' +} + run_kubectl_get_tests() { ### Test retrieval of non-existing pods # Pre-condition: no POD exists From b7cc61816fa46e79f804d2876f3148e09f11840b Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Mon, 22 May 2017 14:22:48 -0700 Subject: [PATCH 10/10] docs: add new generated docs for deprecated commands --- docs/man/man1/kubectl-apiversions.1 | 3 +++ docs/man/man1/kubectl-clusterinfo-dump.1 | 3 +++ docs/man/man1/kubectl-clusterinfo.1 | 3 +++ docs/man/man1/kubectl-resize.1 | 3 +++ docs/man/man1/kubectl-rollingupdate.1 | 3 +++ docs/man/man1/kubectl-run-container.1 | 3 +++ docs/man/man1/kubectl-update.1 | 3 +++ docs/yaml/kubectl/kubectl_apiversions.yaml | 3 +++ docs/yaml/kubectl/kubectl_clusterinfo.yaml | 3 +++ docs/yaml/kubectl/kubectl_resize.yaml | 3 +++ docs/yaml/kubectl/kubectl_rollingupdate.yaml | 3 +++ docs/yaml/kubectl/kubectl_run-container.yaml | 3 +++ docs/yaml/kubectl/kubectl_update.yaml | 3 +++ 13 files changed, 39 insertions(+) create mode 100644 docs/man/man1/kubectl-apiversions.1 create mode 100644 docs/man/man1/kubectl-clusterinfo-dump.1 create mode 100644 docs/man/man1/kubectl-clusterinfo.1 create mode 100644 docs/man/man1/kubectl-resize.1 create mode 100644 docs/man/man1/kubectl-rollingupdate.1 create mode 100644 docs/man/man1/kubectl-run-container.1 create mode 100644 docs/man/man1/kubectl-update.1 create mode 100644 docs/yaml/kubectl/kubectl_apiversions.yaml create mode 100644 docs/yaml/kubectl/kubectl_clusterinfo.yaml create mode 100644 docs/yaml/kubectl/kubectl_resize.yaml create mode 100644 docs/yaml/kubectl/kubectl_rollingupdate.yaml create mode 100644 docs/yaml/kubectl/kubectl_run-container.yaml create mode 100644 docs/yaml/kubectl/kubectl_update.yaml diff --git a/docs/man/man1/kubectl-apiversions.1 b/docs/man/man1/kubectl-apiversions.1 new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/man/man1/kubectl-apiversions.1 @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/man/man1/kubectl-clusterinfo-dump.1 b/docs/man/man1/kubectl-clusterinfo-dump.1 new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/man/man1/kubectl-clusterinfo-dump.1 @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/man/man1/kubectl-clusterinfo.1 b/docs/man/man1/kubectl-clusterinfo.1 new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/man/man1/kubectl-clusterinfo.1 @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/man/man1/kubectl-resize.1 b/docs/man/man1/kubectl-resize.1 new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/man/man1/kubectl-resize.1 @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/man/man1/kubectl-rollingupdate.1 b/docs/man/man1/kubectl-rollingupdate.1 new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/man/man1/kubectl-rollingupdate.1 @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/man/man1/kubectl-run-container.1 b/docs/man/man1/kubectl-run-container.1 new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/man/man1/kubectl-run-container.1 @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/man/man1/kubectl-update.1 b/docs/man/man1/kubectl-update.1 new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/man/man1/kubectl-update.1 @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/yaml/kubectl/kubectl_apiversions.yaml b/docs/yaml/kubectl/kubectl_apiversions.yaml new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/yaml/kubectl/kubectl_apiversions.yaml @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/yaml/kubectl/kubectl_clusterinfo.yaml b/docs/yaml/kubectl/kubectl_clusterinfo.yaml new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/yaml/kubectl/kubectl_clusterinfo.yaml @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/yaml/kubectl/kubectl_resize.yaml b/docs/yaml/kubectl/kubectl_resize.yaml new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/yaml/kubectl/kubectl_resize.yaml @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/yaml/kubectl/kubectl_rollingupdate.yaml b/docs/yaml/kubectl/kubectl_rollingupdate.yaml new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/yaml/kubectl/kubectl_rollingupdate.yaml @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/yaml/kubectl/kubectl_run-container.yaml b/docs/yaml/kubectl/kubectl_run-container.yaml new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/yaml/kubectl/kubectl_run-container.yaml @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file. diff --git a/docs/yaml/kubectl/kubectl_update.yaml b/docs/yaml/kubectl/kubectl_update.yaml new file mode 100644 index 00000000000..b6fd7a0f989 --- /dev/null +++ b/docs/yaml/kubectl/kubectl_update.yaml @@ -0,0 +1,3 @@ +This file is autogenerated, but we've stopped checking such files into the +repository to reduce the need for rebases. Please run hack/generate-docs.sh to +populate this file.