github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #46062 from alexandercampbell/correct-deprecation-errors 

Automatic merge from submit-queue (batch tested with PRs 46201, 45952, 45427, 46247, 46062)

kubectl: fix deprecation warning bug

**What this PR does / why we need it**:

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 resize [...]

	# Should print the same warning but no warning is printed.
	kubectl --v=1 resize [...]

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.

**Release note**:

```release-note
NONE
```
pull/6/head
Kubernetes Submit Queue 2017-05-22 20:58:07 -07:00 committed by GitHub
parent 31bd852ec1 b7cc61816f
commit 644a544d62
23 changed files with 157 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
docs/.generated_docs
View File

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

3
docs/man/man1/kubectl-apiversions.1 Normal file
View File

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

3
docs/man/man1/kubectl-clusterinfo-dump.1 Normal file
View File

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

3
docs/man/man1/kubectl-clusterinfo.1 Normal file
View File

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

3
docs/man/man1/kubectl-resize.1 Normal file
View File

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

3
docs/man/man1/kubectl-rollingupdate.1 Normal file
View File

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

3
docs/man/man1/kubectl-run-container.1 Normal file
View File

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

3
docs/man/man1/kubectl-update.1 Normal file
View File

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

3
docs/yaml/kubectl/kubectl_apiversions.yaml Normal file
View File

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

3
docs/yaml/kubectl/kubectl_clusterinfo.yaml Normal file
View File

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

3
docs/yaml/kubectl/kubectl_resize.yaml Normal file
View File

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

3
docs/yaml/kubectl/kubectl_rollingupdate.yaml Normal file
View File

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

3
docs/yaml/kubectl/kubectl_run-container.yaml Normal file
View File

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

3
docs/yaml/kubectl/kubectl_update.yaml Normal file
View File

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

18
hack/make-rules/test-cmd-util.sh
View File

@ -1114,6 +1114,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
@ -3216,6 +3233,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

9
pkg/kubectl/cmd/apiversions.go
View File

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

11
pkg/kubectl/cmd/clusterinfo.go
View File

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

31
pkg/kubectl/cmd/cmd.go
View File

@ -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),
deprecatedAlias("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),
deprecatedAlias("rollingupdate", NewCmdRollingUpdate(f, out)),
NewCmdScale(f, out),
deprecatedAlias("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),
deprecatedAlias("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),
deprecatedAlias("update", NewCmdReplace(f, out)),
NewCmdConvert(f, out),
},
},
@ -352,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...)
@ -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(deprecatedAlias("apiversions", NewCmdApiVersions(f, out)))
cmds.AddCommand(NewCmdOptions())
return cmds
@ -385,9 +391,26 @@ func printDeprecationWarning(command, alias string) {
glog.Warningf("%s is DEPRECATED and will be removed in a future version. Use %s instead.", alias, command)
}
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)
// 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()
cmd.Use = deprecatedVersion
cmd.Deprecated = fmt.Sprintf("use %q instead", originalName)
cmd.Hidden = true
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: all functionality can be found in \"%s %s\"", baseName, to)
cmd.Short = fmt.Sprintf("Deprecated: use %s", to)
parent.AddCommand(cmd)
return cmd.Name()
}

53
pkg/kubectl/cmd/cmd_test.go
View File

@ -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,53 @@ func genResponseWithJsonEncodedBody(bodyStruct interface{}) (*http.Response, err
}
return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bytesBody(jsonBytes)}, nil
}
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
}
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")
}
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. 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")
}
}

7
pkg/kubectl/cmd/replace.go
View File

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

8
pkg/kubectl/cmd/rollingupdate.go
View File

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

9
pkg/kubectl/cmd/run.go
View File

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

9
pkg/kubectl/cmd/scale.go
View File

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