Fix MAPI->CAPI conversion not adding ControlPlane label

Author: RadekManak

pkg/controllers/machinesync/aws_test.go

@@ -18,6 +18,7 @@ package machinesync
 
 import (
 	"context"
+	"encoding/json"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -91,7 +92,7 @@ var _ = Describe("AWS load balancer validation during MAPI->CAPI conversion", fu
 		Expect(k8sClient.Create(ctx, capiNamespace)).To(Succeed())
 
 		infrastructureName = "cluster-aws-lb"
-		awsClusterBuilder = awsv1resourcebuilder.AWSCluster().WithNamespace(capiNamespace.GetName()).WithName(infrastructureName)
+		awsClusterBuilder = awsv1resourcebuilder.AWSCluster().WithNamespace(capiNamespace.GetName()).WithName(infrastructureName).WithRegion("us-east-1")
 
 		// Create CAPI Cluster that all tests will use
 		capiCluster := clusterv1resourcebuilder.Cluster().WithNamespace(capiNamespace.GetName()).WithName(infrastructureName).WithInfrastructureRef(&corev1.ObjectReference{
@@ -291,6 +292,86 @@ var _ = Describe("AWS load balancer validation during MAPI->CAPI conversion", fu
 		capiMachine := clusterv1resourcebuilder.Machine().WithNamespace(capiNamespace.GetName()).WithName(mapiMachine.GetName()).Build()
 		Eventually(k8sClient.Get(ctx, client.ObjectKeyFromObject(capiMachine), capiMachine), timeout).Should(Succeed())
 	})
+
+	It("should preserve load balancers when toggling authoritativeAPI MAPI -> CAPI -> MAPI", func() {
+		By("Creating AWSCluster with load balancers")
+		loadBalancerSpec := &awsv1.AWSLoadBalancerSpec{
+			Name:             ptr.To("cluster-int"),
+			LoadBalancerType: awsv1.LoadBalancerTypeNLB,
+		}
+		secondaryLoadBalancerSpec := &awsv1.AWSLoadBalancerSpec{
+			Name:             ptr.To("cluster-ext"),
+			LoadBalancerType: awsv1.LoadBalancerTypeNLB,
+		}
+		awsCluster := awsClusterBuilder.WithControlPlaneLoadBalancer(loadBalancerSpec).WithSecondaryControlPlaneLoadBalancer(secondaryLoadBalancerSpec).Build()
+		Expect(k8sClient.Create(ctx, awsCluster)).To(Succeed())
+
+		lbRefs := []mapiv1beta1.LoadBalancerReference{
+			{Name: "cluster-int", Type: mapiv1beta1.NetworkLoadBalancerType},
+			{Name: "cluster-ext", Type: mapiv1beta1.NetworkLoadBalancerType},
+		}
+
+		By("Creating MAPI master machine with load balancers and authoritativeAPI: MachineAPI")
+		mapiMachine := machinev1resourcebuilder.Machine().
+			WithNamespace(mapiNamespace.GetName()).
+			WithGenerateName("master-").
+			AsMaster().
+			WithProviderSpecBuilder(machinev1resourcebuilder.AWSProviderSpec().WithLoadBalancers(lbRefs)).
+			Build()
+		Expect(k8sClient.Create(ctx, mapiMachine)).To(Succeed())
+		Eventually(k.UpdateStatus(mapiMachine, func() { mapiMachine.Status.AuthoritativeAPI = mapiv1beta1.MachineAuthorityMachineAPI })).Should(Succeed())
+
+		By("Verifying MAPI -> CAPI sync is successful")
+		Eventually(k.Object(mapiMachine), timeout).Should(
+			HaveField("Status.Conditions", ContainElement(
+				SatisfyAll(
+					HaveField("Type", Equal(consts.SynchronizedCondition)),
+					HaveField("Status", Equal(corev1.ConditionTrue)),
+					HaveField("Reason", Equal("ResourceSynchronized")),
+					HaveField("Message", Equal("Successfully synchronized MAPI Machine to CAPI")),
+				))),
+		)
+
+		By("Capturing original providerSpec")
+		Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(mapiMachine), mapiMachine)).To(Succeed())
+		originalProviderSpec := mapiMachine.Spec.ProviderSpec.Value.DeepCopy()
+
+		By("Switching authoritativeAPI to ClusterAPI")
+		transitionToAuthoritativeAPI(ctx, k8sClient, k, mapiMachine, mapiv1beta1.MachineAuthorityClusterAPI, timeout)
+
+		By("Verifying CAPI -> MAPI sync is successful")
+		Eventually(k.Object(mapiMachine), timeout).Should(
+			HaveField("Status.Conditions", ContainElement(
+				SatisfyAll(
+					HaveField("Type", Equal(consts.SynchronizedCondition)),
+					HaveField("Status", Equal(corev1.ConditionTrue)),
+					HaveField("Reason", Equal("ResourceSynchronized")),
+					HaveField("Message", Equal("Successfully synchronized CAPI Machine to MAPI")),
+				))),
+		)
+
+		By("Switching authoritativeAPI back to MachineAPI")
+		transitionToAuthoritativeAPI(ctx, k8sClient, k, mapiMachine, mapiv1beta1.MachineAuthorityMachineAPI, timeout)
+
+		By("Verifying MAPI -> CAPI sync is successful")
+		Eventually(k.Object(mapiMachine), timeout).Should(
+			HaveField("Status.Conditions", ContainElement(
+				SatisfyAll(
+					HaveField("Type", Equal(consts.SynchronizedCondition)),
+					HaveField("Status", Equal(corev1.ConditionTrue)),
+					HaveField("Reason", Equal("ResourceSynchronized")),
+					HaveField("Message", Equal("Successfully synchronized MAPI Machine to CAPI")),
+				))),
+		)
+
+		By("Verifying load balancers are unchanged after round trip")
+		Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(mapiMachine), mapiMachine)).To(Succeed())
+
+		var originalSpec, finalSpec mapiv1beta1.AWSMachineProviderConfig
+		Expect(json.Unmarshal(originalProviderSpec.Raw, &originalSpec)).To(Succeed())
+		Expect(json.Unmarshal(mapiMachine.Spec.ProviderSpec.Value.Raw, &finalSpec)).To(Succeed())
+		Expect(finalSpec.LoadBalancers).To(Equal(originalSpec.LoadBalancers), "load balancers should not change after round trip")
+	})
 })
 
 var _ = Describe("validateLoadBalancerReferencesAgainstExpected", func() {

pkg/controllers/machinesync/test_helpers_test.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2025 Red Hat, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package machinesync
+
+import (
+	"context"
+
+	. "github.com/onsi/gomega"
+	mapiv1beta1 "github.com/openshift/api/machine/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/komega"
+)
+
+// transitionToAuthoritativeAPI transitions the AuthoritativeAPI of a machine to the target API.
+// It sets spec.AuthoritativeAPI to the target API and then transitions status.AuthoritativeAPI through the Migrating state to the target API.
+// This simulates the migration controller behavior for unit tests.
+func transitionToAuthoritativeAPI(ctx context.Context, k8sClient client.Client, k komega.Komega, machine *mapiv1beta1.Machine, targetAPI mapiv1beta1.MachineAuthority, timeout interface{}) {
+	if machine.Spec.AuthoritativeAPI == targetAPI && machine.Status.AuthoritativeAPI == targetAPI {
+		return
+	}
+
+	if machine.Spec.AuthoritativeAPI != targetAPI {
+		Eventually(k.Update(machine, func() {
+			machine.Spec.AuthoritativeAPI = targetAPI
+		}), timeout).Should(Succeed(), "Failed to update Machine spec.AuthoritativeAPI to %s", targetAPI)
+	}
+
+	// AuthoritativeAPI must transition through Migrating state
+	if machine.Status.AuthoritativeAPI != mapiv1beta1.MachineAuthorityMigrating {
+		Eventually(k.UpdateStatus(machine, func() {
+			machine.Status.AuthoritativeAPI = mapiv1beta1.MachineAuthorityMigrating
+		}), timeout).Should(Succeed(), "Failed to transition Machine status.AuthoritativeAPI to Migrating")
+	}
+
+	Eventually(k.UpdateStatus(machine, func() {
+		machine.Status.AuthoritativeAPI = targetAPI
+	}), timeout).Should(Succeed(), "Failed to transition Machine status.AuthoritativeAPI to %s", targetAPI)
+}

pkg/conversion/mapi2capi/util.go

@@ -55,6 +55,11 @@ func convertMAPILabelsToCAPI(mapiLabels map[string]string) map[string]string {
 			capiLabelKey, capiLabelVal := transformFunc(mapiLabelVal)
 			capiLabels[capiLabelKey] = capiLabelVal
 
+			// If the machine role is "master", also add the control-plane label.
+			if mapiLabelKey == "machine.openshift.io/cluster-api-machine-role" && mapiLabelVal == "master" {
+				capiLabels[clusterv1.MachineControlPlaneLabel] = ""
+			}
+
 			continue
 		}