From 13afa70c81567a7cb8b47b84e0f7e0fbd7d0d07a Mon Sep 17 00:00:00 2001 From: Jacob Tanenbaum Date: Wed, 26 Oct 2016 09:48:46 -0400 Subject: [PATCH] Implemented both the dry run and local flags. Added test cases to show that both flags are operating as intended. Removed the print statement "running in local mode" as in PR#35112 The previous attempt at the PR (PR#35050) was reverted for causeing a flake. I believe that setting the deployments terminationGracePeriodSeconds to 0 should take care of it. I ran hack/make-rules/test-cmd.sh 50 times in a row without encountering the flake --- hack/make-rules/test-cmd.sh | 22 ++++++++++------- .../deployment-multicontainer-resources.yaml | 24 +++++++++++++++++++ pkg/kubectl/cmd/set/set_resources.go | 9 +++---- 3 files changed, 42 insertions(+), 13 deletions(-) create mode 100644 hack/testdata/deployment-multicontainer-resources.yaml diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index 9253ef4b7a..25ad971969 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -2181,33 +2181,37 @@ __EOF__ ## Set resource limits/request of a deployment # Pre-condition: no deployment exists kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" '' + # Set resources of a local file without talking to the server + kubectl set resources -f hack/testdata/deployment-multicontainer-resources.yaml -c=perl --limits=cpu=300m --requests=cpu=300m --local -o yaml "${kube_flags[@]}" + ! kubectl set resources -f hack/testdata/deployment-multicontainer-resources.yaml -c=perl --limits=cpu=300m --requests=cpu=300m --dry-run -o yaml "${kube_flags[@]}" # Create a deployment - kubectl create -f hack/testdata/deployment-multicontainer.yaml "${kube_flags[@]}" - kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" 'nginx-deployment:' + kubectl create -f hack/testdata/deployment-multicontainer-resources.yaml "${kube_flags[@]}" + kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" 'nginx-deployment-resources:' kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_DEPLOYMENT_R1}:" kube::test::get_object_assert deployment "{{range.items}}{{$deployment_second_image_field}}:{{end}}" "${IMAGE_PERL}:" # Set the deployment's cpu limits - kubectl set resources deployment nginx-deployment --limits=cpu=100m "${kube_flags[@]}" + kubectl set resources deployment nginx-deployment-resources --limits=cpu=100m "${kube_flags[@]}" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 0).resources.limits.cpu}}:{{end}}" "100m:" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 1).resources.limits.cpu}}:{{end}}" "100m:" # Set a non-existing container should fail - ! kubectl set resources deployment nginx-deployment -c=redis --limits=cpu=100m + ! kubectl set resources deployment nginx-deployment-resources -c=redis --limits=cpu=100m # Set the limit of a specific container in deployment - kubectl set resources deployment nginx-deployment -c=nginx --limits=cpu=200m "${kube_flags[@]}" + kubectl set resources deployment nginx-deployment-resources -c=nginx --limits=cpu=200m "${kube_flags[@]}" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 0).resources.limits.cpu}}:{{end}}" "200m:" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 1).resources.limits.cpu}}:{{end}}" "100m:" # Set limits/requests of a deployment specified by a file - kubectl set resources -f hack/testdata/deployment-multicontainer.yaml -c=perl --limits=cpu=300m --requests=cpu=300m "${kube_flags[@]}" + kubectl set resources -f hack/testdata/deployment-multicontainer-resources.yaml -c=perl --limits=cpu=300m --requests=cpu=300m "${kube_flags[@]}" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 0).resources.limits.cpu}}:{{end}}" "200m:" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 1).resources.limits.cpu}}:{{end}}" "300m:" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 1).resources.requests.cpu}}:{{end}}" "300m:" - # Set limits on a local file without talking to the server - kubectl set resources deployment -f hack/testdata/deployment-multicontainer.yaml -c=perl --limits=cpu=300m --requests=cpu=300m --dry-run -o yaml "${kube_flags[@]}" + # Show dry-run works on running deployments + kubectl set resources deployment nginx-deployment-resources -c=perl --limits=cpu=400m --requests=cpu=400m --dry-run -o yaml "${kube_flags[@]}" + ! kubectl set resources deployment nginx-deployment-resources -c=perl --limits=cpu=400m --requests=cpu=400m --local -o yaml "${kube_flags[@]}" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 0).resources.limits.cpu}}:{{end}}" "200m:" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 1).resources.limits.cpu}}:{{end}}" "300m:" kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 1).resources.requests.cpu}}:{{end}}" "300m:" # Clean up - kubectl delete deployment nginx-deployment "${kube_flags[@]}" + kubectl delete deployment nginx-deployment-resources "${kube_flags[@]}" ###################### diff --git a/hack/testdata/deployment-multicontainer-resources.yaml b/hack/testdata/deployment-multicontainer-resources.yaml new file mode 100644 index 0000000000..533e2e46f4 --- /dev/null +++ b/hack/testdata/deployment-multicontainer-resources.yaml @@ -0,0 +1,24 @@ +apiVersion: extensions/v1beta1 +kind: Deployment +metadata: + name: nginx-deployment-resources + labels: + name: nginx-deployment-resources +spec: + replicas: 3 + selector: + matchLabels: + name: nginx + template: + metadata: + labels: + name: nginx + spec: + containers: + - name: nginx + image: gcr.io/google-containers/nginx:test-cmd + ports: + - containerPort: 80 + - name: perl + image: gcr.io/google-containers/perl + terminationGracePeriodSeconds: 0 diff --git a/pkg/kubectl/cmd/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index d583cc1810..9d99124274 100644 --- a/pkg/kubectl/cmd/set/set_resources.go +++ b/pkg/kubectl/cmd/set/set_resources.go @@ -52,7 +52,7 @@ var ( kubectl set resources deployment nginx --limits=cpu=0,memory=0 --requests=cpu=0,memory=0 # Print the result (in yaml format) of updating nginx container limits from a local, without hitting the server - kubectl set resources -f path/to/file.yaml --limits=cpu=200m,memory=512Mi --dry-run -o yaml`) + kubectl set resources -f path/to/file.yaml --limits=cpu=200m,memory=512Mi --local -o yaml`) ) // ResourcesOptions is the start of the data required to perform the operation. As new fields are added, add them here instead of @@ -72,6 +72,7 @@ type ResourcesOptions struct { All bool Record bool ChangeCause string + Local bool Cmd *cobra.Command Limits string @@ -114,6 +115,7 @@ func NewCmdResources(f cmdutil.Factory, out io.Writer, errOut io.Writer) *cobra. cmd.Flags().BoolVar(&options.All, "all", false, "select all resources in the namespace of the specified resource types") cmd.Flags().StringVarP(&options.Selector, "selector", "l", "", "Selector (label query) to filter on") cmd.Flags().StringVarP(&options.ContainerSelector, "containers", "c", "*", "The names of containers in the selected pod templates to change, all containers are selected by default - may use wildcards") + cmd.Flags().BoolVar(&options.Local, "local", false, "If true, set resources will NOT contact api-server but run locally.") cmdutil.AddDryRunFlag(cmd) cmdutil.AddRecordFlag(cmd) cmd.Flags().StringVar(&options.Limits, "limits", options.Limits, "The resource requirement requests for this container. For example, 'cpu=100m,memory=256Mi'. Note that server side components may assign requests depending on the server configuration, such as limit ranges.") @@ -142,7 +144,7 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args //FilenameParam(enforceNamespace, o.Filenames...). FilenameParam(enforceNamespace, &o.FilenameOptions). Flatten() - if !cmdutil.GetDryRunFlag(cmd) { + if !o.Local { builder = builder. SelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, args...). @@ -214,8 +216,7 @@ func (o *ResourcesOptions) Run() error { continue } - if cmdutil.GetDryRunFlag(o.Cmd) { - fmt.Fprintln(o.Err, "info: running in local mode...") + if o.Local || cmdutil.GetDryRunFlag(o.Cmd) { return o.PrintObject(o.Cmd, o.Mapper, info.Object, o.Out) }