From 8295a4cb0df87f8f48a06bf2fa77851e32be7591 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Wed, 4 Sep 2024 19:05:12 +0200 Subject: [PATCH] KCP: remove etcd member in pre-terminate hook --- .../v1beta1/kubeadm_control_plane_types.go | 4 ++ .../kubeadm/internal/control_plane.go | 5 ++ .../internal/controllers/controller.go | 59 +++++++++++++++++++ .../internal/controllers/controller_test.go | 8 ++- .../kubeadm/internal/controllers/helpers.go | 3 + .../internal/controllers/helpers_test.go | 4 ++ .../internal/controllers/remediation.go | 5 -- .../kubeadm/internal/controllers/scale.go | 4 -- 8 files changed, 82 insertions(+), 10 deletions(-) diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go index 1a9ccf073831..51d28b348fd0 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go @@ -65,6 +65,10 @@ const ( // failures in updating remediation retry (the counter restarts from zero). RemediationForAnnotation = "controlplane.cluster.x-k8s.io/remediation-for" + // PreTerminateDeleteHookAnnotation is the annotation KCP sets on Machines to ensure it can later remove the + // etcd member right before Machine termination (i.e. before InfraMachine deletion). + PreTerminateDeleteHookAnnotation = clusterv1.PreTerminateDeleteHookAnnotationPrefix + "/kubeadmcontrolplane" + // DefaultMinHealthyPeriod defines the default minimum period before we consider a remediation on a // machine unrelated from the previous remediation. DefaultMinHealthyPeriod = 1 * time.Hour diff --git a/controlplane/kubeadm/internal/control_plane.go b/controlplane/kubeadm/internal/control_plane.go index bfed636b8326..6fd7a68a0bab 100644 --- a/controlplane/kubeadm/internal/control_plane.go +++ b/controlplane/kubeadm/internal/control_plane.go @@ -164,6 +164,11 @@ func (c *ControlPlane) HasDeletingMachine() bool { return len(c.Machines.Filter(collections.HasDeletionTimestamp)) > 0 } +// DeletingMachines returns machines in the control plane that are in the process of being deleted. +func (c *ControlPlane) DeletingMachines() collections.Machines { + return c.Machines.Filter(collections.HasDeletionTimestamp) +} + // GetKubeadmConfig returns the KubeadmConfig of a given machine. func (c *ControlPlane) GetKubeadmConfig(machineName string) (*bootstrapv1.KubeadmConfig, bool) { kubeadmConfig, ok := c.KubeadmConfigs[machineName] diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go index a77071841c34..0be579851278 100644 --- a/controlplane/kubeadm/internal/controllers/controller.go +++ b/controlplane/kubeadm/internal/controllers/controller.go @@ -395,6 +395,10 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, controlPl return ctrl.Result{}, err } + if result, err := r.reconcilePreTerminateHook(ctx, controlPlane); err != nil || !result.IsZero() { + return result, err + } + // Reconcile unhealthy machines by triggering deletion and requeue if it is considered safe to remediate, // otherwise continue with the other KCP operations. if result, err := r.reconcileUnhealthyMachines(ctx, controlPlane); err != nil || !result.IsZero() { @@ -768,6 +772,61 @@ func (r *KubeadmControlPlaneReconciler) reconcileEtcdMembers(ctx context.Context return nil } +func (r *KubeadmControlPlaneReconciler) reconcilePreTerminateHook(ctx context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error) { + if !controlPlane.HasDeletingMachine() { + return ctrl.Result{}, nil + } + + log := ctrl.LoggerFrom(ctx) + + removedPreTerminateHook := false + for _, deletingMachine := range controlPlane.DeletingMachines() { + log := log.WithValues("Machine", klog.KObj(deletingMachine)) + ctx := ctrl.LoggerInto(ctx, log) + + c := conditions.Get(deletingMachine, clusterv1.PreTerminateDeleteHookSucceededCondition) + if c != nil && c.Status == corev1.ConditionFalse && c.Reason == clusterv1.WaitingExternalHookReason { + if _, exists := deletingMachine.Annotations[controlplanev1.PreTerminateDeleteHookAnnotation]; !exists { + continue + } + + // Forward etcd leader and remove member only if KCP is not in deletion. + if controlPlane.KCP.DeletionTimestamp.IsZero() { + workloadCluster, err := controlPlane.GetWorkloadCluster(ctx) + if err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to remove etcd member for deleting Machine %s: failed to create client to workload cluster", klog.KObj(deletingMachine)) + } + + // Note: In regular deletion cases (remediation, scale down) the leader should have been already moved. + // We're doing this again here in case the Machine became leader again or the Machine deletion was + // triggered in another way (e.g. a user running kubectl delete machine) + etcdLeaderCandidate := controlPlane.Machines.Newest() + if err := workloadCluster.ForwardEtcdLeadership(ctx, deletingMachine, etcdLeaderCandidate); err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to move leadership to candidate Machine %s", etcdLeaderCandidate.Name) + } + if err := workloadCluster.RemoveEtcdMemberForMachine(ctx, deletingMachine); err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to remove etcd member for deleting Machine %s", klog.KObj(deletingMachine)) + } + } + + log.Info("Removing pre-terminate hook from control plane machine") + deletingMachineOriginal := deletingMachine.DeepCopy() + delete(deletingMachine.Annotations, controlplanev1.PreTerminateDeleteHookAnnotation) + if err := r.Client.Patch(ctx, deletingMachine, client.MergeFrom(deletingMachineOriginal)); err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to remove pre-terminate hook from control plane machine %s", klog.KObj(deletingMachine)) + } + removedPreTerminateHook = true + } + } + + if removedPreTerminateHook { + log.Info("Waiting for Machines to be deleted", "machines", strings.Join(controlPlane.Machines.Filter(collections.HasDeletionTimestamp).Names(), ", ")) + return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil + } + + return ctrl.Result{}, nil +} + func (r *KubeadmControlPlaneReconciler) reconcileCertificateExpiries(ctx context.Context, controlPlane *internal.ControlPlane) error { log := ctrl.LoggerFrom(ctx) diff --git a/controlplane/kubeadm/internal/controllers/controller_test.go b/controlplane/kubeadm/internal/controllers/controller_test.go index da71a74ff1b3..667233f00afc 100644 --- a/controlplane/kubeadm/internal/controllers/controller_test.go +++ b/controlplane/kubeadm/internal/controllers/controller_test.go @@ -1748,7 +1748,13 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) { // Verify Labels g.Expect(updatedInplaceMutatingMachine.Labels).Should(Equal(expectedLabels)) // Verify Annotations - g.Expect(updatedInplaceMutatingMachine.Annotations).Should(Equal(kcp.Spec.MachineTemplate.ObjectMeta.Annotations)) + expectedAnnotations := map[string]string{} + for k, v := range kcp.Spec.MachineTemplate.ObjectMeta.Annotations { + expectedAnnotations[k] = v + } + // The pre-terminate annotation should always be added + expectedAnnotations[controlplanev1.PreTerminateDeleteHookAnnotation] = "" + g.Expect(updatedInplaceMutatingMachine.Annotations).Should(Equal(expectedAnnotations)) // Verify Node timeout values g.Expect(updatedInplaceMutatingMachine.Spec.NodeDrainTimeout).Should(And( Not(BeNil()), diff --git a/controlplane/kubeadm/internal/controllers/helpers.go b/controlplane/kubeadm/internal/controllers/helpers.go index f7e3a93f758d..5f1e3617faba 100644 --- a/controlplane/kubeadm/internal/controllers/helpers.go +++ b/controlplane/kubeadm/internal/controllers/helpers.go @@ -386,6 +386,9 @@ func (r *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev annotations[controlplanev1.RemediationForAnnotation] = remediationData } } + // Setting pre-terminate hook so we can later remove the etcd member right before Machine termination + // (i.e. before InfraMachine deletion). + annotations[controlplanev1.PreTerminateDeleteHookAnnotation] = "" // Construct the basic Machine. desiredMachine := &clusterv1.Machine{ diff --git a/controlplane/kubeadm/internal/controllers/helpers_test.go b/controlplane/kubeadm/internal/controllers/helpers_test.go index eb5d8ffb6539..28300075072d 100644 --- a/controlplane/kubeadm/internal/controllers/helpers_test.go +++ b/controlplane/kubeadm/internal/controllers/helpers_test.go @@ -562,6 +562,8 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) { expectedAnnotations[k] = v } expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = clusterConfigurationString + // The pre-terminate annotation should always be added + expectedAnnotations[controlplanev1.PreTerminateDeleteHookAnnotation] = "" g.Expect(createdMachine.Annotations).To(Equal(expectedAnnotations)) // Verify that machineTemplate.ObjectMeta in KCP has not been modified. @@ -646,6 +648,8 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) { } expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = existingClusterConfigurationString expectedAnnotations[controlplanev1.RemediationForAnnotation] = remediationData + // The pre-terminate annotation should always be added + expectedAnnotations[controlplanev1.PreTerminateDeleteHookAnnotation] = "" g.Expect(updatedMachine.Annotations).To(Equal(expectedAnnotations)) // Verify that machineTemplate.ObjectMeta in KCP has not been modified. diff --git a/controlplane/kubeadm/internal/controllers/remediation.go b/controlplane/kubeadm/internal/controllers/remediation.go index 52404d973651..08c246724b39 100644 --- a/controlplane/kubeadm/internal/controllers/remediation.go +++ b/controlplane/kubeadm/internal/controllers/remediation.go @@ -224,11 +224,6 @@ func (r *KubeadmControlPlaneReconciler) reconcileUnhealthyMachines(ctx context.C conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.RemediationFailedReason, clusterv1.ConditionSeverityError, err.Error()) return ctrl.Result{}, err } - if err := workloadCluster.RemoveEtcdMemberForMachine(ctx, machineToBeRemediated); err != nil { - log.Error(err, "Failed to remove etcd member for machine") - conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.RemediationFailedReason, clusterv1.ConditionSeverityError, err.Error()) - return ctrl.Result{}, err - } } parsedVersion, err := semver.ParseTolerant(controlPlane.KCP.Spec.Version) diff --git a/controlplane/kubeadm/internal/controllers/scale.go b/controlplane/kubeadm/internal/controllers/scale.go index 43bcb37d87cc..5e5370f08150 100644 --- a/controlplane/kubeadm/internal/controllers/scale.go +++ b/controlplane/kubeadm/internal/controllers/scale.go @@ -135,10 +135,6 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane( logger.Error(err, "Failed to move leadership to candidate machine", "candidate", etcdLeaderCandidate.Name) return ctrl.Result{}, err } - if err := workloadCluster.RemoveEtcdMemberForMachine(ctx, machineToDelete); err != nil { - logger.Error(err, "Failed to remove etcd member for machine") - return ctrl.Result{}, err - } } parsedVersion, err := semver.ParseTolerant(controlPlane.KCP.Spec.Version)