OTA-253: Stubbing out preflight from target release by fao89 · Pull Request #1930 · openshift/enhancements

fao89 · 2026-01-27T11:24:08Z

https://issues.redhat.com/browse/OCPSTRAT-2843

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED

openshift-ci · 2026-01-27T11:24:25Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign pratikmahajan for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

enhancements/update/OWNERS

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

fao89 · 2026-01-27T11:25:45Z

/cc JoelSpeed hongkailiu wking

openshift-ci-robot · 2026-01-27T11:28:46Z

@fao89: This pull request references OTA-253 which is a valid jira issue.

Details

In response to this:

https://issues.redhat.com/browse/OCPSTRAT-2843

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED

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 openshift-eng/jira-lifecycle-plugin repository.

DavidHurta · 2026-01-28T18:20:38Z

/cc

JoelSpeed · 2026-01-29T14:28:06Z

+
+#### Hypershift / Hosted Control Planes
+
+HyperShift is out of scope for now, as we rush to get something tech-preview for standalone.


What's the rush? When are we trying to get this in for?

We're thinking about skip-level updates, starting with standalone clusters. Possibly for 5.0 -> 5.2. One blocker for that would be some kind of guard for 5.0 -> 5.2 updates, to warn 5.0 cluster admins pre-update about anything they needed to sort out before launching an update. We have Upgradeable, but it does not allow a 5.0 controller to distinguish between "sort before updating to 5.1" and "sort before updating to 5.2". We have conditional updates, but some consumers like the labs update tool don't have access to the cluster to know if a particular update risk applies, so they get confusing if we drag a risk along for too long without some way to either resolve it or get a guard baked into shipped product code. This preflight enhancement is trying to fill that gap, and allow the 5.0 cluster to run 5.2 preflights, to warn about the issues that 5.0 Upgradeable and conditional risks don't cover.

In order to GA 5.0 as a launch point for preflights for standalone clusters, we want to be tech-preview in 4.22. Not all that much dev-time left to get that done, which is why we're in a hurry here. If we miss, maybe we can go from zero to GA in 5.0. Or maybe we miss GAing in 5.0, and 5.0 to 5.2 skip-levels don't happen. Or maybe [the required KEP is still alpha, or maybe folks can't convince enough component operators to commit to the amount of API skew or slower API development skip-level would require, or maybe other things. This enhancement (or some alternative) is necessary for skip-level, but certainly not sufficient.

We could do other things for skip-level, like an UpgradeableNextEUS condition. But I don't like the lack-of-granularity in the current Upgradeable condition. Operators might have several, orthogonal concerns, and a single condition doesn't give them the ability to distinguish. Maybe an UpgradeableNextEUS.* regexp condition? Possible, but then folks are still stuck doing the manual-cred check themselves. So I am trying to get some kind of from-the-target-version preflight in place, because it seems like it neatly addresses those other concerns. And I'm trying to get in place quickly, because folks are currently talking about 5.0 as the time it needs to be GA on standalone, and I realize that GA features take time. But 🤷, I also realize that tech-preview features take time, and if we're already too late in 4.22 to get this in, the earlier folks tell me, the earlier I can start trying to explain the side-effects to the folks who had been pushing that skip-level timing.

Ok, context of this needs to be ready in 5.0 to support skip level makes sense, might want to clarify that in this statement. I'd also outline what risks we have of ignoring hypershift for now, are we likely to design something here that doesn't work for hypershift later?

... are we likely to design something here that doesn't work for hypershift later?

Hopefully not 😅 HyperShift already runs CVO as a child Deployment, so a similar pattern their seems like it should be possible, although obviously it will depend on HyperShift updating their HostedCluster and HostedControlPlane CRDs to make more room for the new ClusterVersion properties behind this effort. Or for folks to plan out some alternative API for carrying that information, if for some reason folks think lifting up the ClusterVersion properties wouldn't work in HyperShift-land. But if anyone does have concerns about boxing ourselves in vs. HyperShift that we can try to address in this enhancement, I'm happy to chip in trying to respond to those concerns and hopefully we can find a place where folks feel like we're sufficiently safe.

@enxebre @csrwng Would either of you be able to skim this to provide an initial temperature on concern for the fact we are ignoring HyperShift in the initial iteration?

From a technical perspective, in hcp the cvo is not the sole authority for upgrades, the cpo also manages the lifecycle of control plane components. Have we give some thought about how we would reconcile this split for this feature?

From a product perspective, not supporting HCP means leaving behind ARO, ROSA, and self-hosted multi-cluster, which are growing consumption models for OpenShift. I'm personally not opposed to getting there gradually though it seems we might want to have a clear technical path at least and timeline

Have we give some thought about how we would reconcile this split for this feature?

I would be very happy to have the control-plane operator absorb this new information from ClusterVersion and integrate it into its own HostedControlPlane status update opinions (where today it's just blindly passing along the ClusterVersion properties). I'm not clear on whether there's capacity among HyperShift maintainers to work out how they want that kind of layering to work.

Another option I'd be very happy with would be to make the CVO HyperShift-aware, and have the HostedControlPlane controller hand off more responsibility to that HyperShift-aware CVO (OTA-951). I haven't heard much out of either my own updates team or the HyperShift team about prioritizing that lift though.

So while I would prefer to see either of those options land to provide a more HyperShift-aware update planning UX, at the moment my expectation is that we just continue to roll with the existing blindly-pass-along-ClusterVersion-properties approach, and we ignore the fact that that doesn't leave much room for HyperShift-specific update advice. Until it bothers someone, and if that ever happens, maybe we can leverage the pain into prioritizing the tighter integration work.

And I expect that blind-copy work to go tech-preview into HyperShift in 5.0 or some such, and am just excluding it from 4.22 because we only have a few more weeks to even get this into 4.22 standalone before it's pencils down.

JoelSpeed · 2026-01-29T17:42:46Z

+When the target release CVO is invoked with the `preflight --format=preflight-v1-json` argument, it runs preflight checks, and reports the results to the cluster's running CVO via a host-mounted volume (just like the `version-*` Pod FIXME https://github.com/openshift/cluster-version-operator/blob/83243780aed4e0d9c4ebff528e54b918d4170fd3/pkg/cvo/updatepayload.go#L271-L275).
+The results will be JSON.
+Because they will be [propagated into `conditionalUpdateRisks`](#retrieving-preflight-check-results), we'll use that structure:


Why doesn't it update the status directly?

Two controllers (current CVO and preflight CVO) trying to share a single property-space can cause write conflicts. Sharing is hard. By maintaining a clear preflight-CVO -> current-CVO -> ClusterVersion status pipeline, there's no need to sort out a sharing plan.

We have API tooling specifically to deal with this (see applyconfiguration), but perhaps the existing CVO code is too set as it is to update to be compatible with this newer pattern?

https://pkg.go.dev/k8s.io/client-go/applyconfigurations says the overall approach is based on server-side apply, and https://pkg.go.dev/github.com/openshift/client-go/config/applyconfigurations/config/v1#ClusterVersionStatusApplyConfiguration.WithConditionalUpdates gives a way to append conditional updates. But I don't understand how either server-side apply or the current OpenShift client-go code could be used to get two controllers to convenienetly share a single map/slice. For example, even as a single writer, how would you use the apply-configuration API to distinguish between "append this new conditional update" and "append this new one, but also clear away any old, stale entries that I used to think were relevant"?

Generally the complex scenario is sharing a list, for a list to be shareable between two entities, it must be marked up as +listType=map, and have some sort of unique identifier (or combination of identifiers) with that is set as the +listMapKey.

Once you've done this, each writer has the opportunity to write to the list with their own opinion of the content of the list. So writer A (which will be called manager A in managedFields) writes the list with entries

- name: Foo - name: Bar

Each time manager A writes to this list, they must send their entire opinion. If they omit the Bar entry next write, it gets removed, if they write with that list empty, all of their entiries get removed. If they want to append, they must write Foo, Bar and their new entry

Write B can come along and they write the list as

- name: Baz - name: Qux

A reader reading the list would then see

- name: Foo - name: Bar - name: Baz - name: Qux

If at any point two managers (A/B) tried to write an object with the same key (name in this example), then a conflict would arise and the write would be rejected. Though writers can force their way through this saying no, I absolutely own this opinion.

In the example where we had two controllers writing to that same status but not sharing a field, ie one writes to status.foo and the other writes to status.bar. As long as both are well behaved (using apply type patches) and not the traditional Update approach, then you'd see managed fields entries showing each owning the bits they care about. Anyone writing with an update would be equivalent to forcing ownership of the entire object and would be expected to do client side merging as many controllers do today

Implements preflight update mode that allows checking update compatibility without committing to performing the actual cluster update. Changes: - Add optional 'mode' field to Update struct with value "Preflight" - Gate new field behind ClusterUpdatePreflight feature (TechPreviewNoUpgrade) - When mode="Preflight", cluster runs compatibility checks only - When mode omitted, normal update behavior is preserved Related: openshift/enhancements#1930 Signed-off-by: Fabricio Aguiar <fabricio.aguiar@gmail.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED

fao89 · 2026-02-09T14:59:35Z

/cc enxebre csrwng

Implements preflight update mode that allows checking update compatibility without committing to performing the actual cluster update. Changes: - Add optional 'mode' field to Update struct with value "Preflight" - Gate new field behind ClusterUpdatePreflight feature (TechPreviewNoUpgrade) - When mode="Preflight", cluster runs compatibility checks only - When mode omitted, normal update behavior is preserved Related: openshift/enhancements#1930 Signed-off-by: Fabricio Aguiar <fabricio.aguiar@gmail.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED

jiajliu · 2026-03-04T03:35:54Z

+```bash
+oc adm upgrade --status-preflight
+```
+Enhanced to display preflight execution status when active, including target version and completion progress.


whether it shows the summary of the risks?

I would expect oc adm upgrade recommend to be where we talk about both preflight levels of preflight results:

Whether a CVO thinks any requested preflight-mode tooling is running smoothly. If we add new conditions to ClusterVersion to talk about how the tooling is going in this effort, this would be new recommend CLI work as a result of this enhancement.

Any risks the CVO thinks are relevant to updates, including those detected by the preflight-risk logic. Nothing new for this enhancement here, you can just draft behind the work OTA-1544: update: support accepted risks #1807 aready committed to.

make sense, it's more clear to split the responsibilities between status-preflight and recommend.

jiajliu · 2026-03-04T03:51:55Z

+
+**Result completeness indicators:**
+
+The `reason` field in risk conditions clearly indicates the nature of preflight results:


I noticed the preflight JSON uses an executionStatus field with values "running|failed|completed"(see https://github.com/openshift/enhancements/pull/1930/changes#diff-8829175143337c6675c8d93640509187ac5ecdd054dad4b6f8e600998eef17b6R442), while risk condition reasons include four values (PreflightRunning, PreflightValidation, PreflightPartial, PreflightFailed). My understanding is that PreflightPartial and PreflightValidation are both mapping to executionStatus="completed", could you confirm if that mapping is correct, or should executionStatus be extended (e.g. to include "partial")?

yes, it is correct!
But I can see it is confusing, I'll try to think of better reasons names

JoelSpeed · 2026-03-05T12:13:17Z

+
+* **Operator-level preflight framework**: This enhancement focuses on cluster-level preflight orchestration.
+    Individual operator preflight implementations are out of scope (those would be developed separately by component teams).
+    * For the initial enhancement, even the cluster-level interface between the target-release CVO and the target-release operators is out of scope.


This line contradicts the next doesn't it?

Implements preflight update mode that allows checking update compatibility without committing to performing the actual cluster update. Changes: - Add optional 'mode' field to Update struct with value "Preflight" - Gate new field behind ClusterUpdatePreflight feature (TechPreviewNoUpgrade) - When mode="Preflight", cluster runs compatibility checks only - When mode omitted, normal update behavior is preserved Related: openshift/enhancements#1930 Signed-off-by: Fabricio Aguiar <fabricio.aguiar@gmail.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED

openshift-bot · 2026-04-03T01:15:47Z

Inactive enhancement proposals go stale after 28d of inactivity.

See https://github.com/openshift/enhancements#life-cycle for details.

Mark the proposal as fresh by commenting /remove-lifecycle stale.
Stale proposals rot after an additional 7d of inactivity and eventually close.
Exclude this proposal from closing by commenting /lifecycle frozen.

If this proposal is safe to close now please do so with /close.

/lifecycle stale

3b365d0 (OTA-1813: Populate risks from alerts, 2026-02-27, openshift#1329) added the alert-risk logic. But the implementation in that commit was fairly directly integrated into pkg/cvo. We want to extend update-risk detection with additional providers in the future, like preflights from the target release [1], and that could get complicated with logic tucked into pkg/cvo. And pkg/cvo is already plenty complicated, even without this functionality inline. This commit refactors to remove a few hundred lines of code from pkg/cvo out to a new pkg/risk. And that allows the pkg/cvo handling to move from "I'm integrating additional risks from alerts" to "I'm integrating additional risks from a source". As we more forward from here, we can continue to extend the functionality of the source, and will only have to tweak the source in cvo.go's New(...). Most of the code is either straightforward interface work or lifted-and-shifted pre-existing code. But the ReplaceAll for U+00A0 No-break space took some time to work out, so I'll explain that in detail here. It flies in the face of [1]: Do not depend on this output being stable. If you need the ability to programmatically interpret the difference, consider using a custom Reporter. I'm manually unwinding the uncertainty they inject in github.com/google/go-cmp/cmp /report_text.go's indentMode.appendIndent, because: * I want to test the error message content, to ensure it's sufficiently readable. * I'm ok manually updating the test expectations if a go-cpm bump brings in rendering improvements. That manual update will give us space to review to ensure we're still readable. [1]: openshift/enhancements#1930 [2]: https://pkg.go.dev/github.com/google/go-cmp/cmp#Diff

3b365d0 (OTA-1813: Populate risks from alerts, 2026-02-27, openshift#1329) added the alert-risk logic. But the implementation in that commit was fairly directly integrated into pkg/cvo. We want to extend update-risk detection with additional providers in the future, like preflights from the target release [1], and that could get complicated with logic tucked into pkg/cvo. And pkg/cvo is already plenty complicated, even without this functionality inline. This commit refactors to remove a few hundred lines of code from pkg/cvo out to a new pkg/risk. And that allows the pkg/cvo handling to move from "I'm integrating additional risks from alerts" to "I'm integrating additional risks from a source". As we more forward from here, we can continue to extend the functionality of the source, and will only have to tweak the source in cvo.go's New(...). Most of the code is either straightforward interface work or lifted-and-shifted pre-existing code. But the ReplaceAll for U+00A0 No-break space took some time to work out, so I'll explain that in detail here. It flies in the face of [2]: Do not depend on this output being stable. If you need the ability to programmatically interpret the difference, consider using a custom Reporter. I'm manually unwinding the uncertainty they inject in github.com/google/go-cmp/cmp /report_text.go's indentMode.appendIndent, because: * I want to test the error message content, to ensure it's sufficiently readable. * I'm ok manually updating the test expectations if a go-cpm bump brings in rendering improvements. That manual update will give us space to review to ensure we're still readable. [1]: openshift/enhancements#1930 [2]: https://pkg.go.dev/github.com/google/go-cmp/cmp#Diff

fao89 · 2026-04-09T07:51:25Z

/remove-lifecycle stale

3b365d0 (OTA-1813: Populate risks from alerts, 2026-02-27, openshift#1329) added the alert-risk logic. But the implementation in that commit was fairly directly integrated into pkg/cvo. We want to extend update-risk detection with additional providers in the future, like preflights from the target release [1], and that could get complicated with logic tucked into pkg/cvo. And pkg/cvo is already plenty complicated, even without this functionality inline. This commit refactors to remove a few hundred lines of code from pkg/cvo out to a new pkg/risk. And that allows the pkg/cvo handling to move from "I'm integrating additional risks from alerts" to "I'm integrating additional risks from a source". As we more forward from here, we can continue to extend the functionality of the source, and will only have to tweak the source in cvo.go's New(...). Most of the code is either straightforward interface work or lifted-and-shifted pre-existing code. But the ReplaceAll for U+00A0 No-break space took some time to work out, so I'll explain that in detail here. It flies in the face of [2]: Do not depend on this output being stable. If you need the ability to programmatically interpret the difference, consider using a custom Reporter. I'm manually unwinding the uncertainty they inject in github.com/google/go-cmp/cmp /report_text.go's indentMode.appendIndent, because: * I want to test the error message content, to ensure it's sufficiently readable. * I'm ok manually updating the test expectations if a go-cpm bump brings in rendering improvements. That manual update will give us space to review to ensure we're still readable. [1]: openshift/enhancements#1930 [2]: https://pkg.go.dev/github.com/google/go-cmp/cmp#Diff

https://issues.redhat.com/browse/OCPSTRAT-2843 Signed-off-by: Fabricio Aguiar <fabricio.aguiar@gmail.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED

Signed-off-by: Fabricio Aguiar <fabricio.aguiar@gmail.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED

openshift-ci · 2026-04-14T16:32:56Z

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

openshift-ci bot requested review from PratikMahajan and petr-muller January 27, 2026 11:24

openshift-ci bot requested review from JoelSpeed, hongkailiu and wking January 27, 2026 11:25

fao89 changed the title ~~Stubbing out preflight from target release~~ OTA-253: Stubbing out preflight from target release Jan 27, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 27, 2026

fao89 force-pushed the preflight-from-target-release branch from 5dac841 to f201cd1 Compare January 27, 2026 12:00

openshift-ci bot requested a review from DavidHurta January 28, 2026 18:20

JoelSpeed reviewed Jan 29, 2026

View reviewed changes

fao89 force-pushed the preflight-from-target-release branch from f201cd1 to 496ddef Compare January 30, 2026 13:06

fao89 mentioned this pull request Jan 30, 2026

OTA-253: Add cluster update preflight mode API openshift/api#2684

Merged

wking reviewed Feb 3, 2026

View reviewed changes

Comment thread enhancements/update/preflight-from-target-release.md

wking reviewed Feb 3, 2026

View reviewed changes