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

switch to upstream controller roles and init functions where easily possible #14126

Merged
merged 1 commit into from
May 17, 2017

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented May 10, 2017

@mfojtik I've tried to the keep the changes small-ish and easy to follow. There are no behavior or flow changes. This simply removes unnecessary roles and switches to upstream initialization where its zero change.

There are more changes to make that involve changes to the flow of controller init function creation. I'll save those for later.

Now that I have fewer infra_sa_policy.go roles to deal with, I'm going to convert them to the upstream construction style.

@deads2k
Copy link
Contributor Author

deads2k commented May 10, 2017

[test]

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

LGTM

}
controllerInitializers := kctrlmgr.NewControllerInitializers()

// TODO I think this should become a blacklist kept in sync during rebases with a unit test.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

I can work on a followup up for that, if you want me to.

@deads2k
Copy link
Contributor Author

deads2k commented May 11, 2017

[merge] This is on the path to allowing bug fix #13996

@deads2k
Copy link
Contributor Author

deads2k commented May 12, 2017

ansible re[merge]

@soltysh
Copy link
Contributor

soltysh commented May 12, 2017

Flake #14161

@deads2k
Copy link
Contributor Author

deads2k commented May 13, 2017

[merge]

@deads2k
Copy link
Contributor Author

deads2k commented May 15, 2017

[test]

@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label May 15, 2017
@deads2k
Copy link
Contributor Author

deads2k commented May 16, 2017

re[test] re[merge]

@deads2k
Copy link
Contributor Author

deads2k commented May 17, 2017

Not seeing how I affected vendor/k8s.io/kubernetes/pkg/controller/node.TestCancel
re[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to d0e3554

@enj
Copy link
Contributor

enj commented May 17, 2017

Not seeing how I affected vendor/k8s.io/kubernetes/pkg/controller/node.TestCancel

@deads2k see #14225

@deads2k
Copy link
Contributor Author

deads2k commented May 17, 2017

@deads2k see #14225

Thanks, merged.

re[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to d0e3554

@openshift-bot
Copy link
Contributor

openshift-bot commented May 17, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/676/) (Base Commit: 3d82103) (Image: devenv-rhel7_6235)

@mfojtik
Copy link
Contributor

mfojtik commented May 17, 2017

flake: #13966

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1518/) (Base Commit: d33e176)

@openshift-bot openshift-bot merged commit 71b89dc into openshift:master May 17, 2017
@deads2k deads2k deleted the auth-14-switch branch August 3, 2017 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants