From b52adf9ed0e3f645cce24c0394c8dab2d73c6d58 Mon Sep 17 00:00:00 2001 From: Damiano Donati Date: Sun, 14 Dec 2025 20:31:31 +0100 Subject: [PATCH 1/3] e2e: createCAPIMachine should only list workers for cloning one --- e2e/framework/machineset.go | 2 +- e2e/machine_migration_helpers.go | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/e2e/framework/machineset.go b/e2e/framework/machineset.go index bfc0249c0..c51b5bf93 100644 --- a/e2e/framework/machineset.go +++ b/e2e/framework/machineset.go @@ -136,7 +136,7 @@ func DeleteMachineSets(ctx context.Context, cl client.Client, machineSets ...*cl // MachineSet to enter the "Running" phase, and for all nodes belonging to those // Machines to be ready. func WaitForMachineSet(cl client.Client, name string, namespace string) { - By(fmt.Sprintf("Waiting for MachineSet machines %q to enter Running phase", name)) + By(fmt.Sprintf("Waiting for CAPI MachineSet machines %q to enter Running phase", name)) machineSet := GetMachineSet(cl, name, namespace) diff --git a/e2e/machine_migration_helpers.go b/e2e/machine_migration_helpers.go index 8626d9745..2ca1835af 100644 --- a/e2e/machine_migration_helpers.go +++ b/e2e/machine_migration_helpers.go @@ -23,7 +23,17 @@ import ( func createCAPIMachine(ctx context.Context, cl client.Client, machineName string) *clusterv1beta1.Machine { Expect(machineName).NotTo(BeEmpty(), "Machine name cannot be empty") - capiMachineList := capiframework.GetMachines(cl) + + workerLabelSelector := metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: clusterv1beta1.MachineControlPlaneLabel, + Operator: metav1.LabelSelectorOpDoesNotExist, + }, + }, + } + + capiMachineList := capiframework.GetMachines(cl, &workerLabelSelector) // The test requires at least one existing CAPI machine to act as a reference for creating a new one. Expect(capiMachineList).NotTo(BeEmpty(), "Should have found CAPI machines in the openshift-cluster-api namespace to use as a reference for creating a new one") From 7c3d9126cc056a9205566942c2c6df48dbc7fc69 Mon Sep 17 00:00:00 2001 From: Damiano Donati Date: Sun, 14 Dec 2025 20:37:37 +0100 Subject: [PATCH 2/3] e2e: fix CAPI MachineSet scale-down test using wrong wait function The MAPI-authoritative MachineSet migration test was using WaitForMachineSet after a scale-down operation. This function is designed for scale-up scenarios where it waits for new machines to reach "Running" phase and verifies node readiness by connecting to the workload cluster. For scale-down operations, this is inappropriate because: - No new machines are being provisioned that need to become running - It requires workload cluster connectivity to verify node status - The remaining machines were already running before the scale-down The test was failing with "not all Machines are running: 0 of 1" after 30 minutes because the CAPI MachineSet controller couldn't connect to the workload cluster to verify node status, causing availableReplicas to be reported as 0. Replace WaitForMachineSet with verifyMachinesetReplicas for the scale-down test, consistent with the analogous test in machineset_migration_capi_authoritative_test.go. The verifyMachinesetReplicas function only verifies the replica count matches the expected value, which is sufficient for scale-down validation. --- e2e/machineset_migration_mapi_authoritative_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/e2e/machineset_migration_mapi_authoritative_test.go b/e2e/machineset_migration_mapi_authoritative_test.go index d1a464a1d..2128e8b11 100644 --- a/e2e/machineset_migration_mapi_authoritative_test.go +++ b/e2e/machineset_migration_mapi_authoritative_test.go @@ -195,7 +195,6 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma It("should succeed scaling down CAPI MachineSet to 1, after the switch of AuthoritativeAPI to ClusterAPI", func() { By("Scaling down CAPI MachineSet to 1") capiframework.ScaleCAPIMachineSet(mapiMSAuthMAPIName, 1, capiframework.CAPINamespace) - capiframework.WaitForMachineSet(cl, mapiMSAuthMAPIName, capiframework.CAPINamespace) By("Verifying both CAPI MachineSet and its MAPI MachineSet mirror are scaled down to 1") verifyMachinesetReplicas(capiMachineSet, 1) From c1764cc9b23ed00f6a40da442657cb3bd05f4b94 Mon Sep 17 00:00:00 2001 From: Damiano Donati Date: Sun, 14 Dec 2025 21:32:15 +0100 Subject: [PATCH 3/3] fix: sync: add NodeRef to CAPI machine status comparison The CAPIMachineStatusEqual function was missing NodeRef in its comparison of CAPI machine status fields. This meant that when a MAPI machine received a node assignment (status.nodeRef), the sync controller didn't detect it as a change and didn't sync it to the CAPI machine mirror. This caused the CAPI machine to have an empty NodeRef, which led to: - CAPI MachineSet controller unable to verify node status - MachineSet reporting availableReplicas: 0 for running machines - Incorrect machine readiness calculations The conversion function already correctly included NodeRef in the converted status, but without the comparison, status updates were not triggered when only NodeRef changed. Add NodeRef to the list of compared fields in CAPIMachineStatusEqual so that changes to NodeRef are properly detected and synced from MAPI to CAPI machine mirrors. --- pkg/util/sync.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/util/sync.go b/pkg/util/sync.go index 410c178ad..b3dbeade9 100644 --- a/pkg/util/sync.go +++ b/pkg/util/sync.go @@ -206,6 +206,10 @@ func CAPIMachineStatusEqual(a, b clusterv1beta1.MachineStatus) map[string]any { diff[".nodeInfo"] = diffNodeInfo } + if diffNodeRef := deep.Equal(a.NodeRef, b.NodeRef); len(diffNodeRef) > 0 { + diff[".nodeRef"] = diffNodeRef + } + if diffConditions := compareCAPIV1Beta1Conditions(a.Conditions, b.Conditions); len(diffConditions) > 0 { diff[".conditions"] = diffConditions }