From c4dbfce6f3a17a8d725dfd0ed0e296a63a2db83a Mon Sep 17 00:00:00 2001 From: Theo Barber-Bany Date: Fri, 3 May 2024 16:54:57 +0200 Subject: [PATCH] Implements paused condition for kubeadm kcp --- .../internal/controllers/controller.go | 62 ++++++----- .../internal/controllers/controller_test.go | 101 ++++++++++++++++++ 2 files changed, 139 insertions(+), 24 deletions(-) diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go index a77071841c34..4b65d6c771aa 100644 --- a/controlplane/kubeadm/internal/controllers/controller.go +++ b/controlplane/kubeadm/internal/controllers/controller.go @@ -161,11 +161,6 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl. log = log.WithValues("Cluster", klog.KObj(cluster)) ctx = ctrl.LoggerInto(ctx, log) - if annotations.IsPaused(cluster, kcp) { - log.Info("Reconciliation is paused for this object") - return ctrl.Result{}, nil - } - // Initialize the patch helper. patchHelper, err := patch.NewHelper(kcp, r.Client) if err != nil { @@ -181,11 +176,14 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl. // patch and return right away instead of reusing the main defer, // because the main defer may take too much time to get cluster status // Patch ObservedGeneration only if the reconciliation completed successfully + + // TODO theobarberbany: Is this ordering correct, do we want finalizer to + // take priority over the paused condition? patchOpts := []patch.Option{patch.WithStatusObservedGeneration{}} if err := patchHelper.Patch(ctx, kcp, patchOpts...); err != nil { return ctrl.Result{}, errors.Wrapf(err, "failed to add finalizer") } - + log.Info("Returning early to add finalizer") return ctrl.Result{}, nil } @@ -193,15 +191,13 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl. // adopt them if necessary. controlPlane, adoptableMachineFound, err := r.initControlPlaneScope(ctx, cluster, kcp) if err != nil { + log.Error(err, "Failed to initControlPlaneScope") return ctrl.Result{}, err } - if adoptableMachineFound { - // if there are no errors but at least one CP machine has been adopted, then requeue and - // wait for the update event for the ownership to be set. - return ctrl.Result{}, nil - } + log.Info("initControlPlaneScope") defer func() { + log.Info("start of deferred update status") // Always attempt to update status. if err := r.updateStatus(ctx, controlPlane); err != nil { var connFailure *internal.RemoteClusterConnectionError @@ -222,6 +218,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl. log.Error(err, "Failed to patch KubeadmControlPlane") reterr = kerrors.NewAggregate([]error{reterr, err}) } + log.Info("patched KubeadmControlPlane") // Only requeue if there is no error, Requeue or RequeueAfter and the object does not have a deletion timestamp. if reterr == nil && res.IsZero() && kcp.ObjectMeta.DeletionTimestamp.IsZero() { @@ -243,6 +240,21 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl. } }() + if annotations.IsPaused(cluster, kcp) { + log.Info("Reconciliation is paused for this object") + conditions.MarkTrue(kcp, clusterv1.PausedCondition) + return ctrl.Result{}, nil + } + log.Info("Object not paused") + conditions.MarkFalse(kcp, clusterv1.PausedCondition, clusterv1.ResourceNotPausedReason, clusterv1.ConditionSeverityInfo, "Resource is operating as expected") + + if adoptableMachineFound { + // if there are no errors but at least one CP machine has been adopted, then requeue and + // wait for the update event for the ownership to be set. + log.Info("Returning early, adoptableMachineFound") + return ctrl.Result{}, nil + } + if !kcp.ObjectMeta.DeletionTimestamp.IsZero() { // Handle deletion reconciliation loop. res, err = r.reconcileDelete(ctx, controlPlane) @@ -325,19 +337,21 @@ func patchKubeadmControlPlane(ctx context.Context, patchHelper *patch.Helper, kc ) // Patch the object, ignoring conflicts on the conditions owned by this controller. - // Also, if requested, we are adding additional options like e.g. Patch ObservedGeneration when issuing the - // patch at the end of the reconcile loop. - options = append(options, patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ - controlplanev1.MachinesCreatedCondition, - clusterv1.ReadyCondition, - controlplanev1.MachinesSpecUpToDateCondition, - controlplanev1.ResizedCondition, - controlplanev1.MachinesReadyCondition, - controlplanev1.AvailableCondition, - controlplanev1.CertificatesAvailableCondition, - }}) - - return patchHelper.Patch(ctx, kcp, options...) + return patchHelper.Patch( + ctx, + kcp, + patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ + controlplanev1.MachinesCreatedCondition, + clusterv1.ReadyCondition, + clusterv1.PausedCondition, + controlplanev1.MachinesSpecUpToDateCondition, + controlplanev1.ResizedCondition, + controlplanev1.MachinesReadyCondition, + controlplanev1.AvailableCondition, + controlplanev1.CertificatesAvailableCondition, + }}, + patch.WithStatusObservedGeneration{}, + ) } // reconcile handles KubeadmControlPlane reconciliation. diff --git a/controlplane/kubeadm/internal/controllers/controller_test.go b/controlplane/kubeadm/internal/controllers/controller_test.go index 5a3c8971f632..4d2761aa2ad1 100644 --- a/controlplane/kubeadm/internal/controllers/controller_test.go +++ b/controlplane/kubeadm/internal/controllers/controller_test.go @@ -64,6 +64,10 @@ import ( "sigs.k8s.io/cluster-api/util/secret" ) +const ( + timeout = time.Second * 30 +) + func TestClusterToKubeadmControlPlane(t *testing.T) { g := NewWithT(t) fakeClient := newFakeClient() @@ -391,6 +395,15 @@ func TestReconcilePaused(t *testing.T) { Client: fakeClient, SecretCachingClient: fakeClient, recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Management: &internal.Management{Client: env}, + Workload: fakeWorkloadCluster{ + Workload: &internal.Workload{ + Client: env, + }, + Status: internal.ClusterStatus{}, + }, + }, } _, err = r.Reconcile(ctx, ctrl.Request{NamespacedName: util.ObjectKey(kcp)}) @@ -2280,6 +2293,94 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { }) } +func TestReconcilePausedCondition(t *testing.T) { + g := NewWithT(t) + + ns, err := env.CreateNamespace(ctx, "test-reconcile-pause-condition") + g.Expect(err).ToNot(HaveOccurred()) + + cluster, kcp, _ := createClusterWithControlPlane(ns.Name) + g.Expect(env.Create(ctx, cluster)).To(Succeed()) + g.Expect(env.Create(ctx, kcp)).To(Succeed()) + defer func(do ...client.Object) { + g.Expect(env.Cleanup(ctx, do...)).To(Succeed()) + }(cluster, kcp, ns) + + // Set cluster.status.InfrastructureReady so we actually enter in the reconcile loop + patch := client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf("{\"status\":{\"infrastructureReady\":%t}}", true))) + g.Expect(env.Status().Patch(ctx, cluster, patch)).To(Succeed()) + + genericInfrastructureMachineTemplate := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "GenericInfrastructureMachineTemplate", + "apiVersion": "infrastructure.cluster.x-k8s.io/v1beta1", + "metadata": map[string]interface{}{ + "name": "infra-foo", + "namespace": cluster.Namespace, + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "hello": "world", + }, + }, + }, + }, + } + g.Expect(env.Create(ctx, genericInfrastructureMachineTemplate)).To(Succeed()) + + r := &KubeadmControlPlaneReconciler{ + Client: env, + SecretCachingClient: secretCachingClient, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Management: &internal.Management{Client: env}, + Workload: fakeWorkloadCluster{ + Workload: &internal.Workload{ + Client: env, + }, + Status: internal.ClusterStatus{}, + }, + }, + } + + // We start unpaused + g.Eventually(func() bool { + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: util.ObjectKey(kcp)}) + g.Expect(err).ToNot(HaveOccurred()) + + key := util.ObjectKey(kcp) + if err := env.Get(ctx, key, kcp); err != nil { + return false + } + + // Checks the condition is set + if !conditions.Has(kcp, clusterv1.PausedCondition) { + return false + } + // The condition is set to false + return conditions.IsFalse(kcp, clusterv1.PausedCondition) + }, timeout).Should(BeTrue()) + + // Pause the cluster + cluster.Spec.Paused = true + g.Expect(env.Update(ctx, cluster)).To(Succeed()) + + g.Eventually(func() bool { + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: util.ObjectKey(kcp)}) + g.Expect(err).ToNot(HaveOccurred()) + + key := util.ObjectKey(kcp) + if err := env.Get(ctx, key, kcp); err != nil { + return false + } + + // The condition is set to true + return conditions.IsTrue(kcp, clusterv1.PausedCondition) + }, timeout).Should(BeTrue()) + +} + // test utils. func newFakeClient(initObjs ...client.Object) client.Client {