Skip to content

Conversation

@hexfusion
Copy link
Contributor

dupe of #1177 for additional CI cycles.

…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 <[email protected]>
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 16, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hexfusion
To complete the pull request process, please assign ericavonb
You can assign the PR to them by writing /assign @ericavonb in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hexfusion
Copy link
Contributor Author

/retest

1 similar comment
@kikisdeliveryservice
Copy link
Contributor

/retest

@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Oct 16, 2019

Due to how ci is setup this is just mirroring the same test that is running in 1777. Basically you can't run 2 PRs on the same commit, if you do only 1 test run will happen and they will link to the same ci run.

@kikisdeliveryservice
Copy link
Contributor

You can make a small change to get a new commit for the changes and push to get a new run.

@openshift-ci-robot
Copy link
Contributor

@hexfusion: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-gcp-upgrade 7e666cc link /test e2e-gcp-upgrade
ci/prow/e2e-aws 7e666cc link /test e2e-aws
ci/prow/e2e-gcp-op 7e666cc link /test e2e-gcp-op
ci/prow/e2e-aws-scaleup-rhel7 7e666cc link /test e2e-aws-scaleup-rhel7

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

@kikisdeliveryservice
Copy link
Contributor

1177 is going to merge, closing this now..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants