Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented May 31, 2023

The job flavor was originally added in 0837634 (#16260). The jobs have subsequently been cloned forward to new minors as part of the branching process. And as older jobs started failing, I'd been dropping them gradually like 856aab2 (#33005). But rounding with @jluhrsen, the jobs no longer serve a useful role, and as 856aab2 points out, rollbacks between minor releases are not supported. Drop the likely-to-fail and not-useful-even-when-it-passes jobs in their entirety, so they stop getting cloned forward during branching.

@wking wking changed the title ci-operator/config/openshift/release: Drop failing cross-minor rollback jobs ci-operator/config/openshift/release: Drop cross-minor rollback jobs May 31, 2023
@openshift-ci-robot openshift-ci-robot added the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label May 31, 2023
@openshift-ci openshift-ci bot requested review from stbenjam and vrutkovs May 31, 2023 17:17
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 31, 2023
@jluhrsen
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 31, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD e120d02 and 2 for PR HEAD 1663941fa8587aa6534872dcf73f3fdf13d8fc4d in total

@stbenjam
Copy link
Member

stbenjam commented May 31, 2023

Some wires are getting crossed because someone just made these informers, but I had my doubts about the usefulness of doing that.

#39488

@wking
Copy link
Member Author

wking commented May 31, 2023

Ah, thanks for the pointer. I think folks are conflating the various jobs with rollback in their name. The ones #39488 should have focused on are the upgrade-rollback-oldest-supported jobs.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 09d4cc8 and 1 for PR HEAD 1663941fa8587aa6534872dcf73f3fdf13d8fc4d in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 8f4925f and 0 for PR HEAD 1663941fa8587aa6534872dcf73f3fdf13d8fc4d in total

@jluhrsen
Copy link
Contributor

/lgtm cancel

need to remove these jobs in release-controller. here's the 4.12 example

I think make release-controller locally will catch it for you

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 31, 2023
The job flavor was originally added in 0837634 (Add
ovn-upgrade-rollback job for 4.7->4.8, 2021-02-24, openshift#16260).  The jobs
have subsequently been cloned forward to new minors as part of the
branching process.  And as older jobs started failing, I'd been
dropping them gradually like 856aab2
(ci-operator/config/openshift/release/openshift-release-master__ci-4.11-upgrade-from-stable-4.10:
Drop failing rollback jobs, 2022-10-11, openshift#33005).  But rounding with
Jamo, the jobs no longer serve a useful role, and as 856aab2 points
out, rollbacks between minor releases are not supported.  Drop the
likely-to-fail and not-useful-even-when-it-passes jobs in their
entirety, so they stop getting cloned forward during branching.

I'm also adjusting the release controller changes from 421c921
(Introducing Rollback informing jobs, 2023-05-19, openshift#39488).  I'm
dropping 4.12 and earlier rollback informers, so we can focus on 4.13
while we feel out the new process.  And I'm pivoting 4.13 away from
the cross-minor job that this pull request drops, and towards the
rollback-oldest-supported job that will help back [1].

[1]: https://issues.redhat.com/browse/OTA-455
@wking wking force-pushed the drop-cross-minor-rollback-jobs branch from 1663941 to c5c89bf Compare June 1, 2023 18:01
@openshift-ci openshift-ci bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 1, 2023
Copy link
Contributor

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2023
@vrutkovs
Copy link
Contributor

vrutkovs commented Jun 6, 2023

Seems it needs another make update though

Because [1]:

  ERROR: The following differences were found:
  3a4
  > 03c544e5d55a55ae9f19d0de7d786341  .//core-services/release-controller/_releases/priv/release-ocp-4.12.json
  35d35
  < 1826a1b520574b66f152f814811c19f6  .//core-services/release-controller/_releases/priv/release-ocp-4.13.json
  42a43
  ...

tells me what files need changing, but not what changes to make to them.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/39897/pull-ci-openshift-release-master-release-controller-config/1664331471080394752
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2023
@openshift-ci-robot
Copy link
Contributor

[REHEARSALNOTIFIER]
@wking: no rehearsable tests are affected by this change

Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 10 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 20 rehearsals
Comment: /pj-rehearse max to run up to 35 rehearsals
Comment: /pj-rehearse auto-ack to run up to 10 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse abort to abort all active rehearsals

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 6, 2023

@wking: all tests passed!

Full PR test history. Your PR dashboard.

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.

Copy link
Contributor

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2023
@vrutkovs
Copy link
Contributor

vrutkovs commented Jun 7, 2023

/approve

@bear-redhat
Copy link
Contributor

/aprpove

@bear-redhat
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bear-redhat, vrutkovs, 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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2023
@openshift-merge-robot openshift-merge-robot merged commit 5e746a7 into openshift:master Jun 7, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 7, 2023

@wking: Updated the following 2 configmaps:

  • ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-release-master__ci-4.12-upgrade-from-stable-4.11.yaml using file ci-operator/config/openshift/release/openshift-release-master__ci-4.12-upgrade-from-stable-4.11.yaml
    • key openshift-release-master__ci-4.13-upgrade-from-stable-4.12.yaml using file ci-operator/config/openshift/release/openshift-release-master__ci-4.13-upgrade-from-stable-4.12.yaml
    • key openshift-release-master__ci-4.14-upgrade-from-stable-4.13.yaml using file ci-operator/config/openshift/release/openshift-release-master__ci-4.14-upgrade-from-stable-4.13.yaml
  • job-config-master-periodics configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-release-master-periodics.yaml using file ci-operator/jobs/openshift/release/openshift-release-master-periodics.yaml
Details

In response to this:

The job flavor was originally added in 0837634 (#16260). The jobs have subsequently been cloned forward to new minors as part of the branching process. And as older jobs started failing, I'd been dropping them gradually like 856aab2 (#33005). But rounding with @jluhrsen, the jobs no longer serve a useful role, and as 856aab2 points out, rollbacks between minor releases are not supported. Drop the likely-to-fail and not-useful-even-when-it-passes jobs in their entirety, so they stop getting cloned forward during branching.

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 drop-cross-minor-rollback-jobs branch June 7, 2023 16:19
jtaleric pushed a commit to jtaleric/release that referenced this pull request Jun 9, 2023
…penshift#39897)

* ci-operator/config/openshift/release: Drop cross-minor rollback jobs

The job flavor was originally added in 0837634 (Add
ovn-upgrade-rollback job for 4.7->4.8, 2021-02-24, openshift#16260).  The jobs
have subsequently been cloned forward to new minors as part of the
branching process.  And as older jobs started failing, I'd been
dropping them gradually like 856aab2
(ci-operator/config/openshift/release/openshift-release-master__ci-4.11-upgrade-from-stable-4.10:
Drop failing rollback jobs, 2022-10-11, openshift#33005).  But rounding with
Jamo, the jobs no longer serve a useful role, and as 856aab2 points
out, rollbacks between minor releases are not supported.  Drop the
likely-to-fail and not-useful-even-when-it-passes jobs in their
entirety, so they stop getting cloned forward during branching.

I'm also adjusting the release controller changes from 421c921
(Introducing Rollback informing jobs, 2023-05-19, openshift#39488).  I'm
dropping 4.12 and earlier rollback informers, so we can focus on 4.13
while we feel out the new process.  And I'm pivoting 4.13 away from
the cross-minor job that this pull request drops, and towards the
rollback-oldest-supported job that will help back [1].

[1]: https://issues.redhat.com/browse/OTA-455

* hack/validate-release-controller-config: Supplemental Git diff

Because [1]:

  ERROR: The following differences were found:
  3a4
  > 03c544e5d55a55ae9f19d0de7d786341  .//core-services/release-controller/_releases/priv/release-ocp-4.12.json
  35d35
  < 1826a1b520574b66f152f814811c19f6  .//core-services/release-controller/_releases/priv/release-ocp-4.13.json
  42a43
  ...

tells me what files need changing, but not what changes to make to them.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/39897/pull-ci-openshift-release-master-release-controller-config/1664331471080394752

---------

Co-authored-by: wking <wking@penguin>
stbenjam added a commit to stbenjam/release that referenced this pull request Jun 13, 2023
openshift#39897 didn't run `make
release-controllers`.
wking added a commit to wking/openshift-release that referenced this pull request Oct 4, 2023
…y-4.14-upgrade-from-stable-4.13: Restore cross-minor rollbacks

We'd dropped the last of these in 856aab2
(ci-operator/config/openshift/release/openshift-release-master__ci-4.11-upgrade-from-stable-4.10:
Drop failing rollback jobs, 2022-10-11, openshift#33005) and 5e746a7
(ci-operator/config/openshift/release: Drop cross-minor rollback jobs,
2023-06-07, openshift#39897).  There's now renewed interest in how these sorts
of rollbacks look, so I'm reviving them for recent releases.  I expect
the issues with these rollbacks will at least include issues with the
cluster-version operator losing the ability to write to ClusterVersion
as the older CRD's enum rejects the capabilities added in the new
release:

  openshift/api $ git diff origin/release-4.13..origin/release-4.14 -- config/v1/types_cluster_version.go | grep kubebuilder:validation:Enum
  -// +kubebuilder:validation:Enum=openshift-samples;baremetal;marketplace;Console;Insights;Storage;CSISnapshot;NodeTuning
  +// +kubebuilder:validation:Enum=openshift-samples;baremetal;marketplace;Console;Insights;Storage;CSISnapshot;NodeTuning;MachineAPI;Build;DeploymentConfig;ImageRegistry
  -// +kubebuilder:validation:Enum=None;v4.11;v4.12;v4.13;vCurrent
  +// +kubebuilder:validation:Enum=None;v4.11;v4.12;v4.13;v4.14;vCurrent

So a cluster updating from 4.13 to 4.14 will enable (possibly
implicitly) MachineAPI and other newly-labeled-in-4.14 capabilities.
And then when the 4.13 ClusterVersion CRD is pushed during the
rollback, those values become illegal, and the Kubernetes API server
will reject the cluster-version operators attempts to write
ClusterVersion status with errors complaining about the unrecognised
MachineAPI and other capability string [1]:

  $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-version-operator/941/pull-ci-openshift-cluster-version-operator-master-e2e-agnostic-ovn-upgrade-out-of-change/1671502401497993216/artifacts/e2e-agnostic-ovn-upgrade-out-of-change/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-7fd84b7b99-8b2qk_cluster-version-operator.log | grep 'ClusterVersion.config.openshift.io "version" is invalid' | tail -n1
  I0621 16:45:41.154360       1 cvo.go:601] Error handling openshift-cluster-version/version: ClusterVersion.config.openshift.io "version" is invalid: status.capabilities.enabledCapabilities[3]: Unsupported value: "MachineAPI": supported values: "openshift-samples", "baremetal", "marketplace", "Console", "Insights", "Storage", "CSISnapshot", "NodeTuning"

[1]: openshift/cluster-version-operator#941 (review)
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. rehearsals-ack Signifies that rehearsal jobs have been acknowledged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants