-
Notifications
You must be signed in to change notification settings - Fork 891
OCPEDGE-2958: Add topologyTransitionStatus field to Infrastructure status #3026
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 | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -138,6 +138,43 @@ type InfrastructureStatus struct { | |||||||||||||||||||||||||||||||||||||||||
| // +optional | ||||||||||||||||||||||||||||||||||||||||||
| InfrastructureTopology TopologyMode `json:"infrastructureTopology,omitempty"` | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // ControlPlaneTopologyTransitionStatus reports the current state of a | ||||||||||||||||||||||||||||||||||||||||||
| // control plane topology transition requested via spec.controlPlaneTopology. | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // "NotTransitioned" means no transition has ever been requested: | ||||||||||||||||||||||||||||||||||||||||||
| // spec.controlPlaneTopology is empty. This is the default state prior to | ||||||||||||||||||||||||||||||||||||||||||
| // the cluster's first transition; once a transition completes, this | ||||||||||||||||||||||||||||||||||||||||||
| // field moves to "Transitioned" and does not return to "NotTransitioned". | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // "Pending" means a transition has been requested and admitted by the | ||||||||||||||||||||||||||||||||||||||||||
| // topology transition controller, and the controller is actively | ||||||||||||||||||||||||||||||||||||||||||
| // reconciling the cluster to the new topology. | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // "Error" means a requested transition could not be admitted, either | ||||||||||||||||||||||||||||||||||||||||||
| // because the requested topology change is not a supported transition or | ||||||||||||||||||||||||||||||||||||||||||
| // because a precondition was not met. The specific reason is reported as | ||||||||||||||||||||||||||||||||||||||||||
| // an Event on this Infrastructure object rather than in this field. | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // "Transitioned" means the most recently requested transition completed | ||||||||||||||||||||||||||||||||||||||||||
| // successfully; spec.controlPlaneTopology matches status.controlPlaneTopology. | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // +kubebuilder:default=NotTransitioned | ||||||||||||||||||||||||||||||||||||||||||
| // +kubebuilder:validation:Enum=NotTransitioned;Pending;Error;RetryWithBackoff;Transitioned | ||||||||||||||||||||||||||||||||||||||||||
| // +openshift:enable:FeatureGate=MutableTopology | ||||||||||||||||||||||||||||||||||||||||||
| // +optional | ||||||||||||||||||||||||||||||||||||||||||
| ControlPlaneTopologyTransitionStatus TopologyTransitionStatus `json:"controlPlaneTopologyTransitionStatus,omitempty"` | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // InfrastrutureTopologyTransitionStatus reports the current state of a | ||||||||||||||||||||||||||||||||||||||||||
| // infrastructure topology transition requested via spec.controlPlaneTopology. | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // See ControlPlaneTopologyTransitionStatus for enum definitinos and meanings. | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // +kubebuilder:default=NotTransitioned | ||||||||||||||||||||||||||||||||||||||||||
| // +kubebuilder:validation:Enum=NotTransitioned;Pending;Error;RetryWithBackoff;Transitioned | ||||||||||||||||||||||||||||||||||||||||||
| // +openshift:enable:FeatureGate=MutableTopology | ||||||||||||||||||||||||||||||||||||||||||
| // +optional | ||||||||||||||||||||||||||||||||||||||||||
| InfrastructureTopologyTransitionStatus TopologyTransitionStatus `json:"infrastructureTopologyTransitionStatus,omitempty"` | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+167
to
+176
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. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Fix typos in the Line 167 misspells This comment regenerates verbatim into:
🐛 Proposed fix- // InfrastrutureTopologyTransitionStatus reports the current state of a
- // infrastructure topology transition requested via spec.controlPlaneTopology.
+ // InfrastructureTopologyTransitionStatus reports the current state of an
+ // infrastructure topology transition requested via spec.controlPlaneTopology.
//
- // See ControlPlaneTopologyTransitionStatus for enum definitinos and meanings.
+ // See ControlPlaneTopologyTransitionStatus for enum definitions and meanings.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // 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 +214,53 @@ const ( | |||||||||||||||||||||||||||||||||||||||||
| ExternalTopologyMode TopologyMode = "External" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // TopologyTransitionStatus tracks the current state of a topology transition | ||||||||||||||||||||||||||||||||||||||||||
| // requested via spec.controlPlaneTopology. Each value corresponds 1:1 to a | ||||||||||||||||||||||||||||||||||||||||||
| // distinct (Progressing, Upgradeable) condition-reason pair reported by the | ||||||||||||||||||||||||||||||||||||||||||
| // topology transition controller on the cluster-config-operator ClusterOperator: | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // - NotTransitioned -> Progressing=False/AsExpected, Upgradeable=True/AsExpected | ||||||||||||||||||||||||||||||||||||||||||
| // - Pending -> Progressing=True/TopologyTransitionInProgress, Upgradeable=False/TopologyTransitionInProgress | ||||||||||||||||||||||||||||||||||||||||||
| // - RetryWithBackoff -> Progressing=True/TopologyTransitionInProgress, Upgradeable=False/TopologyTransitionInProgress | ||||||||||||||||||||||||||||||||||||||||||
| // - Error -> Progressing=False/{UnsupportedTransition|PreflightCheckFailed}, Upgradeable=False/{same} | ||||||||||||||||||||||||||||||||||||||||||
| // - Transitioned -> Progressing=False/TopologyTransitionComplete, Upgradeable=True/TopologyTransitionComplete | ||||||||||||||||||||||||||||||||||||||||||
| type TopologyTransitionStatus string | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const ( | ||||||||||||||||||||||||||||||||||||||||||
| // TopologyTransitionStatusNotTransitioned indicates that no topology transition has | ||||||||||||||||||||||||||||||||||||||||||
| // ever been requested: spec.controlPlaneTopology is empty. This is the | ||||||||||||||||||||||||||||||||||||||||||
| // default state prior to the cluster's first transition. Once a | ||||||||||||||||||||||||||||||||||||||||||
| // transition completes, the field moves to TopologyTransitionStatusTransitioned | ||||||||||||||||||||||||||||||||||||||||||
| // and does not return to NotTransitioned. | ||||||||||||||||||||||||||||||||||||||||||
| TopologyTransitionStatusNotTransitioned TopologyTransitionStatus = "NotTransitioned" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // TopologyTransitionStatusPending indicates that a topology transition | ||||||||||||||||||||||||||||||||||||||||||
| // has been requested and admitted, and the controller is actively | ||||||||||||||||||||||||||||||||||||||||||
| // reconciling the cluster to the new topology. | ||||||||||||||||||||||||||||||||||||||||||
| TopologyTransitionStatusPending TopologyTransitionStatus = "Pending" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // TopologyTransitionStatusRetryWithBackoff indicates that the controller | ||||||||||||||||||||||||||||||||||||||||||
| // encountered a synchronization or API error while reconciling the | ||||||||||||||||||||||||||||||||||||||||||
| // transition. The error is returned to the rate-limited workqueue so the | ||||||||||||||||||||||||||||||||||||||||||
| // transition is retried with backoff. This state is distinct from Error, | ||||||||||||||||||||||||||||||||||||||||||
| // which indicates that admission failed because a transition is unsupported | ||||||||||||||||||||||||||||||||||||||||||
| // or a precondition was not met. | ||||||||||||||||||||||||||||||||||||||||||
| TopologyTransitionStatusRetryWithBackoff TopologyTransitionStatus = "RetryWithBackoff" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // TopologyTransitionStatusError indicates that a requested topology | ||||||||||||||||||||||||||||||||||||||||||
| // transition could not be admitted, either because the requested | ||||||||||||||||||||||||||||||||||||||||||
| // topology change is not a supported transition or because a | ||||||||||||||||||||||||||||||||||||||||||
| // precondition was not met. The specific reason is reported as an Event | ||||||||||||||||||||||||||||||||||||||||||
| // on this Infrastructure object rather than in this field. | ||||||||||||||||||||||||||||||||||||||||||
| TopologyTransitionStatusError TopologyTransitionStatus = "Error" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // TopologyTransitionStatusTransitioned indicates that a topology | ||||||||||||||||||||||||||||||||||||||||||
| // transition completed successfully. This is a terminal state: since | ||||||||||||||||||||||||||||||||||||||||||
| // only one-directional transitions are currently supported, the field | ||||||||||||||||||||||||||||||||||||||||||
| // remains Transitioned for the lifetime of the cluster once reached. | ||||||||||||||||||||||||||||||||||||||||||
| TopologyTransitionStatusTransitioned TopologyTransitionStatus = "Transitioned" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // 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.
Might we need to review this field considering the upcoming
spec.infrastructureTopologyfield from Standalone Infrastructure Topology Transition?More details on the dedicated deliverables' doc.
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 linked doc describes status reporting via CCO condition messages, which is a pattern that this work deprecates and replaces with first class citizen fields under .status. So the work here is orthogonal to the work described in the doc.