Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Mar 17, 2021

Like the cluster-version operator has been doing since db0d358 (#12789), and which we used to do much more broadly until 6663962 (#12544). I'm personally in favor of the broad approach, because we want minor rollbacks to work, so if we break master->PR rollback, we're in big trouble. But even with the narrow scoping for rollback jobs, origin's test are implementing and monitoring the rollback functionality, so they deserve to have a local test excercising it.

… AWS update jobs

Like the cluster-version operator has been doing since db0d358
(CVO: add rollback tests for 4.6+, 2020-10-14, openshift#12789), and which we
used to do much more broadly until 6663962 (upgrade workflows:
remove abort-at from default TEST_OPTIONS, 2020-10-08, openshift#12544).  I'm
personally in favor of the broad approach, because we want minor
rollbacks to work, so if we break master->PR rollback, we're in big
trouble.  But even with the narrow scoping for rollback jobs, origin's
test are implementing and monitoring the rollback functionality, so
they deserve to have a local test excercising it.

I've added this to the AWS update job, because Clayton wants it to be
optional, and the AWS update job has been optional and not always-run
since it was born in bd85872 (add optional e2e-aws-upgrade job,
2020-08-10, openshift#10785).
@wking wking force-pushed the rollback-origin-job branch from eedf2b1 to e8bc08d Compare March 17, 2021 18:13
@wking wking changed the title ci-operator/config/openshift/origin/openshift-origin-master: Rollback GCP update jobs ci-operator/config/openshift/origin/openshift-origin-master: Rollback AWS update jobs Mar 17, 2021
@wking
Copy link
Member Author

wking commented Mar 17, 2021

With eedf2b1 -> e8bc08d, I've pivoted to use AWS update jobs, which have been optional and not always_run since they were born in #10785, since @smarterclayton wanted this to be on an optional job.

@smarterclayton
Copy link
Contributor

The test binary can't break rollback, it can just break our ability to test rollback, which is a less worrisome condition.

@wking
Copy link
Member Author

wking commented Mar 17, 2021

True. Still helps with things like "have you tested this?".

@smarterclayton
Copy link
Contributor

Yes I want the optional job, just not convinced it's required by default.

@smarterclayton
Copy link
Contributor

Can you make it optional?

@wking
Copy link
Member Author

wking commented Mar 17, 2021

The AWS job I've attached it to now was born optional: true and without always_run, so should already be the way you want it.

@smarterclayton
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton, wking

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

The pull request process is described 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2021
@openshift-merge-robot openshift-merge-robot merged commit 09636c4 into openshift:master Mar 17, 2021
@openshift-ci-robot
Copy link
Contributor

@wking: Updated the ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:

  • key openshift-origin-master.yaml using file ci-operator/config/openshift/origin/openshift-origin-master.yaml
Details

In response to this:

Like the cluster-version operator has been doing since db0d358 (#12789), and which we used to do much more broadly until 6663962 (#12544). I'm personally in favor of the broad approach, because we want minor rollbacks to work, so if we break master->PR rollback, we're in big trouble. But even with the narrow scoping for rollback jobs, origin's test are implementing and monitoring the rollback functionality, so they deserve to have a local test excercising it.

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.

@wking wking deleted the rollback-origin-job branch March 17, 2021 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants