-
Notifications
You must be signed in to change notification settings - Fork 591
Comments
OTA-253: Add cluster update preflight mode API#2684
OTA-253: Add cluster update preflight mode API #2684fao89 wants to merge 1 commit intoopenshift:master from
Conversation
openshift-ci-robot
commented
Jan 30, 2026
Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.
For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.
This repository is configured in: LGTM mode
@fao89: This pull request references OTA-253 which is a valid jira issue.
Details
In response to this:
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
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.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new update mode "Preflight" by introducing the UpdateModePolicy type and a Mode field on Update, registers the ClusterUpdatePreflight feature gate, updates OpenAPI and Swagger docs to include the new mode, inserts the gate into feature-gate payloads and documentation, and adds ClusterVersion preflight test configurations. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Hello @fao89! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.
|
@fao89: This pull request references OTA-253 which is a valid jira issue. DetailsIn response to this:
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. |
PR Compliance Guide 🔍
Below is a summary of compliance checks for this PR:
No security concerns identified
No security vulnerabilities detected by AI analysis. Human verification advised for critical code.🎫 No ticket provided
- Create ticket/issue
Codebase context is not defined
Follow the guide to enable codebase context checks.
Generic: Meaningful Naming and Self-Documenting Code
Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Secure Error Handling
Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Secure Logging Practices
Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Security-First Input Validation and Data Handling
Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Comprehensive Audit Trails
Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.
Status:
Audit logging unclear: The diff adds a new administrative action surface (desiredUpdate.mode: Preflight) but does
not show whether requests to use this mode are audit-logged with actor/context and
outcome.
Referred Code
// mode allows an update to be checked for compatibility without committing to updating the cluster. // Allowed values are "Preflight" and omitted. // When omitted, the default mode allows existing clients to request normal updates. // When set to "Preflight", the cluster will run compatibility checks against the target release // without performing an actual update. // +openshift:enable:FeatureGate=ClusterUpdatePreflight // +kubebuilder:validation:Enum=Preflight // +optional Mode UpdateModePolicy `json:"mode,omitempty"`
Learn more about managing compliance generic rules or creating your own custom rules
- Update
Compliance status legend
🟢 - Fully Compliant🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||
[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 everettraven 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:Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
8c3f649 to
b2750d9
Compare
b2750d9 to
fb4267a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@openapi/generated_openapi/zz_generated.openapi.go`:
- Around line 21484-21490: The OpenAPI schema for the "mode" property currently
allows any string; add an enum constraint so only "Preflight" is permitted by
updating the source validation annotation that defines this field (so the
generator emits Enum: ["Preflight"]) and then regenerate the OpenAPI output;
specifically ensure the generated SchemaProps for "mode" includes an Enum entry
(e.g., Enum: []interface{}{"Preflight"}) rather than only Type/Description, by
changing the validation/tag on the source field that corresponds to the "mode"
schema before running the generator.
config/v1/types_cluster_version.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I can see from the slack thread, the documentation should reflect that preflight results appear in status.conditionalUpdates:
conditional update risks becomes a union of things coming from the update graph and pre flight checks
How about something like:
We have to:
- Explicitly state WHERE results appear (status.conditionalUpdates)
- Explain the union behavior (update graph + preflight checks)
- Mention skip-level use case (primary motivation from enhancement PR)
fb4267a to
56aa358
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+enum adds an Enum constraint to the OpenAPI schema and appends auto-generated enum documentation to the description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those are basic CRUD tests and can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add transition tests:
56aa358 to
b955f04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/v1/zz_generated.swagger_doc_generated.go (1)
911-919:⚠️ Potential issue | 🟡 MinorAdd feature-gate note to avoid user confusion.
The description doesn’t mention that
modeis gated behindClusterUpdatePreflight(TechPreviewNoUpgrade). Without this, users may set the field and be surprised if it’s ignored or rejected when the gate is off.✅ Suggested doc tweak
- "mode": "mode determines how an update should be processed. The only valid value is \"Preflight\". When omitted, the cluster performs a normal update by applying the specified version or image to the cluster. This is the standard update behavior. When set to \"Preflight\", the cluster runs compatibility checks against the target release without performing an actual update. The target release's CVO will execute prechecks and report any detected risks in status.conditionalUpdates, alongside risks from the update recommendation service. Results appear in status.conditionalUpdates as a union of risks from both the update graph and preflight checks executed by the target release's CVO. This allows administrators to assess update readiness and address issues before committing to the update. Preflight mode is particularly useful for skip-level updates where upgrade compatibility needs to be verified across multiple minor versions.", + "mode": "mode determines how an update should be processed. The only valid value is \"Preflight\". This field is gated by the ClusterUpdatePreflight feature gate (TechPreviewNoUpgrade). When omitted, the cluster performs a normal update by applying the specified version or image to the cluster. This is the standard update behavior. When set to \"Preflight\", the cluster runs compatibility checks against the target release without performing an actual update. The target release's CVO will execute prechecks and report any detected risks in status.conditionalUpdates, alongside risks from the update recommendation service. Results appear in status.conditionalUpdates as a union of risks from both the update graph and preflight checks executed by the target release's CVO. This allows administrators to assess update readiness and address issues before committing to the update. Preflight mode is particularly useful for skip-level updates where upgrade compatibility needs to be verified across multiple minor versions.",
🤖 Fix all issues with AI agents
In `@config/v1/types_cluster_version.go`:
- Around line 775-789: Update the comment for the Mode field (UpdateModePolicy)
to correctly state that preflight risk details are reported in
status.conditionalUpdateRisks and that status.conditionalUpdates contains
references/entries that point to those risk objects; replace the sentence that
says "Results appear in status.conditionalUpdates as a union of risks..." with
wording that clarifies conditionalUpdates references the risk objects and the
detailed risk payloads are stored in status.conditionalUpdateRisks (keeping the
rest of the preflight explanation and the FeatureGate/optional tags intact).
config/v1/types_cluster_version.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify which status field carries preflight risks.
The doc says risks appear in status.conditionalUpdates, but risk details live in status.conditionalUpdateRisks (with conditionalUpdates referencing them). This could mislead API consumers. Consider tightening the wording.
✏️ Suggested doc tweak
- // risks in status.conditionalUpdates, alongside risks from the update recommendation service. - // Results appear in status.conditionalUpdates as a union of risks from both the update graph and - // preflight checks executed by the target release's CVO. + // risks in status.conditionalUpdateRisks, alongside risks from the update recommendation service, + // and are referenced by status.conditionalUpdates. + // Results appear in status.conditionalUpdateRisks as a union of risks from both the update graph and + // preflight checks executed by the target release's CVO.
🤖 Prompt for AI Agents
In `@config/v1/types_cluster_version.go` around lines 775 - 789, Update the
comment for the Mode field (UpdateModePolicy) to correctly state that preflight
risk details are reported in status.conditionalUpdateRisks and that
status.conditionalUpdates contains references/entries that point to those risk
objects; replace the sentence that says "Results appear in
status.conditionalUpdates as a union of risks..." with wording that clarifies
conditionalUpdates references the risk objects and the detailed risk payloads
are stored in status.conditionalUpdateRisks (keeping the rest of the preflight
explanation and the FeatureGate/optional tags intact).
716fcd7 to
5c1b79e
Compare
fao89
commented
Feb 11, 2026
/test okd-scos-images
PR-Agent: could not fine a component named okd-scos-images in a supported language in this PR.
config/v1/types_cluster_version.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The godoc for types appears in generated API documentation and oc explain output. "Constants below" is a Go source-level reference that means nothing to end users reading the generated docs.
config/v1/types_cluster_version.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are force and mode mutually exclusive? If so, then we sould document that and add a CEL XValidation rule including tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch! I'll work on that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Preflight mode support architecture changes? The documentation should clarify this, ideally with an explicit statement like "When mode is set to Preflight, the same rules for version, image, and architecture apply as for normal updates."
4751344 to
6266de1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that's missing:
config/v1/types_cluster_version.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must be feature gated as well:
We could also make the whole rule more readable, see:
rule="has(self.force) && has(self.mode) && self.force && self.mode != \"\" ? false : true"
to be changed to:
rule="!has(self.mode) || self.mode == \"\" || !has(self.force) || !self.force"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
config/v1/types_cluster_version.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we considered dropping Policy, it seems to imply a governance rule rather than a mode selector.
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
6266de1 to
63d0dc6
Compare
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, I'd simplify this and hide the implementation details:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.mode == "" seems to be unreachable:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add a test for force: false and mode: Preflight:
- name: Should allow Preflight mode when force is explicitly false
initial: |
apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
clusterID: foo
desiredUpdate:
version: 4.22.0
mode: Preflight
force: false
expected: |
apiVersion: config.openshift.io/v1
kind: ClusterVersion
spec:
clusterID: foo
desiredUpdate:
version: 4.22.0
mode: Preflight
Uh oh!
There was an error while loading. Please reload this page.
User description
Implements preflight update mode that allows checking update compatibility without committing to performing the actual cluster update.
Changes:
Related: openshift/enhancements#1930
PR Type
Enhancement
Description
Add optional
modefield toUpdatestruct supporting "Preflight" valueGate new field behind
ClusterUpdatePreflightfeature (TechPreviewNoUpgrade)Enable preflight mode for compatibility checks without actual cluster update
Add comprehensive test coverage for preflight mode with various configurations
Diagram Walkthrough
File Walkthrough
types_cluster_version.go
Define UpdateModePolicy type and add mode field to Updateconfig/v1/types_cluster_version.go
UpdateModePolicytype with enum validation for "Preflight"UpdateModePolicyPreflightfor the preflight mode valuemodefield toUpdatestruct with feature gate annotationzz_generated.swagger_doc_generated.go
Generate swagger documentation for mode fieldconfig/v1/zz_generated.swagger_doc_generated.go
modefield inUpdatestructzz_generated.openapi.go
Generate OpenAPI schema for mode fieldopenapi/generated_openapi/zz_generated.openapi.go
modefield inUpdatestructfeatures.go
Register ClusterUpdatePreflight feature gatefeatures/features.go
FeatureGateClusterUpdatePreflightfeature gate0000_00_cluster-version-operator_01_clusterversions-CustomNoUpgrade.crd.yaml
Update CRD manifest with mode field schemaconfig/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-CustomNoUpgrade.crd.yaml
modefield to CRD schema with enum validation for "Preflight"zz_generated.featuregated-crd-manifests.yaml
Register feature gate in manifest indexconfig/v1/zz_generated.featuregated-crd-manifests.yaml
ClusterUpdatePreflightto the list of feature gates forclusterversions CRD
ClusterUpdatePreflight.yaml
Generate feature-gated CRD manifest for preflight modeconfig/v1/zz_generated.featuregated-crd-manifests/clusterversions.config.openshift.io/ClusterUpdatePreflight.yaml
ClusterUpdatePreflight
modefield and all validationsClusterUpdatePreflight.yaml
Add comprehensive preflight mode test suiteconfig/v1/tests/clusterversions.config.openshift.io/ClusterUpdatePreflight.yaml