From 1c51ac4fc1f40455bb3f95820a5d0f432058e1d7 Mon Sep 17 00:00:00 2001 From: Saad Ali Date: Tue, 1 Nov 2016 13:53:17 -0700 Subject: [PATCH] Revert "fixed some issues with kubectl set resources" --- hack/make-rules/test-cmd.sh | 26 ++++---------------------- pkg/kubectl/cmd/set/set_resources.go | 25 ++++++------------------- 2 files changed, 10 insertions(+), 41 deletions(-) diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index 6a12ed50861..70e3c9a4da2 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -2181,34 +2181,18 @@ __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.yaml -c=perl --limits=cpu=300m --requests=cpu=300m --local -o yaml "${kube_flags[@]}" - ! kubectl set resources -f hack/testdata/deployment-multicontainer.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:' 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=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[@]}" + 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:" # 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:" @@ -2218,9 +2202,7 @@ __EOF__ 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 -f hack/testdata/deployment-multicontainer.yaml -c=perl --limits=cpu=500m --requests=cpu=500m --dry-run -o yaml "${kube_flags[@]}" - kubectl set resources deployment nginx-deployment -c=perl --limits=cpu=500m --requests=cpu=500m --dry-run -o yaml "${kube_flags[@]}" - ! kubectl set resources deployment nginx-deployment -c=perl --limits=cpu=500m --requests=cpu=500m --local -o yaml "${kube_flags[@]}" + kubectl set resources deployment -f hack/testdata/deployment-multicontainer.yaml -c=perl --limits=cpu=300m --requests=cpu=300m --dry-run -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:" diff --git a/pkg/kubectl/cmd/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index 9d991242746..3883e38732e 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 --local -o yaml`) + kubectl set resources -f path/to/file.yaml --limits=cpu=200m,memory=512Mi --dry-run -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,7 +72,6 @@ type ResourcesOptions struct { All bool Record bool ChangeCause string - Local bool Cmd *cobra.Command Limits string @@ -115,7 +114,6 @@ 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.") @@ -144,7 +142,7 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args //FilenameParam(enforceNamespace, o.Filenames...). FilenameParam(enforceNamespace, &o.FilenameOptions). Flatten() - if !o.Local { + if !cmdutil.GetDryRunFlag(cmd) { builder = builder. SelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, args...). @@ -161,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 (in the form of --requests/--limits)") + return fmt.Errorf("you must specify an update to requests or limits or (in the form of --requests/--limits)") } o.ResourceRequirements, err = kubectl.HandleResourceRequirements(map[string]string{"limits": o.Limits, "requests": o.Requests}) @@ -180,19 +178,7 @@ func (o *ResourcesOptions) Run() error { containers, _ := selectContainers(spec.Containers, o.ContainerSelector) if len(containers) != 0 { for i := range containers { - 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 - } + containers[i].Resources = o.ResourceRequirements transformed = true } } else { @@ -216,7 +202,8 @@ func (o *ResourcesOptions) Run() error { continue } - if o.Local || cmdutil.GetDryRunFlag(o.Cmd) { + if cmdutil.GetDryRunFlag(o.Cmd) { + fmt.Fprintln(o.Err, "info: running in local mode...") return o.PrintObject(o.Cmd, o.Mapper, info.Object, o.Out) }