From cefd64fffb1c457a5669e4af7428006edeb3d985 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 193729a6f92005e1133ff91d4da512637fb27c4a 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 74102108d512c78871eaf791f4926f76120371b2 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 }