Skip to content

WIP: Fix desired before sync_worker's work is initialized - #1126

Closed
wking wants to merge 6 commits into
openshift:release-4.13from
wking:desired-before-sync_worker-work-initialized
Closed

WIP: Fix desired before sync_worker's work is initialized#1126
wking wants to merge 6 commits into
openshift:release-4.13from
wking:desired-before-sync_worker-work-initialized

Conversation

@wking

@wking wking commented Dec 11, 2024

Copy link
Copy Markdown
Member

Do not merge; I'm picking #1109 back to 4.13 to see if OCPBUGS-22266 can be closed as a dup of this series.

If the CVO pod gets restarted after a user sends a cluster
upgrade request with `--to-image`, the desired should not ALWAYS be
taken from `.Spec.DesiredUpdate`, e.g., when the targeting payload is blocked
on some precondition check failure.

The user request will be written to `cv.spec.desiredUpdate` where only
the `image` field is set, i.e., the `Version` field is empty.

If the CVO gets restarted, then

1. The desied [1] from `cv.spec.desiredUpdate` will be used to set
   `w.status.Actual` [2] and then applied to `cv.status.desired`.
2. When the next `optr.sync()` happens, the precondition check may become
   non-blocking [3] as the `Version` field is empty now.
3. CVO will then start an upgrade that should have been blocked.

[1]. https://github.com/openshift/cluster-version-operator/blob/1995380b6d755c29b926b846a64ca0039002c2cf/pkg/cvo/cvo.go#L680
[2]. https://github.com/openshift/cluster-version-operator/blob/1995380b6d755c29b926b846a64ca0039002c2cf/pkg/cvo/sync_worker.go#L502
[3]. https://github.com/openshift/cluster-version-operator/blob/1995380b6d755c29b926b846a64ca0039002c2cf/pkg/payload/precondition/clusterversion/rollback.go#L57
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 11, 2024
@openshift-ci

openshift-ci Bot commented Dec 11, 2024

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 Dec 11, 2024
@openshift-ci

openshift-ci Bot commented Dec 11, 2024

Copy link
Copy Markdown
Contributor

@wking: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agnostic-ovn 10a77cd link true /test e2e-agnostic-ovn
ci/prow/e2e-agnostic-ovn-upgrade-into-change 10a77cd link true /test e2e-agnostic-ovn-upgrade-into-change
ci/prow/e2e-agnostic-ovn-upgrade-out-of-change 10a77cd link true /test e2e-agnostic-ovn-upgrade-out-of-change
ci/prow/e2e-agnostic-operator 10a77cd link true /test e2e-agnostic-operator
ci/prow/images 10a77cd link true /test images

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

@wking

wking commented Dec 11, 2024

Copy link
Copy Markdown
Member Author

Dup-ness confirmed. Closing.

@wking wking closed this Dec 11, 2024
@wking
wking deleted the desired-before-sync_worker-work-initialized branch December 11, 2024 01:33
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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants