machinesync: Create Cluster API InfraMachine if not exists while Cluster API Machine exists

Author: chrischdi

pkg/controllers/machinesync/machine_sync_controller.go

@@ -191,14 +191,14 @@ func (r *MachineSyncReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 // Reconcile reconciles CAPI and MAPI machines for their respective namespaces.
 //
-//nolint:funlen
+//nolint:funlen,cyclop
 func (r *MachineSyncReconciler) Reconcile(ctx context.Context, req reconcile.Request) (ctrl.Result, error) {
 	logger := logf.FromContext(ctx, "namespace", req.Namespace, "name", req.Name)
 
 	logger.V(1).Info("Reconciling machine")
 	defer logger.V(1).Info("Finished reconciling machine")
 
-	var mapiMachineNotFound, capiMachineNotFound bool
+	var mapiMachineNotFound, capiMachineNotFound, capiInfraMachineExists bool
 
 	// Get the MAPI Machine.
 	mapiMachine := &mapiv1beta1.Machine{}
@@ -254,12 +254,24 @@ func (r *MachineSyncReconciler) Reconcile(ctx context.Context, req reconcile.Req
 		return ctrl.Result{}, nil
 	}
 
+	// Check for existence of the Cluster API Infrastructure Machine or if it needs to get created from MAPI first.
+	capiInfraMachineExists, err := r.doesCAPIInfraMachineExist(ctx, capiMachine, mapiMachine)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to check for existence of Cluster API infrastructure machine: %w", err)
+	}
+
 	authoritativeAPI := mapiMachine.Status.AuthoritativeAPI
 
 	switch {
 	case authoritativeAPI == mapiv1beta1.MachineAuthorityMachineAPI:
 		return r.reconcileMAPIMachinetoCAPIMachine(ctx, mapiMachine, capiMachine)
 	case authoritativeAPI == mapiv1beta1.MachineAuthorityClusterAPI && !capiMachineNotFound:
+		// Create Cluster API Infrastructure Machine from MAPI if it doesn't exist
+		// the CAPI machine is not marked for deletion and the MAPI machine was created before the Cluster API machine.
+		if !capiInfraMachineExists && capiMachine.DeletionTimestamp.IsZero() && !mapiMachine.CreationTimestamp.After(capiMachine.CreationTimestamp.Time) {
+			return r.reconcileMAPIMachinetoCAPIMachine(ctx, mapiMachine, capiMachine)
+		}
+
 		return r.reconcileCAPIMachinetoMAPIMachine(ctx, capiMachine, mapiMachine)
 	case authoritativeAPI == mapiv1beta1.MachineAuthorityClusterAPI && capiMachineNotFound:
 		return r.reconcileMAPIMachinetoCAPIMachine(ctx, mapiMachine, capiMachine)
@@ -987,18 +999,14 @@ func (r *MachineSyncReconciler) fetchCAPIInfraResources(ctx context.Context, cap
 		return nil, nil, nil
 	}
 
-	var infraCluster, infraMachine client.Object
+	var infraCluster client.Object
 
 	infraClusterKey := client.ObjectKey{
 		Namespace: capiMachine.Namespace,
 		Name:      capiMachine.Spec.ClusterName,
 	}
 
 	infraMachineRef := capiMachine.Spec.InfrastructureRef
-	infraMachineKey := client.ObjectKey{
-		Namespace: infraMachineRef.Namespace,
-		Name:      infraMachineRef.Name,
-	}
 
 	// Validate that required references are not empty to avoid nil pointer issues later.
 	// These are terminal configuration errors that require user intervention.
@@ -1012,7 +1020,7 @@ func (r *MachineSyncReconciler) fetchCAPIInfraResources(ctx context.Context, cap
 			capiMachine.Namespace, capiMachine.Name, errInvalidInfraMachineReference)
 	}
 
-	infraMachine, infraCluster, err := controllers.InitInfraMachineAndInfraClusterFromProvider(r.Platform)
+	_, infraCluster, err := controllers.InitInfraMachineAndInfraClusterFromProvider(r.Platform)
 	if err != nil {
 		return nil, nil, fmt.Errorf("unable to devise Cluster API infra resources: %w", err)
 	}
@@ -1021,13 +1029,50 @@ func (r *MachineSyncReconciler) fetchCAPIInfraResources(ctx context.Context, cap
 		return nil, nil, fmt.Errorf("failed to get Cluster API infrastructure cluster: %w", err)
 	}
 
+	infraMachine, err := r.fetchCAPIInfraMachine(ctx, infraMachineRef.Name, infraMachineRef.Namespace)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to fetch Cluster API infrastructure machine: %w", err)
+	}
+
+	return infraCluster, infraMachine, nil
+}
+
+func (r *MachineSyncReconciler) fetchCAPIInfraMachine(ctx context.Context, name, namespace string) (client.Object, error) {
+	infraMachine, _, err := controllers.InitInfraMachineAndInfraClusterFromProvider(r.Platform)
+	if err != nil {
+		return nil, fmt.Errorf("unable to devise Cluster API infra resources: %w", err)
+	}
+
+	infraMachineKey := client.ObjectKey{
+		Namespace: namespace,
+		Name:      name,
+	}
+
 	if err := r.Get(ctx, infraMachineKey, infraMachine); err != nil && !apierrors.IsNotFound(err) {
-		return nil, nil, fmt.Errorf("failed to get Cluster API infrastructure machine: %w", err)
+		return nil, fmt.Errorf("failed to get: %w", err)
 	} else if apierrors.IsNotFound(err) {
 		infraMachine = nil
 	}
 
-	return infraCluster, infraMachine, nil
+	return infraMachine, nil
+}
+
+// doesCAPIInfraMachineExist checks if the Cluster API Infrastructure Machine exists. It uses the infrastructureRef of the Cluster API Machine with fallback to the name of the MAPI Machine.
+func (r *MachineSyncReconciler) doesCAPIInfraMachineExist(ctx context.Context, capiMachine *clusterv1.Machine, mapiMachine *mapiv1beta1.Machine) (bool, error) {
+	namespace := r.CAPINamespace
+	name := mapiMachine.Name
+
+	if capiMachine != nil {
+		name = capiMachine.Spec.InfrastructureRef.Name
+		namespace = capiMachine.Spec.InfrastructureRef.Namespace
+	}
+
+	infraMachine, err := r.fetchCAPIInfraMachine(ctx, name, namespace)
+	if err != nil {
+		return false, fmt.Errorf("checking existence: %w", err)
+	}
+
+	return infraMachine != nil, nil
 }
 
 //nolint:funlen,gocognit,cyclop

pkg/controllers/machinesync/machine_sync_controller_test.go

@@ -949,20 +949,19 @@ var _ = Describe("With a running MachineSync Reconciler", func() {
 					})
 
 					Context("and the InfraMachine does not exist", func() {
-						It("should update the synchronized condition on the MAPI machine to False", func() {
-							// TODO: Revert this to a useful error message.
-							// We changed how fetchCAPIInfraResources behaves, so now we fail at a later point in the code.
-							// This still fails, the behaviour is the same - it's just a less useful error for an end user.
-							// We need to revisit fetchCAPIInfraResources.
+						It("should create the Cluster API InfraMachine", func() {
 							Eventually(k.Object(mapiMachine), timeout).Should(
 								HaveField("Status.Conditions", ContainElement(
 									SatisfyAll(
 										HaveField("Type", Equal(consts.SynchronizedCondition)),
-										HaveField("Status", Equal(corev1.ConditionFalse)),
-										HaveField("Reason", Equal("FailedToConvertCAPIMachineToMAPI")),
-										HaveField("Message", ContainSubstring("unexpected InfraMachine type, expected AWSMachine, got <nil>")),
+										HaveField("Status", Equal(corev1.ConditionTrue)),
+										HaveField("Reason", Equal("ResourceSynchronized")),
+										HaveField("Message", ContainSubstring("Successfully synchronized CAPI Machine to MAPI")),
 									))),
 							)
+
+							capiInfraMachine := awsv1resourcebuilder.AWSMachine().WithName(mapiMachine.Name).WithNamespace(capiNamespace.Name).Build()
+							Eventually(k.Get(capiInfraMachine), timeout).Should(Succeed())
 						})
 					})
 				})