-
Notifications
You must be signed in to change notification settings - Fork 588
OTA-253: Add cluster update preflight mode API #2684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this | ||
| name: "ClusterVersion" | ||
| crdName: clusterversions.config.openshift.io | ||
| featureGates: | ||
| - ClusterUpdatePreflight | ||
| tests: | ||
| onCreate: | ||
| - 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 image | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| image: quay.io/openshift-release-dev/ocp-release@sha256:abc123 | ||
| mode: Preflight | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| image: quay.io/openshift-release-dev/ocp-release@sha256:abc123 | ||
| mode: Preflight | ||
| - name: Should be able to use Preflight mode with architecture | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| architecture: Multi | ||
| version: 4.22.0 | ||
| mode: Preflight | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| architecture: Multi | ||
| version: 4.22.0 | ||
| mode: Preflight | ||
| - 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 | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.22.0 | ||
| mode: Preflight | ||
| acceptRisks: | ||
| - name: RiskA | ||
| - name: RiskB | ||
| - name: Invalid mode value should be rejected | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.22.0 | ||
| mode: InvalidMode | ||
| expectedError: "Unsupported value: \"InvalidMode\"" | ||
| onUpdate: | ||
| - name: Should be able to change mode from normal to Preflight | ||
| 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 change mode from Preflight to normal | ||
| 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 | ||
| - name: Should be able to change target version while in 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.23.0 | ||
| mode: Preflight | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| spec: | ||
| clusterID: foo | ||
| desiredUpdate: | ||
| version: 4.23.0 | ||
| mode: Preflight |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -283,6 +283,15 @@ type UpdateHistory struct { | |||||||||||||||||||||||||||||||||||||||||||
| // ClusterID is string RFC4122 uuid. | ||||||||||||||||||||||||||||||||||||||||||||
| type ClusterID string | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // UpdateModePolicy defines how an update should be processed. | ||||||||||||||||||||||||||||||||||||||||||||
| // +kubebuilder:validation:Enum=Preflight | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+286
to
+287
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit nit:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| type UpdateModePolicy string | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| const ( | ||||||||||||||||||||||||||||||||||||||||||||
| // UpdateModePolicyPreflight allows an update to be checked for compatibility without committing to updating the cluster. | ||||||||||||||||||||||||||||||||||||||||||||
| UpdateModePolicyPreflight UpdateModePolicy = "Preflight" | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // ClusterVersionArchitecture enumerates valid cluster architectures. | ||||||||||||||||||||||||||||||||||||||||||||
| // +kubebuilder:validation:Enum="Multi";"" | ||||||||||||||||||||||||||||||||||||||||||||
| type ClusterVersionArchitecture string | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -760,6 +769,15 @@ type Update struct { | |||||||||||||||||||||||||||||||||||||||||||
| // +listMapKey=name | ||||||||||||||||||||||||||||||||||||||||||||
| // +optional | ||||||||||||||||||||||||||||||||||||||||||||
| AcceptRisks []AcceptRisk `json:"acceptRisks,omitempty"` | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // 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"` | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+773
to
+780
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
How about something like:
Suggested change
We have to:
|
||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // AcceptRisk represents a risk that is considered acceptable. | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
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'd keep tests cases only which validate actual behavior constraints. Means that basic CRUD is probably out of scope.
We also need a test for
mode: ""as invalid value.