diff --git a/pkg/controllers/machinesync/aws_test.go b/pkg/controllers/machinesync/aws_test.go index 3e5a79558..0c59dc3b2 100644 --- a/pkg/controllers/machinesync/aws_test.go +++ b/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(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(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() { diff --git a/pkg/controllers/machinesync/helpers_test.go b/pkg/controllers/machinesync/helpers_test.go new file mode 100644 index 000000000..4b6c5203b --- /dev/null +++ b/pkg/controllers/machinesync/helpers_test.go @@ -0,0 +1,49 @@ +/* +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 ( + . "github.com/onsi/gomega" + mapiv1beta1 "github.com/openshift/api/machine/v1beta1" + "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(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) +} diff --git a/pkg/conversion/mapi2capi/util.go b/pkg/conversion/mapi2capi/util.go index acf61edcd..a506e9e68 100644 --- a/pkg/conversion/mapi2capi/util.go +++ b/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 }