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 service account for attach-detach controller #10892

Merged
merged 1 commit into from
Sep 14, 2016

Conversation

pmorie
Copy link
Contributor

@pmorie pmorie commented Sep 13, 2016

Adds a service account for the attach-detach controller.

Rules: []authorizationapi.PolicyRule{
// shared informer on PVs
{
Verbs: sets.NewString("list", "watch"),
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need get on any of the list/watch resources?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need it on nodes, which is accounted for.

@liggitt
Copy link
Contributor

liggitt commented Sep 13, 2016

questions on a couple permissions, test fixture update needed, LGTM otherwise

@pmorie
Copy link
Contributor Author

pmorie commented Sep 14, 2016

@liggitt light is green and in-situ test checked out, i can haz merge?

@liggitt
Copy link
Contributor

liggitt commented Sep 14, 2016

Is this 1.3.0?

@@ -44,6 +44,9 @@ const (
InfraPersistentVolumeBinderControllerServiceAccountName = "pv-binder-controller"
PersistentVolumeBinderControllerRoleName = "system:pv-binder-controller"

InfraAttachDetachControllerServiceAccountName = "attach-detach-controller"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this named "pv-…" for consistency with the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm cool with that, I'll rename tonight.

@pmorie
Copy link
Contributor Author

pmorie commented Sep 14, 2016

yep

@liggitt liggitt added this to the 1.3.0 milestone Sep 14, 2016
@liggitt
Copy link
Contributor

liggitt commented Sep 14, 2016

LGTM, [merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to dd1ce84

@openshift-bot openshift-bot merged commit e9081ae into openshift:master Sep 14, 2016
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8851/)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants