OTA-253: Add cluster update preflight mode API #2684
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@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. |
|
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)
Comment |
|
Hello @fao89! Some important instructions when contributing to openshift/api: |
|
@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. |
openshift-ci
bot
added
the
size/XL
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
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: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
fao89
force-pushed
the
preflight-from-target-release
branch
from
8c3f649 to
b2750d9
Compare
openshift-ci
bot
added
size/XXL
fao89
force-pushed
the
preflight-from-target-release
branch
from
b2750d9 to
fb4267a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
| // 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 | ||
| // +optional | ||
| Mode UpdateModePolicy `json:"mode,omitempty"` |
There was a problem hiding this comment.
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:
| // 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 | |
| // +optional | |
| Mode UpdateModePolicy `json:"mode,omitempty"` | |
| // 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. | |
| // 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. | |
| // +openshift:enable:FeatureGate=ClusterUpdatePreflight | |
| // +optional | |
| Mode UpdateModePolicy `json:"mode,omitempty"` |
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)
fao89
force-pushed
the
preflight-from-target-release
branch
from
fb4267a to
56aa358
Compare
openshift-ci
bot
added
size/XL
|
|
||
| // UpdateModePolicy defines how an update should be processed. | ||
| // Valid values are defined as constants below. | ||
| // +kubebuilder:validation:Enum=Preflight |
There was a problem hiding this comment.
+enum adds an Enum constraint to the OpenAPI schema and appends auto-generated enum documentation to the description.
| // +kubebuilder:validation:Enum=Preflight | |
| // +enum | |
| // +kubebuilder:validation:Enum=Preflight |
| - name: Should be able to set mode to Preflight | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.22.0 | ||
| mode: Preflight | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.22.0 | ||
| mode: Preflight | ||
| - name: Should be able to omit mode field (default behavior) | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.22.0 | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.22.0 | ||
| - name: Should be able to use Preflight mode with acceptRisks | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.22.0 | ||
| mode: Preflight | ||
| acceptRisks: | ||
| - name: RiskA | ||
| - name: RiskB |
There was a problem hiding this comment.
I think those are basic CRUD tests and can be removed.
| version: 4.22.0 | ||
| mode: "" | ||
| expectedError: "Unsupported value: \"\"" | ||
| onUpdate: [] No newline at end of file |
There was a problem hiding this comment.
Let's add transition tests:
| onUpdate: [] | |
| onUpdate: | |
| - name: Should be able to transition from normal to Preflight mode | |
| initial: | | |
| apiVersion: config.openshift.io/v1 | |
| kind: ClusterVersion | |
| spec: | |
| clusterID: foo | |
| desiredUpdate: | |
| version: 4.22.0 | |
| updated: | | |
| apiVersion: config.openshift.io/v1 | |
| kind: ClusterVersion | |
| spec: | |
| clusterID: foo | |
| desiredUpdate: | |
| version: 4.22.0 | |
| mode: Preflight | |
| expected: | | |
| apiVersion: config.openshift.io/v1 | |
| kind: ClusterVersion | |
| spec: | |
| clusterID: foo | |
| desiredUpdate: | |
| version: 4.22.0 | |
| mode: Preflight | |
| - name: Should be able to clear Preflight mode | |
| initial: | | |
| apiVersion: config.openshift.io/v1 | |
| kind: ClusterVersion | |
| spec: | |
| clusterID: foo | |
| desiredUpdate: | |
| version: 4.22.0 | |
| mode: Preflight | |
| updated: | | |
| apiVersion: config.openshift.io/v1 | |
| kind: ClusterVersion | |
| spec: | |
| clusterID: foo | |
| desiredUpdate: | |
| version: 4.22.0 | |
| expected: | | |
| apiVersion: config.openshift.io/v1 | |
| kind: ClusterVersion | |
| spec: | |
| clusterID: foo | |
| desiredUpdate: | |
| version: 4.22.0 |
fao89
force-pushed
the
preflight-from-target-release
branch
from
56aa358 to
b955f04
Compare
openshift-ci
bot
added
size/XXL
There was a problem hiding this comment.
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).
| // 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. | ||
| // +openshift:enable:FeatureGate=ClusterUpdatePreflight | ||
| // +optional | ||
| Mode UpdateModePolicy `json:"mode,omitempty"` |
There was a problem hiding this comment.
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).
326a52a to
716fcd7
Compare
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
force-pushed
the
preflight-from-target-release
branch
from
716fcd7 to
5c1b79e
Compare
|
@fao89: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
3 participants
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