Skip to content

OCPCLOUD-2998: Add synchronizedAPI to Machine and MachineSet status #2642

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.

Sign up for GitHub

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

Open
RadekManak wants to merge 1 commit into
base: master
Choose a base branch
from
Open

OCPCLOUD-2998: Add synchronizedAPI to Machine and MachineSet status #2642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions machine/v1beta1/types_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,16 @@ const (
MachineAuthorityMigrating MachineAuthority = "Migrating"
)

type SynchronizedAPI string

const (
// MachineAPISynchronized indicates that the Machine API is the last synchronized API.
MachineAPISynchronized SynchronizedAPI = "MachineAPI"

// ClusterAPISynchronized indicates that the Cluster API is the last synchronized API.
ClusterAPISynchronized SynchronizedAPI = "ClusterAPI"
)

// +genclient
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

Expand Down Expand Up @@ -402,10 +412,22 @@ type MachineStatus struct {
// When set to Migrating, the migration controller is currently performing the handover of authority from one API to the other.
// +kubebuilder:validation:Enum=MachineAPI;ClusterAPI;Migrating
// +kubebuilder:validation:XValidation:rule="self == 'Migrating' || self == oldSelf || oldSelf == 'Migrating'",message="The authoritativeAPI field must not transition directly from MachineAPI to ClusterAPI or vice versa. It must transition through Migrating."
// +kubebuilder:validation:XValidation:rule="oldSelf == ''|| self != ''",message="The authoritativeAPI field must not be set empty once it has a value."
Copy link
Contributor

@JoelSpeed JoelSpeed Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation on this one? Empty isn't actually a valid value so it would be rejected anyway?

Were you trying to make sure it cannot be removed once set? If so, you need a CEL rule on the parent struct, not on the field itself

// +openshift:enable:FeatureGate=MachineAPIMigration
// +optional
AuthoritativeAPI MachineAuthority `json:"authoritativeAPI,omitempty"`

// synchronizedAPI represents the API that is currently in sync with the state of the resource.
// Valid values are "MachineAPI" and "ClusterAPI".
// When omitted (empty value), the resource has not yet been reconciled by the migration controller.
Copy link
Contributor

@JoelSpeed JoelSpeed Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitted doesn't actually mean an empty value, it means the key isn't present

// The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
// It is used to determine the source API of the migration.
Copy link
Contributor

@JoelSpeed JoelSpeed Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source of the last successful synchronization wasn't it?

// +kubebuilder:validation:XValidation:rule="oldSelf == ''|| self != ''",message="The synchronizedAPI field must not be set empty once it has a value."
// +kubebuilder:validation:Enum=MachineAPI;ClusterAPI
Copy link
Contributor

@JoelSpeed JoelSpeed Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to add this on the type itself

// +openshift:enable:FeatureGate=MachineAPIMigration
// +optional
SynchronizedAPI SynchronizedAPI `json:"synchronizedAPI,omitempty"`

// synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
// This field is set when the authoritative resource is updated and the sync controller has updated the non-authoritative resource to match.
// +kubebuilder:validation:Minimum=0
Expand Down
10 changes: 10 additions & 0 deletions machine/v1beta1/types_machineset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@ type MachineSetStatus struct {
// +optional
AuthoritativeAPI MachineAuthority `json:"authoritativeAPI,omitempty"`

// synchronizedAPI represents the API that is currently in sync with the state of the resource.
// Valid values are "MachineAPI" and "ClusterAPI".
// When omitted (empty value), the resource has not yet been reconciled by the migration controller.
// The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
// It is used to determine the source API of the migration.
// +kubebuilder:validation:Enum=MachineAPI;ClusterAPI
// +openshift:enable:FeatureGate=MachineAPIMigration
// +optional
SynchronizedAPI SynchronizedAPI `json:"synchronizedAPI,omitempty"`
Comment on lines +171 to +179
Copy link

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the full MachineSetStatus struct around the synchronizedAPI field
fd -e "types_machineset.go" --type f | head -1 | xargs wc -l

Repository: openshift/api

Length of output: 58

🏁 Script executed:

# Get context around the synchronizedAPI field and check for validation rules
cat -n machine/v1beta1/types_machineset.go | sed -n '110,190p'

Repository: openshift/api

Length of output: 5548

🏁 Script executed:

# Search for all XValidation rules in the MachineSetStatus area
rg -nP 'XValidation|Validation' machine/v1beta1/types_machineset.go -B 2 -A 2

Repository: openshift/api

Length of output: 1219

🏁 Script executed:

# Check the Machine CRD (types_machine.go) for synchronizedAPI immutability patterns
fd -e "types_machine.go" --type f | head -1 | xargs wc -l

Repository: openshift/api

Length of output: 58

🏁 Script executed:

# Search for synchronizedAPI in Machine CRD with validation context
rg -nP 'synchronizedAPI' machine/v1beta1/types_machine.go -B 5 -A 5

Repository: openshift/api

Length of output: 1597

Add immutability validation to prevent clearing synchronizedAPI (and authoritativeAPI) once set.

The synchronizedAPI field in MachineSetStatus is missing an immutability validation rule that already exists in the corresponding MachineStatus field in the Machine CRD. The Machine CRD enforces this constraint with:

+kubebuilder:validation:XValidation:rule="oldSelf == ''|| self != ''",message="The synchronizedAPI field must not be set empty once it has a value."

The same is true for the authoritativeAPI field in MachineSetStatus (line 169), which also lacks this immutability protection despite its presence in the Machine CRD. Both fields should have the same immutability constraint applied to align with the PR objective: "once set the field cannot be cleared."

🤖 Prompt for AI Agents 
In @machine/v1beta1/types_machineset.go around lines 171 - 179, Add the same
immutability XValidation annotations used in the Machine CRD to both
MachineSetStatus.authoritativeAPI and MachineSetStatus.synchronizedAPI so they
cannot be cleared once set: apply
+kubebuilder:validation:XValidation:rule="oldSelf == '' || self !=
''",message="The <field> field must not be set empty once it has a value."
(replace <field> with authoritativeAPI and synchronizedAPI respectively) on the
corresponding fields (AuthoritativeAPI and SynchronizedAPI) to enforce the "once
set cannot be cleared" behavior.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# First, let's find the files containing MachineStatus and MachineSetStatus
echo "=== Finding type definition files ==="
fd -e go -x grep -l "type MachineStatus struct\|type MachineSetStatus struct" {} \;

# Let's look for the synchronizedAPI field in both types
echo -e "\n=== Searching for synchronizedAPI field ==="
rg -n "SynchronizedAPI" --type=go -B 5 -A 1

# Check specifically in the types files
echo -e "\n=== Checking MachineSet status file ==="
cat -n machine/v1beta1/types_machineset.go | sed -n '165,185p'

# Find and check the Machine status file
echo -e "\n=== Finding Machine status file ==="
fd -e go "types_machine.go" -x cat -n {} \; | head -100

Repository: openshift/api

Length of output: 9581

Add XValidation rule to match MachineStatus field validation.

The synchronizedAPI field in MachineSetStatus is missing the immutability validation rule present in MachineStatus. MachineStatus includes:

+kubebuilder:validation:XValidation:rule="oldSelf == ''|| self != ''",message="The synchronizedAPI field must not be set empty once it has a value."

This rule prevents the field from being unset once it has been assigned a value. Add this XValidation rule to MachineSetStatus to maintain consistency and enforce the same state transition constraints across both types.

🤖 Prompt for AI Agents 
In @machine/v1beta1/types_machineset.go around lines 171 - 179,
MachineSetStatus's synchronizedAPI field lacks the XValidation immutability rule
present on MachineStatus; add the same kubebuilder XValidation annotation to the
SynchronizedAPI field in MachineSetStatus so it prevents clearing the field once
set (use the rule: oldSelf == '' || self != '' with the message "The
synchronizedAPI field must not be set empty once it has a value."), placing the
annotation immediately above the SynchronizedAPI field declaration so it applies
to that field.


// synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
// This field is set when the authoritative resource is updated and the sync controller has updated the non-authoritative resource to match.
// +kubebuilder:validation:Minimum=0
Expand Down
18 changes: 18 additions & 0 deletions ...beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ spec:
from MachineAPI to ClusterAPI or vice versa. It must transition
through Migrating.
rule: self == 'Migrating' || self == oldSelf || oldSelf == 'Migrating'
- message: The authoritativeAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
conditions:
description: conditions defines the current state of the Machine
items:
Expand Down Expand Up @@ -567,6 +570,21 @@ spec:
serialized/deserialized from this field.
type: object
x-kubernetes-preserve-unknown-fields: true
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
x-kubernetes-validations:
- message: The synchronizedAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
18 changes: 18 additions & 0 deletions ...1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ spec:
from MachineAPI to ClusterAPI or vice versa. It must transition
through Migrating.
rule: self == 'Migrating' || self == oldSelf || oldSelf == 'Migrating'
- message: The authoritativeAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
conditions:
description: conditions defines the current state of the Machine
items:
Expand Down Expand Up @@ -567,6 +570,21 @@ spec:
serialized/deserialized from this field.
type: object
x-kubernetes-preserve-unknown-fields: true
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
x-kubernetes-validations:
- message: The synchronizedAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
18 changes: 18 additions & 0 deletions .../zz_generated.crd-manifests/0000_10_machine-api_01_machines-TechPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ spec:
from MachineAPI to ClusterAPI or vice versa. It must transition
through Migrating.
rule: self == 'Migrating' || self == oldSelf || oldSelf == 'Migrating'
- message: The authoritativeAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
conditions:
description: conditions defines the current state of the Machine
items:
Expand Down Expand Up @@ -567,6 +570,21 @@ spec:
serialized/deserialized from this field.
type: object
x-kubernetes-preserve-unknown-fields: true
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
x-kubernetes-validations:
- message: The synchronizedAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
11 changes: 11 additions & 0 deletions ...a1/zz_generated.crd-manifests/0000_10_machine-api_01_machinesets-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,17 @@ spec:
description: replicas is the most recently observed number of replicas.
format: int32
type: integer
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
11 changes: 11 additions & 0 deletions ...z_generated.crd-manifests/0000_10_machine-api_01_machinesets-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,17 @@ spec:
description: replicas is the most recently observed number of replicas.
format: int32
type: integer
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
11 changes: 11 additions & 0 deletions ..._generated.crd-manifests/0000_10_machine-api_01_machinesets-TechPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,17 @@ spec:
description: replicas is the most recently observed number of replicas.
format: int32
type: integer
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
18 changes: 18 additions & 0 deletions ...nerated.featuregated-crd-manifests/machines.machine.openshift.io/MachineAPIMigration.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@ spec:
from MachineAPI to ClusterAPI or vice versa. It must transition
through Migrating.
rule: self == 'Migrating' || self == oldSelf || oldSelf == 'Migrating'
- message: The authoritativeAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
conditions:
description: conditions defines the current state of the Machine
items:
Expand Down Expand Up @@ -569,6 +572,21 @@ spec:
serialized/deserialized from this field.
type: object
x-kubernetes-preserve-unknown-fields: true
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
x-kubernetes-validations:
- message: The synchronizedAPI field must not be set empty once it
has a value.
rule: oldSelf == ''|| self != ''
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
11 changes: 11 additions & 0 deletions ...ated.featuregated-crd-manifests/machinesets.machine.openshift.io/MachineAPIMigration.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,17 @@ spec:
description: replicas is the most recently observed number of replicas.
format: int32
type: integer
synchronizedAPI:
description: |-
synchronizedAPI represents the API that is currently in sync with the state of the resource.
Valid values are "MachineAPI" and "ClusterAPI".
When omitted (empty value), the resource has not yet been reconciled by the migration controller.
The migration controller sets `status.synchronizedAPI` to the value of the `status.authoritativeAPI` before it transitions to "Migrating".
It is used to determine the source API of the migration.
enum:
- MachineAPI
- ClusterAPI
type: string
synchronizedGeneration:
description: |-
synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with.
Expand Down
2 changes: 2 additions & 0 deletions machine/v1beta1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading