From b34a4570fa714a1c8cd2e2fbdf8dfa04ec311629 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Thu, 12 Nov 2020 17:47:07 +0100 Subject: [PATCH] KCP remediates unhealthy machines --- api/v1alpha3/condition_consts.go | 6 + controllers/machine_controller.go | 7 +- controllers/machinehealthcheck_controller.go | 6 +- .../kubeadm/controllers/controller.go | 8 +- .../kubeadm/controllers/fakes_test.go | 15 +- .../kubeadm/controllers/remediation.go | 255 +++++++ .../kubeadm/controllers/remediation_test.go | 620 ++++++++++++++++++ .../kubeadm/controllers/suite_test.go | 31 +- .../kubeadm/internal/control_plane.go | 15 + .../kubeadm/internal/control_plane_test.go | 28 + .../machinefilters/machine_filters.go | 10 + .../machinefilters/machine_filters_test.go | 28 + .../kubeadm/internal/workload_cluster.go | 1 + .../kubeadm/internal/workload_cluster_etcd.go | 33 + 14 files changed, 1043 insertions(+), 20 deletions(-) create mode 100644 controlplane/kubeadm/controllers/remediation.go create mode 100644 controlplane/kubeadm/controllers/remediation_test.go diff --git a/api/v1alpha3/condition_consts.go b/api/v1alpha3/condition_consts.go index d6caf969c2a7..7d0314425dae 100644 --- a/api/v1alpha3/condition_consts.go +++ b/api/v1alpha3/condition_consts.go @@ -131,6 +131,12 @@ const ( // WaitingForRemediationReason is the reason used when a machine fails a health check and remediation is needed. WaitingForRemediationReason = "WaitingForRemediation" + // RemediationFailedReason is the reason used when a remediation owner fails to remediate an unhealthy machine. + RemediationFailedReason = "RemediationFailed" + + // RemediationInProgressReason is the reason used when an unhealthy machine is being remediated by the remediation owner. + RemediationInProgressReason = "RemediationInProgress" + // ExternalRemediationTemplateAvailable is set on machinehealthchecks when MachineHealthCheck controller uses external remediation. // ExternalRemediationTemplateAvailable is set to false if external remediation template is not found. ExternalRemediationTemplateAvailable ConditionType = "ExternalRemediationTemplateAvailable" diff --git a/controllers/machine_controller.go b/controllers/machine_controller.go index 6d7bf3ad918c..0ef40a76a5f0 100644 --- a/controllers/machine_controller.go +++ b/controllers/machine_controller.go @@ -219,10 +219,13 @@ func patchMachine(ctx context.Context, patchHelper *patch.Helper, machine *clust // after provisioning - e.g. when a MHC condition exists - or during the deletion process). conditions.SetSummary(machine, conditions.WithConditions( + // Infrastructure problems should take precedence over all the other conditions clusterv1.InfrastructureReadyCondition, + // Boostrap comes after, but it is relevant only during initial machine provisioning. clusterv1.BootstrapReadyCondition, - clusterv1.MachineOwnerRemediatedCondition, + // MHC reported condition should take precedence over the remediation progress clusterv1.MachineHealthCheckSuccededCondition, + clusterv1.MachineOwnerRemediatedCondition, ), conditions.WithStepCounterIf(machine.ObjectMeta.DeletionTimestamp.IsZero()), conditions.WithStepCounterIfOnly( @@ -240,8 +243,8 @@ func patchMachine(ctx context.Context, patchHelper *patch.Helper, machine *clust clusterv1.BootstrapReadyCondition, clusterv1.InfrastructureReadyCondition, clusterv1.DrainingSucceededCondition, - clusterv1.MachineOwnerRemediatedCondition, clusterv1.MachineHealthCheckSuccededCondition, + clusterv1.MachineOwnerRemediatedCondition, }}, ) diff --git a/controllers/machinehealthcheck_controller.go b/controllers/machinehealthcheck_controller.go index 0150f92c9704..0274624b50e2 100644 --- a/controllers/machinehealthcheck_controller.go +++ b/controllers/machinehealthcheck_controller.go @@ -382,7 +382,11 @@ func (r *MachineHealthCheckReconciler) PatchUnhealthyTargets(ctx context.Context } } else { r.Log.Info("Target has failed health check, marking for remediation", "target", t.string(), "reason", condition.Reason, "message", condition.Message) - conditions.MarkFalse(t.Machine, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "MachineHealthCheck failed") + // NOTE: MHC is responsible for creating MachineOwnerRemediatedCondition if missing or to trigger another remediation if the previous one is completed; + // instead, if a remediation is in already progress, the remediation owner is responsible for completing the process and MHC should not overwrite the condition. + if !conditions.Has(t.Machine, clusterv1.MachineOwnerRemediatedCondition) || conditions.IsTrue(t.Machine, clusterv1.MachineOwnerRemediatedCondition) { + conditions.MarkFalse(t.Machine, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "") + } } } diff --git a/controlplane/kubeadm/controllers/controller.go b/controlplane/kubeadm/controllers/controller.go index 7f5356f6bf44..4f01773e3cc2 100644 --- a/controlplane/kubeadm/controllers/controller.go +++ b/controlplane/kubeadm/controllers/controller.go @@ -302,7 +302,7 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * // Aggregate the operational state of all the machines; while aggregating we are adding the // source ref (reason@machine/name) so the problem can be easily tracked down to its source machine. - conditions.SetAggregate(controlPlane.KCP, controlplanev1.MachinesReadyCondition, controlPlane.Machines.ConditionGetters(), conditions.AddSourceRef()) + conditions.SetAggregate(controlPlane.KCP, controlplanev1.MachinesReadyCondition, ownedMachines.ConditionGetters(), conditions.AddSourceRef(), conditions.WithStepCounterIf(false)) // Updates conditions reporting the status of static pods and the status of the etcd cluster. // NOTE: Conditions reporting KCP operation progress like e.g. Resized or SpecUpToDate are inlined with the rest of the execution. @@ -316,6 +316,12 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * 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() { + return result, err + } + // Control plane machines rollout due to configuration changes (e.g. upgrades) takes precedence over other operations. needRollout := controlPlane.MachinesNeedingRollout() switch { diff --git a/controlplane/kubeadm/controllers/fakes_test.go b/controlplane/kubeadm/controllers/fakes_test.go index c370894c6bc5..45b5cc3fb204 100644 --- a/controlplane/kubeadm/controllers/fakes_test.go +++ b/controlplane/kubeadm/controllers/fakes_test.go @@ -56,7 +56,8 @@ func (f *fakeManagementCluster) GetMachinesForCluster(c context.Context, n clien type fakeWorkloadCluster struct { *internal.Workload - Status internal.ClusterStatus + Status internal.ClusterStatus + EtcdMembersResult []string } func (f fakeWorkloadCluster) ForwardEtcdLeadership(_ context.Context, _ *clusterv1.Machine, _ *clusterv1.Machine) error { @@ -95,6 +96,18 @@ func (f fakeWorkloadCluster) UpdateKubeletConfigMap(ctx context.Context, version return nil } +func (f fakeWorkloadCluster) RemoveEtcdMemberForMachine(ctx context.Context, machine *clusterv1.Machine) error { + return nil +} + +func (f fakeWorkloadCluster) RemoveMachineFromKubeadmConfigMap(ctx context.Context, machine *clusterv1.Machine) error { + return nil +} + +func (f fakeWorkloadCluster) EtcdMembers(_ context.Context) ([]string, error) { + return f.EtcdMembersResult, nil +} + type fakeMigrator struct { migrateCalled bool migrateErr error diff --git a/controlplane/kubeadm/controllers/remediation.go b/controlplane/kubeadm/controllers/remediation.go new file mode 100644 index 000000000000..8bed556972af --- /dev/null +++ b/controlplane/kubeadm/controllers/remediation.go @@ -0,0 +1,255 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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 controllers + +import ( + "context" + "fmt" + + "github.com/pkg/errors" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" + "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" + "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/cluster-api/util/patch" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// reconcileUnhealthyMachines tries to remediate KubeadmControlPlane unhealthy machines +// based on the process described in https://github.com/kubernetes-sigs/cluster-api/blob/master/docs/proposals/20191017-kubeadm-based-control-plane.md#remediation-using-delete-and-recreate +func (r *KubeadmControlPlaneReconciler) reconcileUnhealthyMachines(ctx context.Context, controlPlane *internal.ControlPlane) (ret ctrl.Result, retErr error) { + logger := r.Log.WithValues("namespace", controlPlane.KCP.Namespace, "kubeadmControlPlane", controlPlane.KCP.Name, "cluster", controlPlane.Cluster.Name) + + // Gets all machines that have `MachineHealthCheckSucceeded=False` (indicating a problem was detected on the machine) + // and `MachineOwnerRemediated` present, indicating that this controller is responsible for performing remediation. + unhealthyMachines := controlPlane.UnhealthyMachines() + + // If there are no unhealthy machines, return so KCP can proceed with other operations (ctrl.Result nil). + if len(unhealthyMachines) == 0 { + return ctrl.Result{}, nil + } + + // Select the machine to be remediated, which is the oldest machine marked as unhealthy. + // + // NOTE: The current solution is considered acceptable for the most frequent use case (only one unhealthy machine), + // however, in the future this could potentially be improved for the scenario where more than one unhealthy machine exists + // by considering which machine has lower impact on etcd quorum. + machineToBeRemediated := unhealthyMachines.Oldest() + + // Returns if the machine is in the process of being deleted. + if !machineToBeRemediated.ObjectMeta.DeletionTimestamp.IsZero() { + return ctrl.Result{}, nil + } + + patchHelper, err := patch.NewHelper(machineToBeRemediated, r.Client) + if err != nil { + return ctrl.Result{}, err + } + + defer func() { + // Always attempt to Patch the Machine conditions after each reconcileUnhealthyMachines. + if err := patchHelper.Patch(ctx, machineToBeRemediated, patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ + clusterv1.MachineOwnerRemediatedCondition, + }}); err != nil { + logger.Error(err, "Failed to patch control plane Machine", "machine", machineToBeRemediated.Name) + if retErr == nil { + retErr = errors.Wrapf(err, "failed to patch control plane Machine %s", machineToBeRemediated.Name) + } + } + }() + + // Before starting remediation, run preflight checks in order to verify it is safe to remediate. + // If any of the following checks fails, we'll surface the reason in the MachineOwnerRemediated condition. + + desiredReplicas := int(*controlPlane.KCP.Spec.Replicas) + + // The cluster MUST have spec.replicas >= 3, because this is the smallest cluster size that allows any etcd failure tolerance. + if desiredReplicas < 3 { + logger.Info("A control plane machine needs remediation, but the number of desired replicas is less than 3. Skipping remediation", "UnhealthyMachine", machineToBeRemediated.Name, "Replicas", desiredReplicas) + conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP can't remediate if there are less than 3 desired replicas") + return ctrl.Result{}, nil + } + + // The number of replicas MUST be equal to or greater than the desired replicas. This rule ensures that when the cluster + // is missing replicas, we skip remediation and instead perform regular scale up/rollout operations first. + if controlPlane.Machines.Len() < desiredReplicas { + logger.Info("A control plane machine needs remediation, but the current number of replicas is lower that expected. Skipping remediation", "UnhealthyMachine", machineToBeRemediated.Name, "Replicas", desiredReplicas, "CurrentReplicas", controlPlane.Machines.Len()) + conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP waiting for having at least %d control plane machines before triggering remediation", desiredReplicas) + return ctrl.Result{}, nil + } + + // The cluster MUST have no machines with a deletion timestamp. This rule prevents KCP taking actions while the cluster is in a transitional state. + if controlPlane.HasDeletingMachine() { + logger.Info("A control plane machine needs remediation, but there are other control-plane machines being deleted. Skipping remediation", "UnhealthyMachine", machineToBeRemediated.Name) + conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP waiting for control plane machine deletion to complete before triggering remediation") + return ctrl.Result{}, nil + } + + // Remediation MUST preserve etcd quorum. This rule ensures that we will not remove a member that would result in etcd + // losing a majority of members and thus become unable to field new requests. + if controlPlane.IsEtcdManaged() { + canSafelyRemediate, err := r.canSafelyRemoveEtcdMember(ctx, controlPlane, machineToBeRemediated) + if err != nil { + conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.RemediationFailedReason, clusterv1.ConditionSeverityError, err.Error()) + return ctrl.Result{}, err + } + if !canSafelyRemediate { + logger.Info("A control plane machine needs remediation, but removing this machine could result in etcd quorum loss. Skipping remediation", "UnhealthyMachine", machineToBeRemediated.Name) + conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP can't remediate this machine because this could result in etcd loosing quorum") + return ctrl.Result{}, nil + } + } + + workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster)) + if err != nil { + logger.Error(err, "Failed to create client to workload cluster") + return ctrl.Result{}, errors.Wrapf(err, "failed to create client to workload cluster") + } + + // If the machine that is about to be deleted is the etcd leader, move it to the newest member available. + if controlPlane.IsEtcdManaged() { + etcdLeaderCandidate := controlPlane.HealthyMachines().Newest() + if err := workloadCluster.ForwardEtcdLeadership(ctx, machineToBeRemediated, etcdLeaderCandidate); err != nil { + logger.Error(err, "Failed to move leadership to candidate machine", "candidate", etcdLeaderCandidate.Name) + return ctrl.Result{}, err + } + if err := workloadCluster.RemoveEtcdMemberForMachine(ctx, machineToBeRemediated); err != nil { + logger.Error(err, "Failed to remove etcd member for machine") + return ctrl.Result{}, err + } + } + + if err := workloadCluster.RemoveMachineFromKubeadmConfigMap(ctx, machineToBeRemediated); err != nil { + logger.Error(err, "Failed to remove machine from kubeadm ConfigMap") + return ctrl.Result{}, err + } + + if err := r.Client.Delete(ctx, machineToBeRemediated); err != nil { + conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.RemediationFailedReason, clusterv1.ConditionSeverityError, err.Error()) + return ctrl.Result{}, errors.Wrapf(err, "failed to delete unhealthy machine %s", machineToBeRemediated.Name) + } + + logger.Info("Remediating unhealthy machine", "UnhealthyMachine", machineToBeRemediated.Name) + conditions.MarkFalse(machineToBeRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.RemediationInProgressReason, clusterv1.ConditionSeverityWarning, "") + return ctrl.Result{Requeue: true}, nil +} + +// canSafelyRemoveEtcdMember assess if it is possible to remove the member hosted on the machine to be remediated +// without loosing etcd quorum. +// +// The answer mostly depend on the existence of other failing members on top of the one being deleted, and according +// to the etcd fault tolerance specification (see https://github.com/etcd-io/etcd/blob/master/Documentation/faq.md#what-is-failure-tolerance): +// - 3 CP cluster does not tolerate additional failing members on top of the one being deleted (the target +// cluster size after deletion is 2, fault tolerance 0) +// - 5 CP cluster tolerates 1 additional failing members on top of the one being deleted (the target +// cluster size after deletion is 4, fault tolerance 1) +// - 7 CP cluster tolerates 2 additional failing members on top of the one being deleted (the target +// cluster size after deletion is 6, fault tolerance 2) +// - etc. +// +// NOTE: this func assumes the list of members in sync with the list of machines/nodes, it is required to call reconcileEtcdMembers +// ans well as reconcileControlPlaneConditions before this. +func (r *KubeadmControlPlaneReconciler) canSafelyRemoveEtcdMember(ctx context.Context, controlPlane *internal.ControlPlane, machineToBeRemediated *clusterv1.Machine) (bool, error) { + logger := r.Log.WithValues("namespace", controlPlane.KCP.Namespace, "kubeadmControlPlane", controlPlane.KCP.Name, "cluster", controlPlane.Cluster.Name) + + workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, client.ObjectKey{ + Namespace: controlPlane.Cluster.Namespace, + Name: controlPlane.Cluster.Name, + }) + if err != nil { + return false, errors.Wrapf(err, "failed to get client for workload cluster %s", controlPlane.Cluster.Name) + } + + // Gets the etcd status + + // This makes it possible to have a set of etcd members status different from the MHC unhealthy/unhealthy conditions. + etcdMembers, err := workloadCluster.EtcdMembers(ctx) + if err != nil { + return false, errors.Wrapf(err, "failed to get etcdStatus for workload cluster %s", controlPlane.Cluster.Name) + } + + currentTotalMembers := len(etcdMembers) + + logger.Info("etcd cluster before remediation", + "currentTotalMembers", currentTotalMembers, + "currentMembers", etcdMembers) + + // The cluster MUST have at least 3 members, because this is the smallest cluster size that allows any etcd failure tolerance. + // + // NOTE: This should not happen given that we are checking the number of replicas before calling this method, however + // given that this could be destructive, this is an additional safeguard. + if currentTotalMembers < 3 { + logger.Info("etcd cluster with less of 3 members can't be safely remediated") + return false, nil + } + + targetTotalMembers := currentTotalMembers - 1 + targetQuorum := targetTotalMembers/2.0 + 1 + targetUnhealthyMembers := 0 + + healthyMembers := []string{} + unhealthyMembers := []string{} + for _, etcdMember := range etcdMembers { + // Skip the machine to be deleted because it won't be part of the target etcd cluster. + if etcdMember == machineToBeRemediated.Name { + continue + } + + // Search for the machine corresponding to the etcd member. + var machine *clusterv1.Machine + for _, m := range controlPlane.Machines { + if m.Status.NodeRef != nil && m.Status.NodeRef.Name == etcdMember { + machine = m + break + } + } + + // If an etcd member does not have a corresponding machine, it is not possible to retrieve etcd member health + // so we are assuming the worst scenario and considering the member unhealthy. + // + // NOTE: This should not happen given that we are running reconcileEtcdMembers before calling this method. + if machine == nil { + logger.Info("An etcd member does not have a corresponding machine, assuming this member is unhealthy", "MemberName", etcdMember) + targetUnhealthyMembers++ + unhealthyMembers = append(unhealthyMembers, fmt.Sprintf("%s (no machine)", etcdMember)) + continue + } + + // Check member health as reported by machine's health conditions + if !conditions.IsTrue(machine, controlplanev1.MachineEtcdMemberHealthyCondition) { + targetUnhealthyMembers++ + unhealthyMembers = append(unhealthyMembers, fmt.Sprintf("%s (%s)", etcdMember, machine.Name)) + continue + } + + healthyMembers = append(healthyMembers, fmt.Sprintf("%s (%s)", etcdMember, machine.Name)) + } + + logger.Info(fmt.Sprintf("etcd cluster projected after remediation of %s", machineToBeRemediated.Name), + "healthyMembers", healthyMembers, + "unhealthyMembers", unhealthyMembers, + "targetTotalMembers", targetTotalMembers, + "targetQuorum", targetQuorum, + "targetUnhealthyMembers", targetUnhealthyMembers, + "projectedQuorum", targetTotalMembers-targetUnhealthyMembers) + if targetTotalMembers-targetUnhealthyMembers >= targetQuorum { + return true, nil + } + return false, nil +} diff --git a/controlplane/kubeadm/controllers/remediation_test.go b/controlplane/kubeadm/controllers/remediation_test.go new file mode 100644 index 000000000000..84a9d9dfaeee --- /dev/null +++ b/controlplane/kubeadm/controllers/remediation_test.go @@ -0,0 +1,620 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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 controllers + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + utilpointer "k8s.io/utils/pointer" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" + "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" + "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +func TestReconcileUnhealthyMachines(t *testing.T) { + g := NewWithT(t) + ctx := context.TODO() + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + } + ns, err := testEnv.CreateNamespace(ctx, "ns1") + g.Expect(err).ToNot(HaveOccurred()) + + t.Run("Remediation does not happen if there are no unhealthy machines", func(t *testing.T) { + g := NewWithT(t) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(), + } + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeTrue()) // Remediation skipped + g.Expect(err).ToNot(HaveOccurred()) + }) + t.Run("reconcileUnhealthyMachines return early if the machine to be remediated is marked for deletion", func(t *testing.T) { + g := NewWithT(t) + + m := getDeletingMachine(ns.Name, "m1-unhealthy-deleting-", withMachineHealthCheckFailed()) + conditions.MarkFalse(m, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") + conditions.MarkFalse(m, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "") + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m), + } + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeTrue()) // Remediation skipped + g.Expect(err).ToNot(HaveOccurred()) + }) + t.Run("Remediation does not happen if desired replicas < 3", func(t *testing.T) { + g := NewWithT(t) + + m := createMachine(ctx, g, ns.Name, "m1-unhealthy-", withMachineHealthCheckFailed()) + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(1), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m), + } + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeTrue()) // Remediation skipped + g.Expect(err).ToNot(HaveOccurred()) + assertMachineCondition(ctx, g, m, clusterv1.MachineOwnerRemediatedCondition, corev1.ConditionFalse, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP can't remediate if there are less than 3 desired replicas") + + g.Expect(testEnv.Cleanup(ctx, m)).To(Succeed()) + }) + t.Run("Remediation does not happen if number of machines lower than desired", func(t *testing.T) { + g := NewWithT(t) + + m := createMachine(ctx, g, ns.Name, "m1-unhealthy-", withMachineHealthCheckFailed()) + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(3), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m), + } + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeTrue()) // Remediation skipped + g.Expect(err).ToNot(HaveOccurred()) + assertMachineCondition(ctx, g, m, clusterv1.MachineOwnerRemediatedCondition, corev1.ConditionFalse, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP waiting for having at least 3 control plane machines before triggering remediation") + + g.Expect(testEnv.Cleanup(ctx, m)).To(Succeed()) + }) + t.Run("Remediation does not happen if there is a deleting machine", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-healthy-") + m3 := getDeletingMachine(ns.Name, "m3-deleting") // NB. This machine is not created, it gets only added to control plane + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(3), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + } + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeTrue()) // Remediation skipped + g.Expect(err).ToNot(HaveOccurred()) + assertMachineCondition(ctx, g, m1, clusterv1.MachineOwnerRemediatedCondition, corev1.ConditionFalse, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP waiting for control plane machine deletion to complete before triggering remediation") + + g.Expect(testEnv.Cleanup(ctx, m1, m2)).To(Succeed()) + }) + t.Run("Remediation does not happen if there is at least one additional unhealthy etcd member on a 3 machine CP", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-unhealthy-", withUnhealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(3), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeTrue()) // Remediation skipped + g.Expect(err).ToNot(HaveOccurred()) + assertMachineCondition(ctx, g, m1, clusterv1.MachineOwnerRemediatedCondition, corev1.ConditionFalse, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP can't remediate this machine because this could result in etcd loosing quorum") + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3)).To(Succeed()) + }) + t.Run("Remediation does not happen if there is at least two additional unhealthy etcd member on a 5 machine CP", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-unhealthy-", withUnhealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-unhealthy-", withUnhealthyEtcdMember()) + m4 := createMachine(ctx, g, ns.Name, "m4-etcd-healthy-", withHealthyEtcdMember()) + m5 := createMachine(ctx, g, ns.Name, "m5-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(5), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeTrue()) // Remediation skipped + g.Expect(err).ToNot(HaveOccurred()) + assertMachineCondition(ctx, g, m1, clusterv1.MachineOwnerRemediatedCondition, corev1.ConditionFalse, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "KCP can't remediate this machine because this could result in etcd loosing quorum") + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3, m4, m5)).To(Succeed()) + }) + t.Run("Remediation deletes unhealthy machine", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-unhealthy-", withMachineHealthCheckFailed()) + patchHelper, err := patch.NewHelper(m1, testEnv.GetClient()) + g.Expect(err).ToNot(HaveOccurred()) + m1.ObjectMeta.Finalizers = []string{"wait-before-delete"} + g.Expect(patchHelper.Patch(ctx, m1)) + + m2 := createMachine(ctx, g, ns.Name, "m2-healthy-", withHealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(3), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) + + g.Expect(ret.IsZero()).To(BeFalse()) // Remediation completed, requeue + g.Expect(err).ToNot(HaveOccurred()) + + assertMachineCondition(ctx, g, m1, clusterv1.MachineOwnerRemediatedCondition, corev1.ConditionFalse, clusterv1.RemediationInProgressReason, clusterv1.ConditionSeverityWarning, "") + + err = testEnv.Get(ctx, client.ObjectKey{Namespace: m1.Namespace, Name: m1.Name}, m1) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(m1.ObjectMeta.DeletionTimestamp.IsZero()).To(BeFalse()) + + patchHelper, err = patch.NewHelper(m1, testEnv.GetClient()) + g.Expect(err).ToNot(HaveOccurred()) + m1.ObjectMeta.Finalizers = nil + g.Expect(patchHelper.Patch(ctx, m1)) + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3)).To(Succeed()) + }) + + g.Expect(testEnv.Cleanup(ctx, ns)).To(Succeed()) +} + +func TestCanSafelyRemoveEtcdMember(t *testing.T) { + g := NewWithT(t) + ctx := context.TODO() + + ns, err := testEnv.CreateNamespace(ctx, "ns1") + g.Expect(err).ToNot(HaveOccurred()) + + t.Run("Can't safely remediate 1 machine CP", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(1), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeFalse()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1)).To(Succeed()) + }) + t.Run("Can't safely remediate 2 machine CP", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(2), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeFalse()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1, m2)).To(Succeed()) + }) + t.Run("Can safely remediate 3 machines CP without additional etcd member failures", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-healthy-", withHealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(5), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeTrue()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3)).To(Succeed()) + }) + t.Run("Can't safely remediate 3 machines CP with one additional etcd member failure", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-unhealthy-", withUnhealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(5), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeFalse()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3)).To(Succeed()) + }) + t.Run("Can safely remediate 5 machines CP less than 2 additional etcd member failures", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-unhealthy-", withUnhealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-healthy-", withHealthyEtcdMember()) + m4 := createMachine(ctx, g, ns.Name, "m4-etcd-healthy-", withHealthyEtcdMember()) + m5 := createMachine(ctx, g, ns.Name, "m5-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(5), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeTrue()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3, m4, m5)).To(Succeed()) + }) + t.Run("Can't safely remediate 5 machines CP with 2 additional etcd member failures", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-unhealthy-", withUnhealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-unhealthy-", withUnhealthyEtcdMember()) + m4 := createMachine(ctx, g, ns.Name, "m4-etcd-healthy-", withHealthyEtcdMember()) + m5 := createMachine(ctx, g, ns.Name, "m5-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(5), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeFalse()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3, m4, m5)).To(Succeed()) + }) + t.Run("Can safely remediate 7 machines CP with less than 3 additional etcd member failures", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-unhealthy-", withUnhealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-unhealthy-", withUnhealthyEtcdMember()) + m4 := createMachine(ctx, g, ns.Name, "m4-etcd-healthy-", withHealthyEtcdMember()) + m5 := createMachine(ctx, g, ns.Name, "m5-etcd-healthy-", withHealthyEtcdMember()) + m6 := createMachine(ctx, g, ns.Name, "m6-etcd-healthy-", withHealthyEtcdMember()) + m7 := createMachine(ctx, g, ns.Name, "m7-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(5), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5, m6, m7), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeTrue()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3, m4, m5, m6, m7)).To(Succeed()) + }) + t.Run("Can't safely remediate 7 machines CP with 3 additional etcd member failures", func(t *testing.T) { + g := NewWithT(t) + + m1 := createMachine(ctx, g, ns.Name, "m1-mhc-unhealthy-", withMachineHealthCheckFailed()) + m2 := createMachine(ctx, g, ns.Name, "m2-etcd-unhealthy-", withUnhealthyEtcdMember()) + m3 := createMachine(ctx, g, ns.Name, "m3-etcd-unhealthy-", withUnhealthyEtcdMember()) + m4 := createMachine(ctx, g, ns.Name, "m4-etcd-unhealthy-", withUnhealthyEtcdMember()) + m5 := createMachine(ctx, g, ns.Name, "m5-etcd-healthy-", withHealthyEtcdMember()) + m6 := createMachine(ctx, g, ns.Name, "m6-etcd-healthy-", withHealthyEtcdMember()) + m7 := createMachine(ctx, g, ns.Name, "m7-etcd-healthy-", withHealthyEtcdMember()) + + controlPlane := &internal.ControlPlane{ + KCP: &controlplanev1.KubeadmControlPlane{Spec: controlplanev1.KubeadmControlPlaneSpec{ + Replicas: utilpointer.Int32Ptr(5), + }}, + Cluster: &clusterv1.Cluster{}, + Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5, m6, m7), + } + + r := &KubeadmControlPlaneReconciler{ + Client: testEnv.GetClient(), + Log: log.Log, + recorder: record.NewFakeRecorder(32), + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{ + EtcdMembersResult: controlPlane.Machines.Names(), + }, + }, + } + + ret, err := r.canSafelyRemoveEtcdMember(context.TODO(), controlPlane, m1) + g.Expect(ret).To(BeFalse()) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(testEnv.Cleanup(ctx, m1, m2, m3, m4, m5, m6, m7)).To(Succeed()) + }) + g.Expect(testEnv.Cleanup(ctx, ns)).To(Succeed()) +} + +type machineOption func(*clusterv1.Machine) + +func withMachineHealthCheckFailed() machineOption { + return func(machine *clusterv1.Machine) { + conditions.MarkFalse(machine, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") + conditions.MarkFalse(machine, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "") + } +} + +func withHealthyEtcdMember() machineOption { + return func(machine *clusterv1.Machine) { + conditions.MarkTrue(machine, controlplanev1.MachineEtcdMemberHealthyCondition) + } +} + +func withUnhealthyEtcdMember() machineOption { + return func(machine *clusterv1.Machine) { + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "") + } +} + +func withNodeRef(ref string) machineOption { + return func(machine *clusterv1.Machine) { + machine.Status.NodeRef = &corev1.ObjectReference{ + Kind: "Node", + Name: ref, + } + } +} + +func createMachine(ctx context.Context, g *WithT, namespace, name string, options ...machineOption) *clusterv1.Machine { + m := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + GenerateName: name, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "cluster", + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: utilpointer.StringPtr("secret"), + }, + }, + } + g.Expect(testEnv.Create(ctx, m)).To(Succeed()) + + patchHelper, err := patch.NewHelper(m, testEnv.GetClient()) + g.Expect(err).ToNot(HaveOccurred()) + + for _, opt := range append(options, withNodeRef(m.Name)) { + opt(m) + } + + g.Expect(patchHelper.Patch(ctx, m)) + return m +} + +func getDeletingMachine(namespace, name string, options ...machineOption) *clusterv1.Machine { + deletionTime := metav1.Now() + m := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + DeletionTimestamp: &deletionTime, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "cluster", + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: utilpointer.StringPtr("secret"), + }, + }, + } + + for _, opt := range append(options, withNodeRef(m.Name)) { + opt(m) + } + return m +} + +func assertMachineCondition(ctx context.Context, g *WithT, m *clusterv1.Machine, t clusterv1.ConditionType, status corev1.ConditionStatus, reason string, severity clusterv1.ConditionSeverity, message string) { + g.Expect(testEnv.Get(ctx, client.ObjectKey{Namespace: m.Namespace, Name: m.Name}, m)).To(Succeed()) + machineOwnerRemediatedCondition := conditions.Get(m, t) + g.Expect(machineOwnerRemediatedCondition.Status).To(Equal(status)) + g.Expect(machineOwnerRemediatedCondition.Reason).To(Equal(reason)) + g.Expect(machineOwnerRemediatedCondition.Severity).To(Equal(severity)) + g.Expect(machineOwnerRemediatedCondition.Message).To(Equal(message)) +} diff --git a/controlplane/kubeadm/controllers/suite_test.go b/controlplane/kubeadm/controllers/suite_test.go index f64120b0d38c..8b456addd7cb 100644 --- a/controlplane/kubeadm/controllers/suite_test.go +++ b/controlplane/kubeadm/controllers/suite_test.go @@ -17,6 +17,8 @@ limitations under the License. package controllers import ( + "fmt" + "os" "testing" . "github.com/onsi/ginkgo" @@ -42,22 +44,21 @@ func TestAPIs(t *testing.T) { []Reporter{printer.NewlineReporter{}}) } -var _ = BeforeSuite(func(done Done) { - By("bootstrapping test environment") +func TestMain(m *testing.M) { + // Bootstrapping test environment testEnv = helpers.NewTestEnvironment() - - By("starting the manager") go func() { - defer GinkgoRecover() - Expect(testEnv.StartManager()).To(Succeed()) + if err := testEnv.StartManager(); err != nil { + panic(fmt.Sprintf("Failed to start the envtest manager: %v", err)) + } }() - - close(done) -}, 60) - -var _ = AfterSuite(func() { - if testEnv != nil { - By("tearing down the test environment") - Expect(testEnv.Stop()).To(Succeed()) + // Run tests + code := m.Run() + // Tearing down the test environment + if err := testEnv.Stop(); err != nil { + panic(fmt.Sprintf("Failed to stop the envtest: %v", err)) } -}) + + // Report exit code + os.Exit(code) +} diff --git a/controlplane/kubeadm/internal/control_plane.go b/controlplane/kubeadm/internal/control_plane.go index cb1568b8b120..983af69d8c9f 100644 --- a/controlplane/kubeadm/internal/control_plane.go +++ b/controlplane/kubeadm/internal/control_plane.go @@ -300,6 +300,21 @@ func (c *ControlPlane) IsEtcdManaged() bool { return c.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration == nil || c.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External == nil } +// UnhealthyMachines returns the list of control plane machines marked as unhealthy by MHC. +func (c *ControlPlane) UnhealthyMachines() FilterableMachineCollection { + return c.Machines.Filter(machinefilters.HasUnhealthyCondition) +} + +// HealthyMachines returns the list of control plane machines not marked as unhealthy by MHC. +func (c *ControlPlane) HealthyMachines() FilterableMachineCollection { + return c.Machines.Filter(machinefilters.Not(machinefilters.HasUnhealthyCondition)) +} + +// HasUnhealthyMachine returns true if any machine in the control plane is marked as unhealthy by MHC. +func (c *ControlPlane) HasUnhealthyMachine() bool { + return len(c.UnhealthyMachines()) > 0 +} + func (c *ControlPlane) PatchMachines(ctx context.Context) error { errList := []error{} for i := range c.Machines { diff --git a/controlplane/kubeadm/internal/control_plane_test.go b/controlplane/kubeadm/internal/control_plane_test.go index 04915542a5f5..50d10ccb6869 100644 --- a/controlplane/kubeadm/internal/control_plane_test.go +++ b/controlplane/kubeadm/internal/control_plane_test.go @@ -29,6 +29,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" + "sigs.k8s.io/cluster-api/util/conditions" ) func TestControlPlane(t *testing.T) { @@ -117,3 +118,30 @@ func withFailureDomain(fd string) machineOpt { m.Spec.FailureDomain = &fd } } + +func TestHasUnhealthyMachine(t *testing.T) { + // healthy machine (without MachineHealthCheckSucceded condition) + healthyMachine1 := &clusterv1.Machine{} + // healthy machine (with MachineHealthCheckSucceded == true) + healthyMachine2 := &clusterv1.Machine{} + conditions.MarkTrue(healthyMachine2, clusterv1.MachineHealthCheckSuccededCondition) + // unhealthy machine NOT eligible for KCP remediation (with MachineHealthCheckSucceded == False, but without MachineOwnerRemediated condition) + unhealthyMachineNOTOwnerRemediated := &clusterv1.Machine{} + conditions.MarkFalse(unhealthyMachineNOTOwnerRemediated, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") + // unhealthy machine eligible for KCP remediation (with MachineHealthCheckSucceded == False, with MachineOwnerRemediated condition) + unhealthyMachineOwnerRemediated := &clusterv1.Machine{} + conditions.MarkFalse(unhealthyMachineOwnerRemediated, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") + conditions.MarkFalse(unhealthyMachineOwnerRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "") + + c := ControlPlane{ + Machines: NewFilterableMachineCollection( + healthyMachine1, + healthyMachine2, + unhealthyMachineNOTOwnerRemediated, + unhealthyMachineOwnerRemediated, + ), + } + + g := NewWithT(t) + g.Expect(c.HasUnhealthyMachine()).To(BeTrue()) +} diff --git a/controlplane/kubeadm/internal/machinefilters/machine_filters.go b/controlplane/kubeadm/internal/machinefilters/machine_filters.go index a54fc3e9fda5..588c272e4633 100644 --- a/controlplane/kubeadm/internal/machinefilters/machine_filters.go +++ b/controlplane/kubeadm/internal/machinefilters/machine_filters.go @@ -140,6 +140,16 @@ func HasDeletionTimestamp(machine *clusterv1.Machine) bool { return !machine.DeletionTimestamp.IsZero() } +// HasUnhealthyCondition returns a filter to find all machines that have a MachineHealthCheckSucceeded condition set to False, +// indicating a problem was detected on the machine, and the MachineOwnerRemediated condition set, indicating that KCP is +// responsible of performing remediation as owner of the machine. +func HasUnhealthyCondition(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return conditions.IsFalse(machine, clusterv1.MachineHealthCheckSuccededCondition) && conditions.IsFalse(machine, clusterv1.MachineOwnerRemediatedCondition) +} + // IsReady returns a filter to find all machines with the ReadyCondition equals to True. func IsReady() Func { return func(machine *clusterv1.Machine) bool { diff --git a/controlplane/kubeadm/internal/machinefilters/machine_filters_test.go b/controlplane/kubeadm/internal/machinefilters/machine_filters_test.go index 09a22784bb54..160c4d7c684e 100644 --- a/controlplane/kubeadm/internal/machinefilters/machine_filters_test.go +++ b/controlplane/kubeadm/internal/machinefilters/machine_filters_test.go @@ -21,6 +21,7 @@ import ( "time" . "github.com/onsi/gomega" + "sigs.k8s.io/cluster-api/util/conditions" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -79,6 +80,33 @@ func TestOr(t *testing.T) { }) } +func TestHasUnhealthyCondition(t *testing.T) { + t.Run("healthy machine (without HealthCheckSucceeded condition) should return false", func(t *testing.T) { + g := NewWithT(t) + m := &clusterv1.Machine{} + g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeFalse()) + }) + t.Run("healthy machine (with HealthCheckSucceeded condition == True) should return false", func(t *testing.T) { + g := NewWithT(t) + m := &clusterv1.Machine{} + conditions.MarkTrue(m, clusterv1.MachineHealthCheckSuccededCondition) + g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeFalse()) + }) + t.Run("unhealthy machine NOT eligible for KCP remediation (with withHealthCheckSucceeded condition == False but without OwnerRemediated) should return false", func(t *testing.T) { + g := NewWithT(t) + m := &clusterv1.Machine{} + conditions.MarkFalse(m, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") + g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeFalse()) + }) + t.Run("unhealthy machine eligible for KCP (with HealthCheckSucceeded condition == False and with OwnerRemediated) should return true", func(t *testing.T) { + g := NewWithT(t) + m := &clusterv1.Machine{} + conditions.MarkFalse(m, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") + conditions.MarkFalse(m, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "") + g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeTrue()) + }) +} + func TestHasDeletionTimestamp(t *testing.T) { t.Run("machine with deletion timestamp returns true", func(t *testing.T) { g := NewWithT(t) diff --git a/controlplane/kubeadm/internal/workload_cluster.go b/controlplane/kubeadm/internal/workload_cluster.go index d0ade2b14ec9..0ac8a0644ea4 100644 --- a/controlplane/kubeadm/internal/workload_cluster.go +++ b/controlplane/kubeadm/internal/workload_cluster.go @@ -59,6 +59,7 @@ type WorkloadCluster interface { ClusterStatus(ctx context.Context) (ClusterStatus, error) UpdateStaticPodConditions(ctx context.Context, controlPlane *ControlPlane) UpdateEtcdConditions(ctx context.Context, controlPlane *ControlPlane) + EtcdMembers(ctx context.Context) ([]string, error) // Upgrade related tasks. ReconcileKubeletRBACBinding(ctx context.Context, version semver.Version) error diff --git a/controlplane/kubeadm/internal/workload_cluster_etcd.go b/controlplane/kubeadm/internal/workload_cluster_etcd.go index 93fed7348ef8..be71b360087b 100644 --- a/controlplane/kubeadm/internal/workload_cluster_etcd.go +++ b/controlplane/kubeadm/internal/workload_cluster_etcd.go @@ -196,3 +196,36 @@ func (w *Workload) ForwardEtcdLeadership(ctx context.Context, machine *clusterv1 } return nil } + +type EtcdMemberStatus struct { + Name string + Responsive bool +} + +// EtcdStatus returns the current status of the etcd cluster +// NOTE: This methods uses control plane machines/nodes only to get in contact with etcd, +// but then it relies on etcd as ultimate source of truth for the list of members. +// This is intended to allow informed decisions on actions impacting etcd quorum. +func (w *Workload) EtcdMembers(ctx context.Context) ([]string, error) { + nodes, err := w.getControlPlaneNodes(ctx) + if err != nil { + return nil, errors.Wrap(err, "failed to list control plane nodes") + } + + etcdClient, err := w.etcdClientGenerator.forLeader(ctx, nodes.Items) + if err != nil { + return nil, errors.Wrap(err, "failed to create etcd client") + } + defer etcdClient.Close() + + members, err := etcdClient.Members(ctx) + if err != nil { + return nil, errors.Wrap(err, "failed to list etcd members using etcd client") + } + + names := []string{} + for _, member := range members { + names = append(names, member.Name) + } + return names, nil +}