simplify controller wiring #14333
simplify controller wiring #14333
Conversation
|
[test] |
openshift-bot
added
the
needs-rebase
| saTokens.ServiceServingCA = append(saTokens.ServiceServingCA, serviceServingCA...) | ||
| } | ||
| ret["serviceaccount-tokens"] = saTokens.RunController | ||
| // this matches the upstream name |
There was a problem hiding this comment.
can we rename the controller as well?
There was a problem hiding this comment.
can we rename the controller as well?
doing.
|
|
||
| ret["serviceaccount-pull-secrets"] = controller.RunServiceAccountPullSecretsController | ||
| ret["origin-namespace"] = controller.RunOriginNamespaceController | ||
| ret["openshift.io/serviceaccount-pull-secrets"] = controller.RunServiceAccountPullSecretsController |
There was a problem hiding this comment.
openshift.io/controllers/ ?
how about some generator with:
ret.AddOpenShiftController(controller.RunServiceAccount...)
ret.AddKubeController(...)
return ret.InitializersMap()
too much?
There was a problem hiding this comment.
openshift.io/controllers/ ?
how about some generator with:
ret.AddOpenShiftController(controller.RunServiceAccount...)
ret.AddKubeController(...)return ret.InitializersMap()
too much?
I think we actually end up creating a method to create NewKubeControllerInitializers and then one method that does NewAllControllerInitializers or something. That will help us prep for splitting apart.
|
2 small nits, LGTM after rebase and green test :-) |
deads2k
force-pushed
the
controller-06-wire
branch
from
6fe293e to
f73a372
Compare
|
comments addressed. [merge] |
deads2k
removed
the
needs-rebase
deads2k
force-pushed
the
controller-06-wire
branch
from
f73a372 to
4e4253f
Compare
|
Evaluated for origin test up to 4e4253f |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1979/) (Base Commit: 5cfbdc3) |
|
Evaluated for origin merge up to 4e4253f |
|
continuous-integration/openshift-jenkins/merge FAILURE (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/942/) (Base Commit: c9a1d10) |
|
included in #14317 |
4 participants
This builds on @mfojtik's pull to simplify the wiring and reduce us to a single list.
@enj if you have time to review before dcut, you'll thank yourself