From 0fb2fc8389de34908d82995268ce9c4d32538a93 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Mon, 18 Mar 2024 17:16:01 +0100 Subject: [PATCH] Avoid leaving orphaned InfrastructureCluster when create control plane fails --- .../topology/cluster/reconcile_state.go | 77 ++++--- .../topology/cluster/reconcile_state_test.go | 216 ++++++++++++++++-- 2 files changed, 238 insertions(+), 55 deletions(-) diff --git a/internal/controllers/topology/cluster/reconcile_state.go b/internal/controllers/topology/cluster/reconcile_state.go index a375f58bf9b6..583d37c6bec4 100644 --- a/internal/controllers/topology/cluster/reconcile_state.go +++ b/internal/controllers/topology/cluster/reconcile_state.go @@ -27,6 +27,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/wait" @@ -75,17 +76,30 @@ func (r *Reconciler) reconcileState(ctx context.Context, s *scope.Scope) error { } // Reconcile desired state of the InfrastructureCluster object. - if err := r.reconcileInfrastructureCluster(ctx, s); err != nil { - return err + createdInfraCluster, errInfraCluster := r.reconcileInfrastructureCluster(ctx, s) + if errInfraCluster != nil { + return errInfraCluster } // Reconcile desired state of the ControlPlane object. - if err := r.reconcileControlPlane(ctx, s); err != nil { - return err + createdControlPlane, errControlPlane := r.reconcileControlPlane(ctx, s) + if errControlPlane != nil { + // NOTE: report control plane error immediately only if we did not just create the infrastructure cluster; otherwise attempt reconcile cluster before returning. + if !createdInfraCluster { + return errControlPlane + } + + // In this case (reconcileInfrastructureCluster passed reporting creation of the infrastructure cluster object, reconcileControlPlane - which is expected to create the control plane object - failed), + // if the creation of the control plane actually did not happen, blank out ControlPlaneRef from desired cluster. + if s.Current.Cluster.Spec.ControlPlaneRef == nil && !createdControlPlane { + s.Desired.Cluster.Spec.ControlPlaneRef = nil + } } // Reconcile desired state of the Cluster object. - if err := r.reconcileCluster(ctx, s); err != nil { + errCluster := r.reconcileCluster(ctx, s) + err := kerrors.NewAggregate([]error{errControlPlane, errCluster}) + if err != nil { return err } @@ -283,12 +297,12 @@ func (r *Reconciler) callAfterClusterUpgrade(ctx context.Context, s *scope.Scope } // reconcileInfrastructureCluster reconciles the desired state of the InfrastructureCluster object. -func (r *Reconciler) reconcileInfrastructureCluster(ctx context.Context, s *scope.Scope) error { +func (r *Reconciler) reconcileInfrastructureCluster(ctx context.Context, s *scope.Scope) (bool, error) { ctx, _ = tlog.LoggerFrom(ctx).WithObject(s.Desired.InfrastructureCluster).Into(ctx) ignorePaths, err := contract.InfrastructureCluster().IgnorePaths(s.Desired.InfrastructureCluster) if err != nil { - return errors.Wrap(err, "failed to calculate ignore paths") + return false, errors.Wrap(err, "failed to calculate ignore paths") } return r.reconcileReferencedObject(ctx, reconcileReferencedObjectInput{ @@ -301,14 +315,14 @@ func (r *Reconciler) reconcileInfrastructureCluster(ctx context.Context, s *scop // reconcileControlPlane works to bring the current state of a managed topology in line with the desired state. This involves // updating the cluster where needed. -func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) error { +func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) (bool, error) { // If the ControlPlane has defined a current or desired MachineHealthCheck attempt to reconcile it. // MHC changes are not Kubernetes version dependent, therefore proceed with MHC reconciliation // even if the Control Plane is pending an upgrade. if s.Desired.ControlPlane.MachineHealthCheck != nil || s.Current.ControlPlane.MachineHealthCheck != nil { // Reconcile the current and desired state of the MachineHealthCheck. if err := r.reconcileMachineHealthCheck(ctx, s.Current.ControlPlane.MachineHealthCheck, s.Desired.ControlPlane.MachineHealthCheck); err != nil { - return err + return false, err } } @@ -316,7 +330,7 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) // Do not reconcile the control plane yet to avoid updating the control plane while it is still pending a // version upgrade. This will prevent the control plane from performing a double rollout. if s.UpgradeTracker.ControlPlane.IsPendingUpgrade { - return nil + return false, nil } // If the clusterClass mandates the controlPlane has infrastructureMachines, reconcile it. if s.Blueprint.HasControlPlaneInfrastructureMachine() { @@ -324,7 +338,7 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) cpInfraRef, err := contract.ControlPlane().MachineTemplate().InfrastructureRef().Get(s.Desired.ControlPlane.Object) if err != nil { - return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.InfrastructureMachineTemplate}) + return false, errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.InfrastructureMachineTemplate}) } // Create or update the MachineInfrastructureTemplate of the control plane. @@ -337,25 +351,26 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) templateNamePrefix: controlPlaneInfrastructureMachineTemplateNamePrefix(s.Current.Cluster.Name), }, ); err != nil { - return err + return false, err } // The controlPlaneObject.Spec.machineTemplate.infrastructureRef has to be updated in the desired object err = contract.ControlPlane().MachineTemplate().InfrastructureRef().Set(s.Desired.ControlPlane.Object, refToUnstructured(cpInfraRef)) if err != nil { - return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object}) + return false, errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object}) } } // Create or update the ControlPlaneObject for the ControlPlaneState. ctx, _ = tlog.LoggerFrom(ctx).WithObject(s.Desired.ControlPlane.Object).Into(ctx) - if err := r.reconcileReferencedObject(ctx, reconcileReferencedObjectInput{ + created, err := r.reconcileReferencedObject(ctx, reconcileReferencedObjectInput{ cluster: s.Current.Cluster, current: s.Current.ControlPlane.Object, desired: s.Desired.ControlPlane.Object, versionGetter: contract.ControlPlane().Version().Get, - }); err != nil { - return err + }) + if err != nil { + return created, err } // If the controlPlane has infrastructureMachines and the InfrastructureMachineTemplate has changed on this reconcile @@ -364,7 +379,7 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) if s.Blueprint.HasControlPlaneInfrastructureMachine() && s.Current.ControlPlane.InfrastructureMachineTemplate != nil { if s.Current.ControlPlane.InfrastructureMachineTemplate.GetName() != s.Desired.ControlPlane.InfrastructureMachineTemplate.GetName() { if err := r.Client.Delete(ctx, s.Current.ControlPlane.InfrastructureMachineTemplate); err != nil { - return errors.Wrapf(err, "failed to delete oldinfrastructure machine template %s of control plane %s", + return created, errors.Wrapf(err, "failed to delete oldinfrastructure machine template %s of control plane %s", tlog.KObj{Obj: s.Current.ControlPlane.InfrastructureMachineTemplate}, tlog.KObj{Obj: s.Current.ControlPlane.Object}, ) @@ -372,7 +387,7 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) } } - return nil + return created, nil } // reconcileMachineHealthCheck creates, updates, deletes or leaves untouched a MachineHealthCheck depending on the difference between the @@ -824,7 +839,7 @@ func (r *Reconciler) createMachinePool(ctx context.Context, s *scope.Scope, mp * log := tlog.LoggerFrom(ctx).WithMachinePool(mp.Object) cluster := s.Current.Cluster infraCtx, _ := log.WithObject(mp.InfrastructureMachinePoolObject).Into(ctx) - if err := r.reconcileReferencedObject(infraCtx, reconcileReferencedObjectInput{ + if _, err := r.reconcileReferencedObject(infraCtx, reconcileReferencedObjectInput{ cluster: cluster, desired: mp.InfrastructureMachinePoolObject, }); err != nil { @@ -832,7 +847,7 @@ func (r *Reconciler) createMachinePool(ctx context.Context, s *scope.Scope, mp * } bootstrapCtx, _ := log.WithObject(mp.BootstrapObject).Into(ctx) - if err := r.reconcileReferencedObject(bootstrapCtx, reconcileReferencedObjectInput{ + if _, err := r.reconcileReferencedObject(bootstrapCtx, reconcileReferencedObjectInput{ cluster: cluster, desired: mp.BootstrapObject, }); err != nil { @@ -883,7 +898,7 @@ func (r *Reconciler) updateMachinePool(ctx context.Context, s *scope.Scope, curr cluster := s.Current.Cluster infraCtx, _ := log.WithObject(desiredMP.InfrastructureMachinePoolObject).Into(ctx) - if err := r.reconcileReferencedObject(infraCtx, reconcileReferencedObjectInput{ + if _, err := r.reconcileReferencedObject(infraCtx, reconcileReferencedObjectInput{ cluster: cluster, current: currentMP.InfrastructureMachinePoolObject, desired: desiredMP.InfrastructureMachinePoolObject, @@ -892,7 +907,7 @@ func (r *Reconciler) updateMachinePool(ctx context.Context, s *scope.Scope, curr } bootstrapCtx, _ := log.WithObject(desiredMP.BootstrapObject).Into(ctx) - if err := r.reconcileReferencedObject(bootstrapCtx, reconcileReferencedObjectInput{ + if _, err := r.reconcileReferencedObject(bootstrapCtx, reconcileReferencedObjectInput{ cluster: cluster, current: currentMP.BootstrapObject, desired: desiredMP.BootstrapObject, @@ -1022,7 +1037,7 @@ type reconcileReferencedObjectInput struct { // reconcileReferencedObject reconciles the desired state of the referenced object. // NOTE: After a referenced object is created it is assumed that the reference should // never change (only the content of the object can eventually change). Thus, we are checking for strict compatibility. -func (r *Reconciler) reconcileReferencedObject(ctx context.Context, in reconcileReferencedObjectInput) error { +func (r *Reconciler) reconcileReferencedObject(ctx context.Context, in reconcileReferencedObjectInput) (bool, error) { log := tlog.LoggerFrom(ctx) // If there is no current object, create it. @@ -1030,36 +1045,36 @@ func (r *Reconciler) reconcileReferencedObject(ctx context.Context, in reconcile log.Infof("Creating %s", tlog.KObj{Obj: in.desired}) helper, err := r.patchHelperFactory(ctx, nil, in.desired, structuredmerge.IgnorePaths(in.ignorePaths)) if err != nil { - return errors.Wrap(createErrorWithoutObjectName(ctx, err, in.desired), "failed to create patch helper") + return false, errors.Wrap(createErrorWithoutObjectName(ctx, err, in.desired), "failed to create patch helper") } if err := helper.Patch(ctx); err != nil { - return createErrorWithoutObjectName(ctx, err, in.desired) + return false, createErrorWithoutObjectName(ctx, err, in.desired) } r.recorder.Eventf(in.cluster, corev1.EventTypeNormal, createEventReason, "Created %q", tlog.KObj{Obj: in.desired}) - return nil + return true, nil } // Check if the current and desired referenced object are compatible. if allErrs := check.ObjectsAreStrictlyCompatible(in.current, in.desired); len(allErrs) > 0 { - return allErrs.ToAggregate() + return false, allErrs.ToAggregate() } // Check differences between current and desired state, and eventually patch the current object. patchHelper, err := r.patchHelperFactory(ctx, in.current, in.desired, structuredmerge.IgnorePaths(in.ignorePaths)) if err != nil { - return errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: in.current}) + return false, errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: in.current}) } if !patchHelper.HasChanges() { log.V(3).Infof("No changes for %s", tlog.KObj{Obj: in.desired}) - return nil + return false, nil } log.Infof("Patching %s", tlog.KObj{Obj: in.desired}) if err := patchHelper.Patch(ctx); err != nil { - return errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: in.current}) + return false, errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: in.current}) } r.recorder.Eventf(in.cluster, corev1.EventTypeNormal, updateEventReason, "Updated %q%s", tlog.KObj{Obj: in.desired}, logUnstructuredVersionChange(in.current, in.desired, in.versionGetter)) - return nil + return false, nil } func logUnstructuredVersionChange(current, desired *unstructured.Unstructured, versionGetter unstructuredVersionGetter) string { diff --git a/internal/controllers/topology/cluster/reconcile_state_test.go b/internal/controllers/topology/cluster/reconcile_state_test.go index 7dffa9f535be..e437a38803cc 100644 --- a/internal/controllers/topology/cluster/reconcile_state_test.go +++ b/internal/controllers/topology/cluster/reconcile_state_test.go @@ -1202,14 +1202,16 @@ func TestReconcileInfrastructureCluster(t *testing.T) { externalChanges string desired *unstructured.Unstructured want *unstructured.Unstructured + wantCreated bool wantErr bool }{ { - name: "Should create desired InfrastructureCluster if the current does not exists yet", - original: nil, - desired: clusterInfrastructure1, - want: clusterInfrastructure1, - wantErr: false, + name: "Should create desired InfrastructureCluster if the current does not exists yet", + original: nil, + desired: clusterInfrastructure1, + want: clusterInfrastructure1, + wantCreated: true, + wantErr: false, }, { name: "No-op if current InfrastructureCluster is equal to desired", @@ -1280,12 +1282,13 @@ func TestReconcileInfrastructureCluster(t *testing.T) { patchHelperFactory: serverSideApplyPatchHelperFactory(env, ssa.NewCache()), recorder: env.GetEventRecorderFor("test"), } - err = r.reconcileInfrastructureCluster(ctx, s) + created, err := r.reconcileInfrastructureCluster(ctx, s) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return } g.Expect(err).ToNot(HaveOccurred()) + g.Expect(created).To(Equal(tt.wantCreated)) got := tt.want.DeepCopy() // this is required otherwise Get will modify tt.want err = env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(tt.want), got) @@ -1383,17 +1386,19 @@ func TestReconcileControlPlane(t *testing.T) { upgradeTracker *scope.UpgradeTracker desired *scope.ControlPlaneState want *scope.ControlPlaneState + wantCreated bool wantRotation bool wantErr bool }{ // Testing reconciliation of a control plane without machines. { - name: "Should create desired ControlPlane without machine infrastructure if the current does not exist", - class: ccWithoutControlPlaneInfrastructure, - original: nil, - desired: &scope.ControlPlaneState{Object: controlPlaneWithoutInfrastructure.DeepCopy()}, - want: &scope.ControlPlaneState{Object: controlPlaneWithoutInfrastructure.DeepCopy()}, - wantErr: false, + name: "Should create desired ControlPlane without machine infrastructure if the current does not exist", + class: ccWithoutControlPlaneInfrastructure, + original: nil, + desired: &scope.ControlPlaneState{Object: controlPlaneWithoutInfrastructure.DeepCopy()}, + want: &scope.ControlPlaneState{Object: controlPlaneWithoutInfrastructure.DeepCopy()}, + wantCreated: true, + wantErr: false, }, { name: "Should update the ControlPlane without machine infrastructure", @@ -1440,12 +1445,13 @@ func TestReconcileControlPlane(t *testing.T) { // Testing reconciliation of a control plane with machines. { - name: "Should create desired ControlPlane with machine infrastructure if the current does not exist", - class: ccWithControlPlaneInfrastructure, - original: nil, - desired: &scope.ControlPlaneState{Object: controlPlaneWithInfrastructure.DeepCopy(), InfrastructureMachineTemplate: infrastructureMachineTemplate.DeepCopy()}, - want: &scope.ControlPlaneState{Object: controlPlaneWithInfrastructure.DeepCopy(), InfrastructureMachineTemplate: infrastructureMachineTemplate.DeepCopy()}, - wantErr: false, + name: "Should create desired ControlPlane with machine infrastructure if the current does not exist", + class: ccWithControlPlaneInfrastructure, + original: nil, + desired: &scope.ControlPlaneState{Object: controlPlaneWithInfrastructure.DeepCopy(), InfrastructureMachineTemplate: infrastructureMachineTemplate.DeepCopy()}, + want: &scope.ControlPlaneState{Object: controlPlaneWithInfrastructure.DeepCopy(), InfrastructureMachineTemplate: infrastructureMachineTemplate.DeepCopy()}, + wantCreated: true, + wantErr: false, }, { name: "Should rotate machine infrastructure in case of changes to the desired template", @@ -1559,12 +1565,13 @@ func TestReconcileControlPlane(t *testing.T) { } // Run reconcileControlPlane with the states created in the initial section of the test. - err = r.reconcileControlPlane(ctx, s) + created, err := r.reconcileControlPlane(ctx, s) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return } g.Expect(err).ToNot(HaveOccurred()) + g.Expect(created).To(Equal(tt.wantCreated)) // Create ControlPlane object for fetching data into gotControlPlaneObject := builder.TestControlPlane("", "").Build() @@ -1814,7 +1821,7 @@ func TestReconcileControlPlaneMachineHealthCheck(t *testing.T) { } // Run reconcileControlPlane with the states created in the initial section of the test. - err = r.reconcileControlPlane(ctx, s) + _, err = r.reconcileControlPlane(ctx, s) g.Expect(err).ToNot(HaveOccurred()) gotCP := s.Desired.ControlPlane.Object.DeepCopy() @@ -2554,7 +2561,8 @@ func TestReconcileReferencedObjectSequences(t *testing.T) { // desired is the desired control-plane object handed over to reconcileReferencedObject. desired object // want is the expected control-plane object after calling reconcileReferencedObject. - want object + want object + wantCreated bool } tests := []struct { @@ -2595,6 +2603,7 @@ func TestReconcileReferencedObjectSequences(t *testing.T) { }, }, }, + wantCreated: true, }, reconcileStep{ name: "Drop enable-hostpath-provisioner", @@ -2639,6 +2648,7 @@ func TestReconcileReferencedObjectSequences(t *testing.T) { }, }, }, + wantCreated: true, }, reconcileStep{ name: "Drop the label with dots", @@ -2686,6 +2696,7 @@ func TestReconcileReferencedObjectSequences(t *testing.T) { "foo": "ccValue", }, }, + wantCreated: true, }, externalStep{ name: "User changes value", @@ -2745,6 +2756,7 @@ func TestReconcileReferencedObjectSequences(t *testing.T) { }, }, }, + wantCreated: true, }, externalStep{ name: "User adds an additional extraArg", @@ -2807,6 +2819,7 @@ func TestReconcileReferencedObjectSequences(t *testing.T) { "machineTemplate": map[string]interface{}{}, }, }, + wantCreated: true, }, externalStep{ name: "User adds an additional object", @@ -2989,12 +3002,14 @@ func TestReconcileReferencedObjectSequences(t *testing.T) { s.Desired.ControlPlane.Object.Object["spec"] = step.desired.spec } - // Execute a reconcile.0 - g.Expect(r.reconcileReferencedObject(ctx, reconcileReferencedObjectInput{ + // Execute a reconcile + created, err := r.reconcileReferencedObject(ctx, reconcileReferencedObjectInput{ cluster: s.Current.Cluster, current: s.Current.ControlPlane.Object, desired: s.Desired.ControlPlane.Object, - })).To(Succeed()) + }) + g.Expect(err).To(Succeed()) + g.Expect(created).To(Equal(step.wantCreated)) // Build the object for comparison. want := &unstructured.Unstructured{ @@ -3221,6 +3236,159 @@ func TestReconcileMachineDeploymentMachineHealthCheck(t *testing.T) { } } +func TestReconcileState(t *testing.T) { + t.Run("Cluster get reconciled with infrastructure Ref only when reconcileInfrastructureCluster pass and reconcileControlPlane fails ", func(t *testing.T) { + g := NewWithT(t) + + currentdCluster := builder.Cluster(metav1.NamespaceDefault, "cluster1").Build() + + infrastructureCluster := builder.TestInfrastructureCluster(metav1.NamespaceDefault, "infrastructure-cluster1").Build() + controlPlane := builder.TestControlPlane(metav1.NamespaceDefault, "controlplane-cluster1").Build() + desiredCluster := builder.Cluster(metav1.NamespaceDefault, "cluster1"). + WithInfrastructureCluster(infrastructureCluster). + WithControlPlane(controlPlane). + Build() + + // cluster requires a UID because reconcileClusterShim will create a cluster shim + // which has the cluster set as Owner in an OwnerReference. + // A valid OwnerReferences requires a uid. + currentdCluster.SetUID("foo") + + // NOTE: it is ok to use create given that the Cluster are created by user. + g.Expect(env.CreateAndWait(ctx, currentdCluster)).To(Succeed()) + + s := scope.New(currentdCluster) + s.Blueprint = &scope.ClusterBlueprint{ClusterClass: &clusterv1.ClusterClass{}} + s.Current.ControlPlane = &scope.ControlPlaneState{} + s.Desired = &scope.ClusterState{Cluster: desiredCluster, InfrastructureCluster: infrastructureCluster, ControlPlane: &scope.ControlPlaneState{Object: controlPlane}} + + // Create namespace and modify input to have correct namespace set + namespace, err := env.CreateNamespace(ctx, "reconcile-cluster") + g.Expect(err).ToNot(HaveOccurred()) + prepareControlPlaneState(g, s.Desired.ControlPlane, namespace.GetName()) + + // Force reconcile control plane to fail + controlPlane.SetNamespace("do-not-exist") + + r := Reconciler{ + Client: env, + patchHelperFactory: serverSideApplyPatchHelperFactory(env, ssa.NewCache()), + recorder: env.GetEventRecorderFor("test"), + } + err = r.reconcileState(ctx, s) + g.Expect(err).To(HaveOccurred()) + + got := currentdCluster.DeepCopy() + err = env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(currentdCluster), got) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(got.Spec.InfrastructureRef).ToNot(BeNil()) + g.Expect(got.Spec.ControlPlaneRef).To(BeNil()) + + g.Expect(env.CleanupAndWait(ctx, infrastructureCluster, currentdCluster)).To(Succeed()) + }) + t.Run("Cluster get reconciled with both infrastructure Ref and control plane ref when both reconcileInfrastructureCluster and reconcileControlPlane pass", func(t *testing.T) { + g := NewWithT(t) + + currentdCluster := builder.Cluster(metav1.NamespaceDefault, "cluster1").Build() + + infrastructureCluster := builder.TestInfrastructureCluster(metav1.NamespaceDefault, "infrastructure-cluster1").Build() + controlPlane := builder.TestControlPlane(metav1.NamespaceDefault, "controlplane-cluster1").Build() + desiredCluster := builder.Cluster(metav1.NamespaceDefault, "cluster1"). + WithInfrastructureCluster(infrastructureCluster). + WithControlPlane(controlPlane). + Build() + + // cluster requires a UID because reconcileClusterShim will create a cluster shim + // which has the cluster set as Owner in an OwnerReference. + // A valid OwnerReferences requires a uid. + currentdCluster.SetUID("foo") + + // NOTE: it is ok to use create given that the Cluster are created by user. + g.Expect(env.CreateAndWait(ctx, currentdCluster)).To(Succeed()) + + s := scope.New(currentdCluster) + s.Blueprint = &scope.ClusterBlueprint{ClusterClass: &clusterv1.ClusterClass{}} + s.Current.ControlPlane = &scope.ControlPlaneState{} + s.Desired = &scope.ClusterState{Cluster: desiredCluster, InfrastructureCluster: infrastructureCluster, ControlPlane: &scope.ControlPlaneState{Object: controlPlane}} + + // Create namespace and modify input to have correct namespace set + namespace, err := env.CreateNamespace(ctx, "reconcile-cluster") + g.Expect(err).ToNot(HaveOccurred()) + prepareControlPlaneState(g, s.Desired.ControlPlane, namespace.GetName()) + + r := Reconciler{ + Client: env, + patchHelperFactory: serverSideApplyPatchHelperFactory(env, ssa.NewCache()), + recorder: env.GetEventRecorderFor("test"), + } + err = r.reconcileState(ctx, s) + g.Expect(err).ToNot(HaveOccurred()) + + got := currentdCluster.DeepCopy() + err = env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(currentdCluster), got) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(got.Spec.InfrastructureRef).ToNot(BeNil()) + g.Expect(got.Spec.ControlPlaneRef).ToNot(BeNil()) + + g.Expect(env.CleanupAndWait(ctx, infrastructureCluster, controlPlane, currentdCluster)).To(Succeed()) + }) + t.Run("Cluster does not get reconciled when reconcileControlPlane fails and infrastructure Ref is set", func(t *testing.T) { + g := NewWithT(t) + + infrastructureCluster := builder.TestInfrastructureCluster(metav1.NamespaceDefault, "infrastructure-cluster1").Build() + controlPlane := builder.TestControlPlane(metav1.NamespaceDefault, "controlplane-cluster1").Build() + + currentdCluster := builder.Cluster(metav1.NamespaceDefault, "cluster1"). + WithInfrastructureCluster(infrastructureCluster). + Build() + + desiredCluster := builder.Cluster(metav1.NamespaceDefault, "cluster1"). + WithInfrastructureCluster(infrastructureCluster). + WithControlPlane(controlPlane). + Build() + + // cluster requires a UID because reconcileClusterShim will create a cluster shim + // which has the cluster set as Owner in an OwnerReference. + // A valid OwnerReferences requires a uid. + currentdCluster.SetUID("foo") + + // NOTE: it is ok to use create given that the Cluster are created by user. + g.Expect(env.CreateAndWait(ctx, currentdCluster)).To(Succeed()) + + s := scope.New(currentdCluster) + s.Blueprint = &scope.ClusterBlueprint{ClusterClass: &clusterv1.ClusterClass{}} + s.Current.ControlPlane = &scope.ControlPlaneState{} + s.Desired = &scope.ClusterState{Cluster: desiredCluster, InfrastructureCluster: infrastructureCluster, ControlPlane: &scope.ControlPlaneState{Object: controlPlane}} + + // Create namespace and modify input to have correct namespace set + namespace, err := env.CreateNamespace(ctx, "reconcile-cluster") + g.Expect(err).ToNot(HaveOccurred()) + prepareControlPlaneState(g, s.Desired.ControlPlane, namespace.GetName()) + + // Force reconcile control plane to fail + controlPlane.SetNamespace("do-not-exist") + + r := Reconciler{ + Client: env, + patchHelperFactory: serverSideApplyPatchHelperFactory(env, ssa.NewCache()), + recorder: env.GetEventRecorderFor("test"), + } + err = r.reconcileState(ctx, s) + g.Expect(err).To(HaveOccurred()) + + got := currentdCluster.DeepCopy() + err = env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(currentdCluster), got) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(got.Spec.InfrastructureRef).ToNot(BeNil()) + g.Expect(got.Spec.ControlPlaneRef).To(BeNil()) + + g.Expect(env.CleanupAndWait(ctx, infrastructureCluster, controlPlane, currentdCluster)).To(Succeed()) + }) +} + func newFakeMachineDeploymentTopologyState(name string, infrastructureMachineTemplate, bootstrapTemplate *unstructured.Unstructured, machineHealthCheck *clusterv1.MachineHealthCheck) *scope.MachineDeploymentState { mdState := &scope.MachineDeploymentState{ Object: builder.MachineDeployment(metav1.NamespaceDefault, name).