-
Notifications
You must be signed in to change notification settings - Fork 462
Bug 1762565: pkg/controller/template: avoid resyncing templates if the cc didn't change #1182
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
Bug 1762565: pkg/controller/template: avoid resyncing templates if the cc didn't change #1182
Conversation
…hange There are two main reasons for this patch. The first one is obivious and avoids spending time retemplating when there's no need since the ControllerConfig's spec hasn't changed. The second reason is king of tricky but what can happen is the following: 0) between 4.1 and 4.2 MCO changed the etcd-member image name placeholder from `setupEtcdEnv` to `setupEtcdEnvKey` (along with other changes to image names) 1) when an upgrade from 4.1 starts the following happens: a) the new MCO is rolled out b) the new MCC is rolled out (note the template controller is also rolled out here and can start before the new CC is rolled out with the new image name placeholder!) c) RACE! the template controller starts running before the new ControllerConfig with the new image name placeholder is created d) the MCC generates templates w/o the image name for the etcd member e)_cluster dies The patch does a simple thing but ensures that the above scenario isn't hit since we're not going to retemplate anymore if the CC hasn't changed (yet). Signed-off-by: Antonio Murdaca <runcom@linux.com>
|
@openshift-cherrypick-robot: No Bugzilla bug is referenced in the title of this pull request. DetailsIn response to this:
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. |
|
/lgtm |
|
/approve |
|
@smarterclayton for override if we hit the gcp upgrade issue. |
kikisdeliveryservice
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kikisdeliveryservice, mrunalp, openshift-cherrypick-robot, rphillips The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@openshift-cherrypick-robot: This pull request references Bugzilla bug 1762565, which is invalid:
Comment DetailsIn response to this:
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. |
|
Overriding BZ bot. Given the failure wont be reproducible in a 4.2 -> 4.3 upgrade, we cant verify it on master. |
|
@openshift-cherrypick-robot: All pull requests linked via external trackers have merged. Bugzilla bug 1762565 has been moved to the MODIFIED state. DetailsIn response to this:
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. |
This is an automated cherry-pick of #1177
/assign rphillips