-
Notifications
You must be signed in to change notification settings - Fork 889
OCPEDGE-2989: Add controlPlaneTopologyTransitions to Infrastructure status #3029
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
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,6 +138,35 @@ type InfrastructureStatus struct { | |
| // +optional | ||
| InfrastructureTopology TopologyMode `json:"infrastructureTopology,omitempty"` | ||
|
|
||
| // controlPlaneTopologyTransitions reports, as controller-computed observed state, | ||
| // the control-plane topology transitions that originate at the current | ||
| // status.controlPlaneTopology and whether each can currently be initiated. It is | ||
| // advisory: the cluster may change between a status read and a spec write, so the | ||
| // cluster-config-operator revalidates any requested transition; consumers such as | ||
| // the CLI must not treat Available as an admission guarantee. Transitions are | ||
| // requested via spec.controlPlaneTopology, not through this field. When omitted, | ||
| // the controller has not yet completed its first evaluation; an empty list is also | ||
| // valid and intentionally carries the same meaning as omitted, since this field does | ||
| // not currently distinguish "not yet evaluated" from "evaluated with no applicable | ||
| // transitions". source and target below only cover the topologies that support a | ||
| // transition today; when status.controlPlaneTopology is a topology outside that set, | ||
| // this field is expected to remain omitted or empty. Entries are keyed by the | ||
| // (source, target) topology pair and list order is not significant. At most 4 | ||
| // entries are permitted, matching the full cardinality of the 2-value source/target | ||
| // enum below. If source/target are ever widened to cover additional | ||
| // transition-eligible topologies (HighlyAvailableArbiter, DualReplica; External is | ||
| // not expected to participate in a transition and should not be added here), | ||
| // MaxItems must be increased to match the new cardinality, up to 16 for all four | ||
| // non-External TopologyMode values. | ||
| // +openshift:enable:FeatureGate=MutableTopology | ||
| // +listType=map | ||
| // +listMapKey=source | ||
| // +listMapKey=target | ||
| // +kubebuilder:validation:MinItems=0 | ||
| // +kubebuilder:validation:MaxItems=4 | ||
| // +optional | ||
| ControlPlaneTopologyTransitions []ControlPlaneTopologyTransition `json:"controlPlaneTopologyTransitions,omitempty"` | ||
|
|
||
| // cpuPartitioning expresses if CPU partitioning is a currently enabled feature in the cluster. | ||
| // CPU Partitioning means that this cluster can support partitioning workloads to specific CPU Sets. | ||
| // Valid values are "None" and "AllNodes". When omitted, the default value is "None". | ||
|
|
@@ -177,6 +206,70 @@ const ( | |
| ExternalTopologyMode TopologyMode = "External" | ||
| ) | ||
|
|
||
| // ControlPlaneTopologyTransition describes one control-plane topology transition | ||
| // available from the cluster's current topology and whether it can currently be | ||
| // initiated. reason must be set whenever availability is Unavailable or Unknown; | ||
| // this is enforced by a validation rule on the entry as a whole. | ||
| // +kubebuilder:validation:XValidation:rule="self.availability == 'Available' || has(self.reason)",message="reason is required when availability is not Available" | ||
| type ControlPlaneTopologyTransition struct { | ||
| // source is the topology this transition starts from. It equals the current | ||
| // status.controlPlaneTopology. Valid values are SingleReplica and HighlyAvailable. | ||
| // +kubebuilder:validation:Enum=SingleReplica;HighlyAvailable | ||
| // +required | ||
| Source TopologyMode `json:"source,omitempty"` | ||
|
|
||
| // target is the topology this transition would move the control plane to. | ||
| // Valid values are SingleReplica and HighlyAvailable. | ||
| // +kubebuilder:validation:Enum=SingleReplica;HighlyAvailable | ||
| // +required | ||
| Target TopologyMode `json:"target,omitempty"` | ||
|
|
||
| // availability indicates whether this transition can currently be initiated. | ||
| // Valid values are Available, Unavailable, and Unknown. Available means the | ||
| // controller evaluated the transition and its preconditions pass. Unavailable | ||
| // means the transition is defined but cannot be initiated now; see reason and | ||
| // message. Unknown means the controller has not completed evaluation. | ||
| // +required | ||
| Availability TransitionAvailability `json:"availability,omitempty"` | ||
|
|
||
| // reason is a CamelCase machine-readable explanation of the availability, e.g. | ||
| // PreflightCheckFailed or SourceTopologyMismatch. It is required when | ||
| // availability is Unavailable or Unknown and is normally omitted when Available. | ||
| // The set of reasons is diagnostic and not exhaustive. Must start with an | ||
| // uppercase letter and contain only alphanumeric characters, and must be | ||
| // between 1 and 128 characters long. | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=128 | ||
| // +kubebuilder:validation:XValidation:rule=`self.matches('^[A-Z][A-Za-z0-9]*$')`,message="reason must be CamelCase, matching ^[A-Z][A-Za-z0-9]*$" | ||
| // +optional | ||
| Reason string `json:"reason,omitempty"` | ||
|
|
||
| // message is a human-readable explanation, primarily for Unavailable | ||
| // transitions (e.g. a concise summary of the failing preconditions). It is for | ||
| // humans only and must not be parsed. It may be truncated by the controller. | ||
| // When set, it must be between 1 and 2048 characters long. | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=2048 | ||
| // +optional | ||
| Message string `json:"message,omitempty"` | ||
| } | ||
|
Comment on lines
+214
to
+255
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. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Enforce both transition invariants at the status schema boundary The generated CRD accepts self-transitions and entries whose 🤖 Prompt for AI Agents |
||
|
|
||
| // TransitionAvailability indicates whether a topology transition can currently be | ||
| // initiated. | ||
| // +kubebuilder:validation:Enum=Available;Unavailable;Unknown | ||
| type TransitionAvailability string | ||
|
|
||
| const ( | ||
| // TransitionAvailable means the transition can be initiated now. | ||
| TransitionAvailable TransitionAvailability = "Available" | ||
|
|
||
| // TransitionUnavailable means the transition is defined but cannot be initiated now. | ||
| TransitionUnavailable TransitionAvailability = "Unavailable" | ||
|
|
||
| // TransitionUnknown means the controller has not completed evaluation. | ||
| TransitionUnknown TransitionAvailability = "Unknown" | ||
| ) | ||
|
|
||
| // CPUPartitioningMode defines the mode for CPU partitioning | ||
| type CPUPartitioningMode string | ||
|
|
||
|
|
||
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: openshift/api
Length of output: 50370
🤖 get_repo_knowledge executed:
get_repo_knowledge openshift/api /tmp/coderabbit-repo-knowledge/openshift-api-bb088791/conventionsLength of output: 5937
🏁 Script executed:
Repository: openshift/api
Length of output: 33946
🤖 get_repo_knowledge executed:
get_repo_knowledge openshift/api /tmp/coderabbit-repo-knowledge/openshift-api-bb088791Length of output: 86
🏁 Script executed:
Repository: openshift/api
Length of output: 50369
🏁 Script executed:
Repository: openshift/api
Length of output: 17179
🏁 Script executed:
Repository: openshift/api
Length of output: 21411
Enforce the transition invariants in
InfrastructureStatus.The item-level validation does not link
controlPlaneTopologyTransitionstostatus.controlPlaneTopology. The current fixture accepts both a mismatchedsourceandsource == target. Add anInfrastructureStatus-level CEL rule so every entry satisfiest.source == self.controlPlaneTopology && t.source != t.target. Regenerate the CRDs and add fixture cases that reject both forms.🤖 Prompt for AI Agents