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.

Sign up for GitHub

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

Jump to bottom

Fixing clusterautoscaler rbac #3145

Merged
merged 2 commits into from
Aug 6, 2017
Merged

Fixing clusterautoscaler rbac #3145

merged 2 commits into from
Aug 6, 2017

Conversation

BradErz
Copy link
Contributor

@BradErz BradErz commented Aug 5, 2017

Fixes #3144

I have tested on my cluster and it can now update the status of the pods. Im not sure if it also needs the permission to patch on the endpoint

Maybe someone with better RBAC knowlage can pick up on this? @chrislovecnm

I also gave the file some pretty treatment.. sorry habbit

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 5, 2017
@k8s-ci-robot
Copy link
Contributor

Hi @BradErz. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 5, 2017
@BradErz
Copy link
Contributor Author

BradErz commented Aug 5, 2017

/assign @chrislovecnm @justinsb

@justinsb
Copy link
Member

justinsb commented Aug 6, 2017

Looks good. I believe you just moved the deployment and didn't change anything in there - can you please let me know if that is the case?

Adding the patch permission makes sense. @sethpollack you've done a lot with cluster autoscaler - want to have a look at this one?

Unless anyone objects though, I'm inclined to get this in, as it seems to be a strict improvement.

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 6, 2017
@justinsb justinsb assigned sethpollack and unassigned chrislovecnm Aug 6, 2017
@sethpollack
Copy link
Contributor

lgtm

@BradErz
Copy link
Contributor Author

BradErz commented Aug 6, 2017

@justinsb yea, the deployment hasn't changed. Just moved it to the bottom of the file.

@justinsb
Copy link
Member

justinsb commented Aug 6, 2017

/lgtm

Thanks for taking a look @sethpollack , and thanks for fixing @BradErz

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BradErz, justinsb

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 6, 2017
@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 0942ae3 into kubernetes:master Aug 6, 2017
erickt added a commit to erickt/charts that referenced this pull request Nov 17, 2017
@erickt
[stable/cluster-autoscaler] Bump to 1.0.3, allow patching pod status
2bbeb2c 
This updates the autoscaler to 1.0.3, and adds a few new permissions
that were implemented by Kops:

* kubernetes/kops#3145 - Fixing clusterautoscaler
  rbac.
* kubernetes/kops#3419 - Allow cluster autoscaler
  to scale down.
@erickt erickt mentioned this pull request Nov 17, 2017
Merged
unguiculus pushed a commit to helm/charts that referenced this pull request Dec 9, 2017
@erickt @unguiculus
[stable/cluster-autoscaler] Bump to 1.0.3, allow patching pod status (#…
47a8f22 
…2776)

This updates the autoscaler to 1.0.3, and adds a few new permissions
that were implemented by Kops:

* kubernetes/kops#3145 - Fixing clusterautoscaler
  rbac.
* kubernetes/kops#3419 - Allow cluster autoscaler
  to scale down.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sethpollack sethpollack sethpollack approved these changes

Assignees

@justinsb justinsb

@sethpollack sethpollack

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@BradErz @k8s-ci-robot @justinsb @sethpollack @k8s-github-robot @chrislovecnm