From fe3bcad128e537e72ec58ff22b68137d8b2f593f Mon Sep 17 00:00:00 2001 From: Jacob Tanenbaum Date: Tue, 18 Oct 2016 11:42:58 -0400 Subject: [PATCH] fixed some issues with kubectl set resources when using kubectl set resources it resets all resource fields that are not being set. for example # kubectl set resources deployments nginx --limits=cpu=100m followed by # kubectl set resources deployments nginx --limits=memory=256Mi would result in the nginx deployment only limiting memory at 256Mi with the previous limit placed on the cpu being wiped out. This behavior is corrected so that each invocation only modifies fields set in that command and changed the testing so that the desired behavior is checked. Also a typo: you must specify an update to requests or limits or (in the form of --requests/--limits) corrected to you must specify an update to requests or limits (in the form of --requests/--limits) changelog: - fixed a typo in hack/make-rules/test-cmd.sh "effecting" to "affecting" --- hack/make-rules/test-cmd.sh | 19 ++++++++++++++++--- pkg/kubectl/cmd/set/set_resources.go | 16 ++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index dab33477e7e..56b7e61fa31 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -2168,12 +2168,25 @@ __EOF__ 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[@]}" - 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:" + kubectl set resources deployment nginx-deployment --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}}" "200m:" + # Set the deployments memory limits without affecting the cpu limits + kubectl set resources deployment nginx-deployment --limits=memory=256Mi "${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 0).resources.limits.memory}}:{{end}}" "256Mi:" + # Set the deployments resource requests without effecting the deployments limits + kubectl set resources deployment nginx-deployment --requests=cpu=100m,memory=256Mi "${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 0).resources.limits.memory}}:{{end}}" "256Mi:" + kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 0).resources.requests.cpu}}:{{end}}" "100m:" + kube::test::get_object_assert deployment "{{range.items}}{{(index .spec.template.spec.containers 0).resources.requests.memory}}:{{end}}" "256Mi:" + # Set the deployments resource requests and limits to zero + kubectl set resources deployment nginx-deployment --requests=cpu=0,memory=0 --limits=cpu=0,memory=0 "${kube_flags[@]}" # Set a non-existing container should fail ! kubectl set resources deployment nginx-deployment -c=redis --limits=cpu=100m # Set the limit of a specific container in deployment + kubectl set resources deployment nginx-deployment --limits=cpu=100m "${kube_flags[@]}" kubectl set resources deployment nginx-deployment -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:" diff --git a/pkg/kubectl/cmd/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index a95362a27bc..276d4b84267 100644 --- a/pkg/kubectl/cmd/set/set_resources.go +++ b/pkg/kubectl/cmd/set/set_resources.go @@ -159,7 +159,7 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args func (o *ResourcesOptions) Validate() error { var err error if len(o.Limits) == 0 && len(o.Requests) == 0 { - return fmt.Errorf("you must specify an update to requests or limits or (in the form of --requests/--limits)") + return fmt.Errorf("you must specify an update to requests or limits (in the form of --requests/--limits)") } o.ResourceRequirements, err = kubectl.HandleResourceRequirements(map[string]string{"limits": o.Limits, "requests": o.Requests}) @@ -178,7 +178,19 @@ func (o *ResourcesOptions) Run() error { containers, _ := selectContainers(spec.Containers, o.ContainerSelector) if len(containers) != 0 { for i := range containers { - containers[i].Resources = o.ResourceRequirements + if len(o.Limits) != 0 && len(containers[i].Resources.Limits) == 0 { + containers[i].Resources.Limits = make(api.ResourceList) + } + for key, value := range o.ResourceRequirements.Limits { + containers[i].Resources.Limits[key] = value + } + + if len(o.Requests) != 0 && len(containers[i].Resources.Requests) == 0 { + containers[i].Resources.Requests = make(api.ResourceList) + } + for key, value := range o.ResourceRequirements.Requests { + containers[i].Resources.Requests[key] = value + } transformed = true } } else {