Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'kubectl set resources' #27206

Merged
merged 1 commit into from
Oct 15, 2016

Conversation

JacobTanenbaum
Copy link
Contributor

@JacobTanenbaum JacobTanenbaum commented Jun 10, 2016

Add "kubectl set resources" for easier updating container memory/cpu limits/requests (for pods or resources with pod templates).

Usage

kubectl set resources (-f FILENAME | TYPE NAME) ([--limits=LIMITS & --requests=REQUESTS])

Examples
Set a deployments nginx container cpu limits to "200m and memory to "512Mi"
kubectl set resources deployment nginx -c=nginx --limits=cpu=200m,memory=512Mi

Set the limit and requests for all containers in nginx
kubectl set resources deployment nginx --limits=cpu=200m,memory=512Mi --requests=cpu=100m,memory=256Mi

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

Remove limits on containers in nginx
kubectl set resources deployment nginx --limits=cpu=0,memory=0

Ref: #21648

EDIT: removed the '--remove' flag example


This change is Reviewable

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@k8s-github-robot k8s-github-robot added kind/old-docs size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 10, 2016
@pwittrock
Copy link
Member

@JacobTanenbaum I'll take a look at this next week. It won't get merged until after 1.3 freeze is over.

@0xmichalis
Copy link
Contributor

@kubernetes/kubectl

@derekwaynecarr
Copy link
Member

I like the feature.

How about 'kubectl set resources' instead?

/cc @vishh

On Saturday, June 11, 2016, Michail Kargakis notifications@github.com
wrote:

@kubernetes/kubectl https://github.com/orgs/kubernetes/teams/kubectl


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#27206 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AF8dbBqwK6WYeuPE8FmyXl7GLPdweHv1ks5qKqaggaJpZM4IzYBd
.

@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 11, 2016 via email

allErrs := []error{}
patches := CalculatePatches(o.Infos, o.Encoder, func(info *resource.Info) (bool, error) {
transformed := false
_, err := o.UpdatePodSpecForObject(info.Object, func(spec *api.PodSpec) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other things may have resources besides pods (quota and limits). We should make an argument for or against whether this command should be polymorphic to type. The container selector is probably the main issue - we'd need a different selector for sub quota info.

@googlebot
Copy link

CLAs look good, thanks!

@JacobTanenbaum
Copy link
Contributor Author

Does everyone like 'kubectl set resources'?

continue
}

//no changes
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarterclayton I believe that I put the continue where you meant. If I understand correctly an empty patch is not an error so I added a note to stderr when a patch is empty that the resource was not changed

@vishh
Copy link
Contributor

vishh commented Jun 21, 2016

I prefer kubectl set resources as well.

@k8s-github-robot k8s-github-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 22, 2016
@derekwaynecarr derekwaynecarr added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 10, 2016
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 10, 2016
@derekwaynecarr derekwaynecarr added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2016
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 13, 2016
@eparis eparis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 13, 2016
@@ -2211,6 +2242,7 @@ __EOF__
kubectl delete deployment nginx-deployment "${kube_flags[@]}"



Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why add whitespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace removed

@@ -4,3 +4,6 @@


[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/pkg/cloudprovider/providers/openstack/MAINTAINERS.md?pixel)]()


[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/_tmp/pkg/cloudprovider/providers/openstack/MAINTAINERS.md?pixel)]()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug of some sort... should not be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed entry

@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 13, 2016
@eparis
Copy link
Contributor

eparis commented Oct 13, 2016

The unit/integration test failure can be reliably recreated doing make test-cmd

Apparently:

output_message=$(! kubectl get pod --context="" --kubeconfig=missing 2>&1)

is the failing line. Before this PR the command returned an error. After the PR the command does not return an error.

@eparis
Copy link
Contributor

eparis commented Oct 13, 2016

The command should fail and output_message should include missing: no such file or directory

instead it is passing and output_message is getting No resources found.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 13, 2016
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2016
Add a way to set resource limits/requests on running pods

Ref: kubernetes#21648

I squashed the commits to make rebasing easier
Change log:

- fixed a typo that caused the command to be run with kubectl set set instead of the correct kubectl set limit

- added a ResourcesWithPodTemplates to pkg/kubectl/cmd/util/factory.go
     instead of hardcoding these resources move there description all in one place

- Fixing some of the flow control in kubectl set limit

- update the help info

- changed the name of ResourcesWithPodTemplates to ResourcesWithPodSpecs to more accuratly describe what it is doing
    and changed the variable names to lower case to conform to go's variable naming convention

- changing the name of the command from 'set limit' to 'set resources'

- Adding the new file pkg/kubectl/cmd/set/set_resources.go

- changes to the test cases to reflect the change from 'kubectl set limit' to 'kubectl set resources'

- comment removed

- adding the man page to the git repository attempting to fix Jenkins tests

- adding the user guide

- fixed a few typos

- typo in hack/cmd-test.sh

- implamenting suggestions for command help text

- adding the dry-run flag

- removing the "remove" option in favor of zeroing out request/limits in order to remove them

- changed limits/requests to requests/limit

- changing ResourcesWithPodSpec

- updated generated docs and removed whitespace

- change priint on success message from "resource limits/requests updated" to "resource requirements updated"

- minor rebasing issues - 'hack/test-cmd.sh' now passes

- cmdutil.PrintSuccess added another argument

- fixing mungedocs failure

- removed whitespace from hack/make-rules/test-cmd.sh and an erroneous entry from pkg/cloudprovider/providers/openstack/MAINTAINERS.md

- fixed typo in Short: field of the cobra command

- rebased

- Creating a new factory in the ResourcesWithPodSpecs() so that the testing will pass

- changing ResourcesWithPodSpecs, it doesn't need to be a method of factory
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2016
@eparis eparis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2016
@eparis eparis added this to the v1.5 milestone Oct 14, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 901bbee. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit e2ef58e into kubernetes:master Oct 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet