deploy: collapse deployer into deployments controller #9691
Conversation
|
Change looks ok, why not delete the old code though? |
|
I will remove the controller in a follow-up |
Removed in a separate commit here |
|
[test] |
|
#9129, [test] |
|
[test] |
|
@ironcladlou can you help me do a pass here? |
|
Will take a look |
|
This LGTM me too, more awesome work as usual. Is there any intent to do another level of collapsing of this into the deploymentConfigController (using the same strategy to enqueue DCs on RC update) or would that make the controller too unwieldy? |
I think we will always need the deployment controller and we may avoid precreating replica sets for upstream deployments, if we have a separate path in this controller that handles/tags appropriately those replica sets. |
|
Evaluated for origin test up to 84e1faf |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6007/) |
|
@smarterclayton any other comments here? I will update the deployment controller to start retrying on update conflicts in a follow-up. |
|
No, [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6007/) (Image: devenv-rhel7_4567) |
|
Evaluated for origin merge up to 84e1faf |
@smarterclayton I wasn't hot about finishing this but the thing is that the deployer pod controller currently uses its own pod and rc caches whereas the deployment controller is sharing, a possible source for races?
Closes #9296
[test]
@ironcladlou @mfojtik fyi