From e400d471bc95989e3d52c4088b8adee26d3f63aa Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Tue, 10 Nov 2020 23:13:48 +0100 Subject: [PATCH] Add KCP conditions, split reconcileHealth into preflight and reconcileEtcdMembers, make both use conditions --- .../kubeadm/api/v1alpha3/condition_consts.go | 66 ++ controlplane/kubeadm/controllers/consts.go | 6 +- .../kubeadm/controllers/controller.go | 140 +-- .../kubeadm/controllers/controller_test.go | 48 +- .../kubeadm/controllers/fakes_test.go | 23 +- controlplane/kubeadm/controllers/scale.go | 104 ++ .../kubeadm/controllers/scale_test.go | 355 ++++-- .../kubeadm/controllers/upgrade_test.go | 26 +- controlplane/kubeadm/internal/cluster_test.go | 114 -- .../kubeadm/internal/control_plane.go | 52 +- controlplane/kubeadm/internal/etcd/etcd.go | 9 + .../kubeadm/internal/etcd/util/set.go | 201 ---- .../kubeadm/internal/etcd/util/util.go | 16 +- .../kubeadm/internal/workload_cluster.go | 114 +- .../internal/workload_cluster_conditions.go | 553 +++++++++ .../workload_cluster_conditions_test.go | 1042 +++++++++++++++++ .../kubeadm/internal/workload_cluster_etcd.go | 107 +- .../internal/workload_cluster_etcd_test.go | 101 +- .../kubeadm/internal/workload_cluster_test.go | 163 --- 19 files changed, 2240 insertions(+), 1000 deletions(-) delete mode 100644 controlplane/kubeadm/internal/etcd/util/set.go create mode 100644 controlplane/kubeadm/internal/workload_cluster_conditions.go create mode 100644 controlplane/kubeadm/internal/workload_cluster_conditions_test.go diff --git a/controlplane/kubeadm/api/v1alpha3/condition_consts.go b/controlplane/kubeadm/api/v1alpha3/condition_consts.go index bf512965e6cc..a5b3e4a61202 100644 --- a/controlplane/kubeadm/api/v1alpha3/condition_consts.go +++ b/controlplane/kubeadm/api/v1alpha3/condition_consts.go @@ -66,3 +66,69 @@ const ( // ScalingDownReason (Severity=Info) documents a KubeadmControlPlane that is decreasing the number of replicas. ScalingDownReason = "ScalingDown" ) + +const ( + // ControlPlaneComponentsHealthyCondition reports the overall status of control plane components + // implemented as static pods generated by kubeadm including kube-api-server, kube-controller manager, + // kube-scheduler and etcd if managed. + ControlPlaneComponentsHealthyCondition clusterv1.ConditionType = "ControlPlaneComponentsHealthy" + + // ControlPlaneComponentsUnhealthyReason (Severity=Error) documents a control plane component not healthy. + ControlPlaneComponentsUnhealthyReason = "ControlPlaneComponentsUnhealthy" + + // ControlPlaneComponentsUnknownReason reports a control plane component in unknown status. + ControlPlaneComponentsUnknownReason = "ControlPlaneComponentsUnknown" + + // ControlPlaneComponentsInspectionFailedReason documents a failure in inspecting the control plane component status. + ControlPlaneComponentsInspectionFailedReason = "ControlPlaneComponentsInspectionFailed" + + // MachineAPIServerPodHealthyCondition reports a machine's kube-apiserver's operational status. + MachineAPIServerPodHealthyCondition clusterv1.ConditionType = "APIServerPodHealthy" + + // MachineControllerManagerPodHealthyCondition reports a machine's kube-controller-manager's health status. + MachineControllerManagerPodHealthyCondition clusterv1.ConditionType = "ControllerManagerPodHealthy" + + // MachineSchedulerPodHealthyCondition reports a machine's kube-scheduler's operational status. + MachineSchedulerPodHealthyCondition clusterv1.ConditionType = "SchedulerPodHealthy" + + // MachineEtcdPodHealthyCondition reports a machine's etcd pod's operational status. + // NOTE: This conditions exists only if a stacked etcd cluster is used. + MachineEtcdPodHealthyCondition clusterv1.ConditionType = "EtcdPodHealthy" + + // PodProvisioningReason (Severity=Info) documents a pod waiting to be provisioned i.e., Pod is in "Pending" phase. + PodProvisioningReason = "PodProvisioning" + + // PodMissingReason (Severity=Error) documents a pod does not exist. + PodMissingReason = "PodMissing" + + // PodFailedReason (Severity=Error) documents if a pod failed during provisioning i.e., e.g CrashLoopbackOff, ImagePullBackOff + // or if all the containers in a pod have terminated. + PodFailedReason = "PodFailed" + + // PodInspectionFailedReason documents a failure in inspecting the pod status. + PodInspectionFailedReason = "PodInspectionFailed" +) + +const ( + // EtcdClusterHealthyCondition documents the overall etcd cluster's health. + EtcdClusterHealthyCondition clusterv1.ConditionType = "EtcdClusterHealthyCondition" + + // EtcdClusterInspectionFailedReason documents a failure in inspecting the etcd cluster status. + EtcdClusterInspectionFailedReason = "EtcdClusterInspectionFailed" + + // EtcdClusterUnknownReason reports an etcd cluster in unknown status. + EtcdClusterUnknownReason = "EtcdClusterUnknown" + + // EtcdClusterUnhealthyReason (Severity=Error) is set when the etcd cluster is unhealthy. + EtcdClusterUnhealthyReason = "EtcdClusterUnhealthy" + + // MachineEtcdMemberHealthyCondition report the machine's etcd member's health status. + // NOTE: This conditions exists only if a stacked etcd cluster is used. + MachineEtcdMemberHealthyCondition clusterv1.ConditionType = "EtcdMemberHealthy" + + // EtcdMemberInspectionFailedReason documents a failure in inspecting the etcd member status. + EtcdMemberInspectionFailedReason = "MemberInspectionFailed" + + // EtcdMemberUnhealthyReason (Severity=Error) documents a Machine's etcd member is unhealthy. + EtcdMemberUnhealthyReason = "EtcdMemberUnhealthy" +) diff --git a/controlplane/kubeadm/controllers/consts.go b/controlplane/kubeadm/controllers/consts.go index 6c637913da78..798207f449af 100644 --- a/controlplane/kubeadm/controllers/consts.go +++ b/controlplane/kubeadm/controllers/consts.go @@ -23,9 +23,9 @@ const ( // all control plane machines have been deleted. deleteRequeueAfter = 30 * time.Second - // healthCheckFailedRequeueAfter is how long to wait before trying to scale - // up/down if some target cluster health check has failed - healthCheckFailedRequeueAfter = 20 * time.Second + // preflightFailedRequeueAfter is how long to wait before trying to scale + // up/down if some preflight check for those operation has failed + preflightFailedRequeueAfter = 15 * time.Second // dependentCertRequeueAfter is how long to wait before checking again to see if // dependent certificates have been created. diff --git a/controlplane/kubeadm/controllers/controller.go b/controlplane/kubeadm/controllers/controller.go index 1d3309c1ed6e..7f5356f6bf44 100644 --- a/controlplane/kubeadm/controllers/controller.go +++ b/controlplane/kubeadm/controllers/controller.go @@ -304,9 +304,15 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * // 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()) - // reconcileControlPlaneHealth returns err if there is a machine being deleted or if the control plane is unhealthy. - // If the control plane is not yet initialized, this call shouldn't fail. - if result, err := r.reconcileControlPlaneHealth(ctx, cluster, kcp, controlPlane); err != nil || !result.IsZero() { + // 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. + if result, err := r.reconcileControlPlaneConditions(ctx, controlPlane); err != nil || !result.IsZero() { + return result, err + } + + // Ensures the number of etcd members is in sync with the number of machines/nodes. + // NOTE: This is usually required after a machine deletion. + if result, err := r.reconcileEtcdMembers(ctx, controlPlane); err != nil || !result.IsZero() { return result, err } @@ -315,9 +321,7 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * switch { case len(needRollout) > 0: logger.Info("Rolling out Control Plane machines", "needRollout", needRollout.Names()) - // NOTE: we are using Status.UpdatedReplicas from the previous reconciliation only to provide a meaningful message - // and this does not influence any reconciliation logic. - conditions.MarkFalse(controlPlane.KCP, controlplanev1.MachinesSpecUpToDateCondition, controlplanev1.RollingUpdateInProgressReason, clusterv1.ConditionSeverityWarning, "Rolling %d replicas with outdated spec (%d replicas up to date)", len(needRollout), kcp.Status.UpdatedReplicas) + conditions.MarkFalse(controlPlane.KCP, controlplanev1.MachinesSpecUpToDateCondition, controlplanev1.RollingUpdateInProgressReason, clusterv1.ConditionSeverityWarning, "Rolling %d replicas with outdated spec (%d replicas up to date)", len(needRollout), len(controlPlane.Machines)-len(needRollout)) return r.upgradeControlPlane(ctx, cluster, kcp, controlPlane, needRollout) default: // make sure last upgrade operation is marked as completed. @@ -384,27 +388,6 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, clu logger := r.Log.WithValues("namespace", kcp.Namespace, "kubeadmControlPlane", kcp.Name, "cluster", cluster.Name) logger.Info("Reconcile KubeadmControlPlane deletion") - // Ignore the health check results here as well as the errors, health check functions are to set health related conditions on Machines. - // Errors may be due to not being able to get workload cluster nodes. - workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster)) - if err != nil { - r.Log.V(2).Info("Cannot get remote client to workload cluster during delete reconciliation", "err", err.Error()) - } else { - // Do a health check of the Control Plane components - _, err = workloadCluster.ControlPlaneIsHealthy(ctx) - if err != nil { - // Do nothing - r.Log.V(2).Info("Control plane did not pass control plane health check during delete reconciliation", "err", err.Error()) - } - - // Do a health check of the etcd - _, err = workloadCluster.EtcdIsHealthy(ctx) - if err != nil { - // Do nothing - r.Log.V(2).Info("Control plane did not pass etcd health check during delete reconciliation", "err", err.Error()) - } - } - // Gets all machines, not just control plane machines. allMachines, err := r.managementCluster.GetMachinesForCluster(ctx, util.ObjectKey(cluster)) if err != nil { @@ -418,6 +401,18 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, clu return ctrl.Result{}, nil } + controlPlane, err := internal.NewControlPlane(ctx, r.Client, cluster, kcp, ownedMachines) + if err != nil { + logger.Error(err, "failed to initialize control plane") + return ctrl.Result{}, err + } + + // Updates conditions reporting the status of static pods and the status of the etcd cluster. + // NOTE: Ignoring failures given that we are deleting + if _, err := r.reconcileControlPlaneConditions(ctx, controlPlane); err != nil { + logger.Info("failed to reconcile conditions", "error", err.Error()) + } + // 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. // However, during delete we are hiding the counter (1 of x) because it does not make sense given that @@ -469,62 +464,69 @@ func (r *KubeadmControlPlaneReconciler) ClusterToKubeadmControlPlane(o handler.M return nil } -// reconcileControlPlaneHealth performs health checks for control plane components and etcd -// It removes any etcd members that do not have a corresponding node. -// Also, as a final step, checks if there is any machines that is being deleted. -func (r *KubeadmControlPlaneReconciler) reconcileControlPlaneHealth(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, controlPlane *internal.ControlPlane) (ctrl.Result, error) { - // If there is no KCP-owned control-plane machines, then control-plane has not been initialized yet. - if controlPlane.Machines.Len() == 0 { +// reconcileControlPlaneConditions is responsible of reconciling conditions reporting the status of static pods and +// the status of the etcd cluster. +func (r *KubeadmControlPlaneReconciler) reconcileControlPlaneConditions(ctx context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error) { + // If the cluster is not yet initialized, there is no way to connect to the workload cluster and fetch information + // for updating conditions. Return early. + if !controlPlane.KCP.Status.Initialized { return ctrl.Result{}, nil } - workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster)) + workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster)) if err != nil { - // Failing at connecting to the workload cluster can mean workload cluster is unhealthy for a variety of reasons such as etcd quorum loss. return ctrl.Result{}, errors.Wrap(err, "cannot get remote client to workload cluster") } - errList := []error{} + // Update conditions status + workloadCluster.UpdateStaticPodConditions(ctx, controlPlane) + workloadCluster.UpdateEtcdConditions(ctx, controlPlane) - // Do a health check of the Control Plane components - checkResult, err := workloadCluster.ControlPlaneIsHealthy(ctx) - if err != nil { - errList = append(errList, errors.Wrap(err, "failed to pass control-plane health check")) - } else if err := checkResult.Aggregate(controlPlane); err != nil { - r.recorder.Eventf(kcp, corev1.EventTypeWarning, "ControlPlaneUnhealthy", - "Waiting for control plane to pass control plane health check to continue reconciliation: %v", err) - errList = append(errList, errors.Wrap(err, "failed to pass control-plane health check")) + // Patch machines with the updated conditions. + if err := controlPlane.PatchMachines(ctx); err != nil { + return ctrl.Result{}, err } - // If KCP should manage etcd, ensure etcd is healthy. - if controlPlane.IsEtcdManaged() { - checkResult, err := workloadCluster.EtcdIsHealthy(ctx) - if err != nil { - errList = append(errList, errors.Wrap(err, "failed to pass etcd health check")) - } else if err := checkResult.Aggregate(controlPlane); err != nil { - errList = append(errList, errors.Wrap(err, "failed to pass etcd health check")) - r.recorder.Eventf(kcp, corev1.EventTypeWarning, "ControlPlaneUnhealthy", - "Waiting for control plane to pass etcd health check to continue reconciliation: %v", err) - // If there are any etcd members that do not have corresponding nodes, remove them from etcd and from the kubeadm configmap. - // This will solve issues related to manual control-plane machine deletion. - workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster)) - if err != nil { - errList = append(errList, errors.Wrap(err, "cannot get remote client to workload cluster")) - } else if err := workloadCluster.ReconcileEtcdMembers(ctx); err != nil { - errList = append(errList, errors.Wrap(err, "failed attempt to remove potential hanging etcd members to pass etcd health check to continue reconciliation")) - } - } + // KCP will be patched at the end of Reconcile to reflect updated conditions, so we can return now. + return ctrl.Result{}, nil +} + +// reconcileEtcdMembers ensures the number of etcd members is in sync with the number of machines/nodes. +// This is usually required after a machine deletion. +// +// NOTE: this func uses KCP conditions, it is required to call reconcileControlPlaneConditions before this. +func (r *KubeadmControlPlaneReconciler) reconcileEtcdMembers(ctx context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error) { + logger := r.Log.WithValues("namespace", controlPlane.KCP.Namespace, "kubeadmControlPlane", controlPlane.KCP.Name, "cluster", controlPlane.Cluster.Name) + + // If etcd is not managed by KCP this is a no-op. + if !controlPlane.IsEtcdManaged() { + return ctrl.Result{}, nil + } + + // If there is no KCP-owned control-plane machines, then control-plane has not been initialized yet. + if controlPlane.Machines.Len() == 0 { + return ctrl.Result{}, nil } - if len(errList) > 0 { - return ctrl.Result{}, kerrors.NewAggregate(errList) + // Potential inconsistencies between the list of members and the list of machines/nodes are + // surfaced using the EtcdClusterHealthyCondition; if this condition is true, meaning no inconsistencies exists, return early. + if conditions.IsTrue(controlPlane.KCP, controlplanev1.EtcdClusterHealthyCondition) { + return ctrl.Result{}, nil } - // We need this check for scale up as well as down to avoid scaling up when there is a machine being deleted. - // This should be at the end of this method as no need to wait for machine to be completely deleted to reconcile etcd. - // TODO: Revisit during machine remediation implementation which may need to cover other machine phases. - if controlPlane.HasDeletingMachine() { - return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil + workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster)) + if err != nil { + // Failing at connecting to the workload cluster can mean workload cluster is unhealthy for a variety of reasons such as etcd quorum loss. + return ctrl.Result{}, errors.Wrap(err, "cannot get remote client to workload cluster") + } + + removedMembers, err := workloadCluster.ReconcileEtcdMembers(ctx) + if err != nil { + return ctrl.Result{}, errors.Wrap(err, "failed attempt to reconcile etcd members") + } + + if len(removedMembers) > 0 { + logger.Info("Etcd members without nodes removed from the cluster", "members", removedMembers) } return ctrl.Result{}, nil diff --git a/controlplane/kubeadm/controllers/controller_test.go b/controlplane/kubeadm/controllers/controller_test.go index b6315bc66080..0346812432c8 100644 --- a/controlplane/kubeadm/controllers/controller_test.go +++ b/controlplane/kubeadm/controllers/controller_test.go @@ -397,10 +397,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { fmc := &fakeManagementCluster{ Machines: internal.FilterableMachineCollection{}, - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, } objs := []runtime.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} for i := 0; i < 3; i++ { @@ -468,10 +465,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { fmc := &fakeManagementCluster{ Machines: internal.FilterableMachineCollection{}, - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, } objs := []runtime.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} for i := 0; i < 3; i++ { @@ -585,10 +579,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { fmc := &fakeManagementCluster{ Machines: internal.FilterableMachineCollection{}, - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, } objs := []runtime.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} for i := 0; i < 3; i++ { @@ -671,10 +662,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { }, }, }, - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, } fakeClient := newFakeClient(g, cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy(), fmc.Machines["test0"].DeepCopy()) @@ -1187,10 +1175,7 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { Client: fakeClient, managementCluster: &fakeManagementCluster{ Management: &internal.Management{Client: fakeClient}, - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, }, Log: log.Log, recorder: record.NewFakeRecorder(32), @@ -1240,10 +1225,7 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { Client: fakeClient, managementCluster: &fakeManagementCluster{ Management: &internal.Management{Client: fakeClient}, - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, }, Log: log.Log, recorder: record.NewFakeRecorder(32), @@ -1275,10 +1257,7 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { Client: fakeClient, managementCluster: &fakeManagementCluster{ Management: &internal.Management{Client: fakeClient}, - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, }, recorder: record.NewFakeRecorder(32), Log: log.Log, @@ -1394,6 +1373,11 @@ func createClusterWithControlPlane() (*clusterv1.Cluster, *controlplanev1.Kubead return cluster, kcp, genericMachineTemplate } +func setKCPHealthy(kcp *controlplanev1.KubeadmControlPlane) { + conditions.MarkTrue(kcp, controlplanev1.ControlPlaneComponentsHealthyCondition) + conditions.MarkTrue(kcp, controlplanev1.EtcdClusterHealthyCondition) +} + func createMachineNodePair(name string, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ready bool) (*clusterv1.Machine, *corev1.Node) { machine := &clusterv1.Machine{ TypeMeta: metav1.TypeMeta{ @@ -1446,6 +1430,14 @@ func createMachineNodePair(name string, cluster *clusterv1.Cluster, kcp *control return machine, node } +func setMachineHealthy(m *clusterv1.Machine) { + conditions.MarkTrue(m, controlplanev1.MachineAPIServerPodHealthyCondition) + conditions.MarkTrue(m, controlplanev1.MachineControllerManagerPodHealthyCondition) + conditions.MarkTrue(m, controlplanev1.MachineSchedulerPodHealthyCondition) + conditions.MarkTrue(m, controlplanev1.MachineEtcdPodHealthyCondition) + conditions.MarkTrue(m, controlplanev1.MachineEtcdMemberHealthyCondition) +} + // newCluster return a CAPI cluster object func newCluster(namespacedName *types.NamespacedName) *clusterv1.Cluster { return &clusterv1.Cluster{ diff --git a/controlplane/kubeadm/controllers/fakes_test.go b/controlplane/kubeadm/controllers/fakes_test.go index 48e8b921f6f3..c370894c6bc5 100644 --- a/controlplane/kubeadm/controllers/fakes_test.go +++ b/controlplane/kubeadm/controllers/fakes_test.go @@ -18,7 +18,6 @@ package controllers import ( "context" - "errors" "github.com/blang/semver" "k8s.io/apimachinery/pkg/runtime" @@ -57,17 +56,15 @@ func (f *fakeManagementCluster) GetMachinesForCluster(c context.Context, n clien type fakeWorkloadCluster struct { *internal.Workload - Status internal.ClusterStatus - ControlPlaneHealthy bool - EtcdHealthy bool + Status internal.ClusterStatus } func (f fakeWorkloadCluster) ForwardEtcdLeadership(_ context.Context, _ *clusterv1.Machine, _ *clusterv1.Machine) error { return nil } -func (f fakeWorkloadCluster) ReconcileEtcdMembers(ctx context.Context) error { - return nil +func (f fakeWorkloadCluster) ReconcileEtcdMembers(ctx context.Context) ([]string, error) { + return nil, nil } func (f fakeWorkloadCluster) ClusterStatus(_ context.Context) (internal.ClusterStatus, error) { @@ -98,20 +95,6 @@ func (f fakeWorkloadCluster) UpdateKubeletConfigMap(ctx context.Context, version return nil } -func (f fakeWorkloadCluster) ControlPlaneIsHealthy(ctx context.Context) (internal.HealthCheckResult, error) { - if !f.ControlPlaneHealthy { - return nil, errors.New("control plane is not healthy") - } - return nil, nil -} - -func (f fakeWorkloadCluster) EtcdIsHealthy(ctx context.Context) (internal.HealthCheckResult, error) { - if !f.EtcdHealthy { - return nil, errors.New("etcd is not healthy") - } - return nil, nil -} - type fakeMigrator struct { migrateCalled bool migrateErr error diff --git a/controlplane/kubeadm/controllers/scale.go b/controlplane/kubeadm/controllers/scale.go index 27faa9e77dce..1fe8f482d6d7 100644 --- a/controlplane/kubeadm/controllers/scale.go +++ b/controlplane/kubeadm/controllers/scale.go @@ -18,15 +18,18 @@ package controllers import ( "context" + "strings" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + kerrors "k8s.io/apimachinery/pkg/util/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/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/conditions" ctrl "sigs.k8s.io/controller-runtime" ) @@ -62,6 +65,11 @@ func (r *KubeadmControlPlaneReconciler) initializeControlPlane(ctx context.Conte func (r *KubeadmControlPlaneReconciler) scaleUpControlPlane(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, controlPlane *internal.ControlPlane) (ctrl.Result, error) { logger := controlPlane.Logger() + // Run preflight checks to ensure that the control plane is stable before proceeding with a scale up/scale down operation; if not, wait. + if result, err := r.preflightChecks(ctx, controlPlane); err != nil || !result.IsZero() { + return result, err + } + // Create the bootstrap configuration bootstrapSpec := controlPlane.JoinControlPlaneConfig() fd := controlPlane.NextFailureDomainForScaleUp() @@ -84,6 +92,11 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane( ) (ctrl.Result, error) { logger := controlPlane.Logger() + // run preflight checks ensuring the control plane is stable before proceeding with a scale up/scale down operation; if not, wait. + if result, err := r.preflightChecks(ctx, controlPlane); err != nil || !result.IsZero() { + return result, err + } + workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster)) if err != nil { logger.Error(err, "Failed to create client to workload cluster") @@ -130,6 +143,97 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane( return ctrl.Result{Requeue: true}, nil } +// preflightChecks checks if the control plane is stable before proceeding with a scale up/scale down operation, +// where stable means that: +// - There are no machine deletion in progress +// - All the health conditions on KCP are true. +// - All the health conditions on the control plane machines are true. +// If the control plane is not passing preflight checks, it requeue. +// +// NOTE: this func uses KCP conditions, it is required to call reconcileControlPlaneConditions before this. +func (r *KubeadmControlPlaneReconciler) preflightChecks(_ context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error) { //nolint:unparam + logger := r.Log.WithValues("namespace", controlPlane.KCP.Namespace, "kubeadmControlPlane", controlPlane.KCP.Name, "cluster", controlPlane.Cluster.Name) + + // If there is no KCP-owned control-plane machines, then control-plane has not been initialized yet, + // so it is considered ok to proceed. + if controlPlane.Machines.Len() == 0 { + return ctrl.Result{}, nil + } + + // If there are deleting machines, wait for the operation to complete. + if controlPlane.HasDeletingMachine() { + logger.Info("Waiting for machines to be deleted", "Machines", strings.Join(controlPlane.Machines.Filter(machinefilters.HasDeletionTimestamp).Names(), ", ")) + return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil + } + + // Check machine health conditions; if there are conditions with False or Unknown, then wait. + allMachineHealthConditions := []clusterv1.ConditionType{ + controlplanev1.MachineAPIServerPodHealthyCondition, + controlplanev1.MachineControllerManagerPodHealthyCondition, + controlplanev1.MachineSchedulerPodHealthyCondition, + } + if controlPlane.IsEtcdManaged() { + allMachineHealthConditions = append(allMachineHealthConditions, + controlplanev1.MachineEtcdPodHealthyCondition, + controlplanev1.MachineEtcdMemberHealthyCondition, + ) + } + machineErrors := []error{} + for _, machine := range controlPlane.Machines { + for _, condition := range allMachineHealthConditions { + if err := preflightCheckCondition("machine", machine, condition); err != nil { + machineErrors = append(machineErrors, err) + } + } + } + if len(machineErrors) > 0 { + aggregatedError := kerrors.NewAggregate(machineErrors) + r.recorder.Eventf(controlPlane.KCP, corev1.EventTypeWarning, "ControlPlaneUnhealthy", + "Waiting for control plane to pass preflight checks to continue reconciliation: %v", aggregatedError) + logger.Info("Waiting for control plane to pass preflight checks", "failures", aggregatedError.Error()) + + return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil + } + + // Check KCP conditions ; if there are health problems wait. + // NOTE: WE are checking KCP conditions for problems that can't be assigned to a specific machine, e.g. + // a control plane node without a corresponding machine + allKcpHealthConditions := []clusterv1.ConditionType{ + controlplanev1.ControlPlaneComponentsHealthyCondition, + controlplanev1.EtcdClusterHealthyCondition, + } + kcpErrors := []error{} + for _, condition := range allKcpHealthConditions { + if err := preflightCheckCondition("control plane", controlPlane.KCP, condition); err != nil { + kcpErrors = append(kcpErrors, err) + } + } + if len(kcpErrors) > 0 { + aggregatedError := kerrors.NewAggregate(kcpErrors) + r.recorder.Eventf(controlPlane.KCP, corev1.EventTypeWarning, "ControlPlaneUnhealthy", + "Waiting for control plane to pass preflight checks to continue reconciliation: %v", aggregatedError) + logger.Info("Waiting for control plane to pass preflight checks", "failures", aggregatedError.Error()) + + return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil + } + + return ctrl.Result{}, nil +} + +func preflightCheckCondition(kind string, obj conditions.Getter, condition clusterv1.ConditionType) error { + c := conditions.Get(obj, condition) + if c == nil { + return errors.Errorf("%s %s does not have %s condition", kind, obj.GetName(), condition) + } + if c.Status == corev1.ConditionFalse { + return errors.Errorf("%s %s reports %s condition is false (%s, %s)", kind, obj.GetName(), condition, c.Severity, c.Message) + } + if c.Status == corev1.ConditionUnknown { + return errors.Errorf("%s %s reports %s condition is unknown (%s)", kind, obj.GetName(), condition, c.Message) + } + return nil +} + func selectMachineForScaleDown(controlPlane *internal.ControlPlane, outdatedMachines internal.FilterableMachineCollection) (*clusterv1.Machine, error) { machines := controlPlane.Machines if outdatedMachines.Len() > 0 { diff --git a/controlplane/kubeadm/controllers/scale_test.go b/controlplane/kubeadm/controllers/scale_test.go index 3d372d4add37..d9cffdbec16e 100644 --- a/controlplane/kubeadm/controllers/scale_test.go +++ b/controlplane/kubeadm/controllers/scale_test.go @@ -23,6 +23,7 @@ import ( "time" . "github.com/onsi/gomega" + "sigs.k8s.io/cluster-api/util/conditions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -81,22 +82,21 @@ func TestKubeadmControlPlaneReconciler_initializeControlPlane(t *testing.T) { } func TestKubeadmControlPlaneReconciler_scaleUpControlPlane(t *testing.T) { - t.Run("creates a control plane Machine if health checks pass", func(t *testing.T) { + t.Run("creates a control plane Machine if preflight checks pass", func(t *testing.T) { g := NewWithT(t) cluster, kcp, genericMachineTemplate := createClusterWithControlPlane() + setKCPHealthy(kcp) initObjs := []runtime.Object{cluster.DeepCopy(), kcp.DeepCopy(), genericMachineTemplate.DeepCopy()} fmc := &fakeManagementCluster{ Machines: internal.NewFilterableMachineCollection(), - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, - }, + Workload: fakeWorkloadCluster{}, } for i := 0; i < 2; i++ { m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster, kcp, true) + setMachineHealthy(m) fmc.Machines.Insert(m) initObjs = append(initObjs, m.DeepCopy()) } @@ -124,7 +124,7 @@ func TestKubeadmControlPlaneReconciler_scaleUpControlPlane(t *testing.T) { g.Expect(fakeClient.List(context.Background(), &controlPlaneMachines)).To(Succeed()) g.Expect(controlPlaneMachines.Items).To(HaveLen(3)) }) - t.Run("does not create a control plane Machine if health checks fail", func(t *testing.T) { + t.Run("does not create a control plane Machine if preflight checks fail", func(t *testing.T) { cluster, kcp, genericMachineTemplate := createClusterWithControlPlane() cluster.Spec.ControlPlaneEndpoint.Host = "nodomain.example.com" cluster.Spec.ControlPlaneEndpoint.Port = 6443 @@ -137,93 +137,110 @@ func TestKubeadmControlPlaneReconciler_scaleUpControlPlane(t *testing.T) { initObjs = append(initObjs, m.DeepCopy()) } - testCases := []struct { - name string - etcdUnHealthy bool - controlPlaneUnHealthy bool - expectErr bool - expectResult ctrl.Result - }{ - { - name: "etcd health check fails", - etcdUnHealthy: true, - expectErr: true, - expectResult: ctrl.Result{}, - }, - { - name: "controlplane component health check fails", - controlPlaneUnHealthy: true, - expectErr: false, - expectResult: ctrl.Result{RequeueAfter: healthCheckFailedRequeueAfter}, - }, - } - for _, tc := range testCases { - g := NewWithT(t) - - fakeClient := newFakeClient(g, initObjs...) - fmc := &fakeManagementCluster{ - Machines: beforeMachines.DeepCopy(), - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: !tc.controlPlaneUnHealthy, - EtcdHealthy: !tc.etcdUnHealthy, - }, - } - - r := &KubeadmControlPlaneReconciler{ - Client: fakeClient, - managementCluster: fmc, - managementClusterUncached: fmc, - Log: log.Log, - recorder: record.NewFakeRecorder(32), - } + g := NewWithT(t) - _, err := r.reconcile(context.Background(), cluster, kcp) - g.Expect(err).To(HaveOccurred()) + fakeClient := newFakeClient(g, initObjs...) + fmc := &fakeManagementCluster{ + Machines: beforeMachines.DeepCopy(), + Workload: fakeWorkloadCluster{}, + } - // scaleUpControlPlane is never called due to health check failure and new machine is not created to scale up. - controlPlaneMachines := &clusterv1.MachineList{} - g.Expect(fakeClient.List(context.Background(), controlPlaneMachines)).To(Succeed()) - g.Expect(controlPlaneMachines.Items).To(HaveLen(len(beforeMachines))) + r := &KubeadmControlPlaneReconciler{ + Client: fakeClient, + managementCluster: fmc, + managementClusterUncached: fmc, + Log: log.Log, + recorder: record.NewFakeRecorder(32), + } - endMachines := internal.NewFilterableMachineCollectionFromMachineList(controlPlaneMachines) - for _, m := range endMachines { - bm, ok := beforeMachines[m.Name] - bm.SetResourceVersion("1") - g.Expect(ok).To(BeTrue()) - g.Expect(m).To(Equal(bm)) - } + result, err := r.reconcile(context.Background(), cluster, kcp) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: preflightFailedRequeueAfter})) + + // scaleUpControlPlane is never called due to health check failure and new machine is not created to scale up. + controlPlaneMachines := &clusterv1.MachineList{} + g.Expect(fakeClient.List(context.Background(), controlPlaneMachines)).To(Succeed()) + g.Expect(controlPlaneMachines.Items).To(HaveLen(len(beforeMachines))) + + endMachines := internal.NewFilterableMachineCollectionFromMachineList(controlPlaneMachines) + for _, m := range endMachines { + bm, ok := beforeMachines[m.Name] + bm.SetResourceVersion("1") + g.Expect(ok).To(BeTrue()) + g.Expect(m).To(Equal(bm)) } }) } func TestKubeadmControlPlaneReconciler_scaleDownControlPlane_NoError(t *testing.T) { - g := NewWithT(t) + t.Run("deletes control plane Machine if preflight checks pass", func(t *testing.T) { + g := NewWithT(t) - machines := map[string]*clusterv1.Machine{ - "one": machine("one"), - } + machines := map[string]*clusterv1.Machine{ + "one": machine("one"), + } + setMachineHealthy(machines["one"]) + fakeClient := newFakeClient(g, machines["one"]) - r := &KubeadmControlPlaneReconciler{ - Log: log.Log, - recorder: record.NewFakeRecorder(32), - Client: newFakeClient(g, machines["one"]), - managementCluster: &fakeManagementCluster{ - Workload: fakeWorkloadCluster{ - ControlPlaneHealthy: true, - EtcdHealthy: true, + r := &KubeadmControlPlaneReconciler{ + Log: log.Log, + recorder: record.NewFakeRecorder(32), + Client: fakeClient, + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{}, }, - }, - } - cluster := &clusterv1.Cluster{} - kcp := &controlplanev1.KubeadmControlPlane{} - controlPlane := &internal.ControlPlane{ - KCP: kcp, - Cluster: cluster, - Machines: machines, - } + } + + cluster := &clusterv1.Cluster{} + kcp := &controlplanev1.KubeadmControlPlane{} + setKCPHealthy(kcp) + controlPlane := &internal.ControlPlane{ + KCP: kcp, + Cluster: cluster, + Machines: machines, + } + + result, err := r.scaleDownControlPlane(context.Background(), cluster, kcp, controlPlane, controlPlane.Machines) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{Requeue: true})) + + controlPlaneMachines := clusterv1.MachineList{} + g.Expect(fakeClient.List(context.Background(), &controlPlaneMachines)).To(Succeed()) + g.Expect(controlPlaneMachines.Items).To(HaveLen(0)) + }) + t.Run("does not deletes control plane Machine if preflight checks fails", func(t *testing.T) { + g := NewWithT(t) - _, err := r.scaleDownControlPlane(context.Background(), cluster, kcp, controlPlane, controlPlane.Machines) - g.Expect(err).ToNot(HaveOccurred()) + machines := map[string]*clusterv1.Machine{ + "one": machine("one"), + } + fakeClient := newFakeClient(g, machines["one"]) + + r := &KubeadmControlPlaneReconciler{ + Log: log.Log, + recorder: record.NewFakeRecorder(32), + Client: fakeClient, + managementCluster: &fakeManagementCluster{ + Workload: fakeWorkloadCluster{}, + }, + } + + cluster := &clusterv1.Cluster{} + kcp := &controlplanev1.KubeadmControlPlane{} + controlPlane := &internal.ControlPlane{ + KCP: kcp, + Cluster: cluster, + Machines: machines, + } + + result, err := r.scaleDownControlPlane(context.Background(), cluster, kcp, controlPlane, controlPlane.Machines) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: preflightFailedRequeueAfter})) + + controlPlaneMachines := clusterv1.MachineList{} + g.Expect(fakeClient.List(context.Background(), &controlPlaneMachines)).To(Succeed()) + g.Expect(controlPlaneMachines.Items).To(HaveLen(1)) + }) } func TestSelectMachineForScaleDown(t *testing.T) { @@ -298,6 +315,182 @@ func TestSelectMachineForScaleDown(t *testing.T) { } } +func TestPreflightChecks(t *testing.T) { + testCases := []struct { + name string + kcp *controlplanev1.KubeadmControlPlane + machines []*clusterv1.Machine + expectResult ctrl.Result + }{ + { + name: "control plane without machines (not initialized) should pass", + kcp: &controlplanev1.KubeadmControlPlane{}, + expectResult: ctrl.Result{}, + }, + { + name: "control plane with a deleting machine should requeue", + kcp: &controlplanev1.KubeadmControlPlane{}, + machines: []*clusterv1.Machine{ + { + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + }, + }, + }, + expectResult: ctrl.Result{RequeueAfter: deleteRequeueAfter}, + }, + { + name: "control plane with an unhealthy machine condition should requeue", + kcp: &controlplanev1.KubeadmControlPlane{}, + machines: []*clusterv1.Machine{ + { + Status: clusterv1.MachineStatus{ + Conditions: clusterv1.Conditions{ + *conditions.FalseCondition(controlplanev1.MachineAPIServerPodHealthyCondition, "fooReason", clusterv1.ConditionSeverityError, ""), + *conditions.TrueCondition(controlplanev1.MachineControllerManagerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineSchedulerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineEtcdPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + }, + }, + }, + expectResult: ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, + }, + { + name: "control plane with healthy machine conditions but with unhealthy kcp conditions should requeue", + kcp: &controlplanev1.KubeadmControlPlane{ + Status: controlplanev1.KubeadmControlPlaneStatus{ + Conditions: clusterv1.Conditions{ + *conditions.FalseCondition(controlplanev1.ControlPlaneComponentsHealthyCondition, "fooReason", clusterv1.ConditionSeverityError, ""), + *conditions.TrueCondition(controlplanev1.EtcdClusterHealthyCondition), + }, + }, + }, + machines: []*clusterv1.Machine{ + { + Status: clusterv1.MachineStatus{ + Conditions: clusterv1.Conditions{ + *conditions.TrueCondition(controlplanev1.MachineAPIServerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineControllerManagerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineSchedulerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineEtcdPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + }, + }, + }, + expectResult: ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, + }, + { + name: "control plane with an healthy machine and an healthy kcp condition should pass", + kcp: &controlplanev1.KubeadmControlPlane{ + Status: controlplanev1.KubeadmControlPlaneStatus{ + Conditions: clusterv1.Conditions{ + *conditions.TrueCondition(controlplanev1.ControlPlaneComponentsHealthyCondition), + *conditions.TrueCondition(controlplanev1.EtcdClusterHealthyCondition), + }, + }, + }, + machines: []*clusterv1.Machine{ + { + Status: clusterv1.MachineStatus{ + Conditions: clusterv1.Conditions{ + *conditions.TrueCondition(controlplanev1.MachineAPIServerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineControllerManagerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineSchedulerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineEtcdPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + }, + }, + }, + expectResult: ctrl.Result{}, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + r := &KubeadmControlPlaneReconciler{ + Log: log.Log, + recorder: record.NewFakeRecorder(32), + } + controlPlane := &internal.ControlPlane{ + Cluster: &clusterv1.Cluster{}, + KCP: tt.kcp, + Machines: internal.NewFilterableMachineCollection(tt.machines...), + } + result, err := r.preflightChecks(context.TODO(), controlPlane) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).To(Equal(tt.expectResult)) + }) + } +} + +func TestPreflightCheckCondition(t *testing.T) { + condition := clusterv1.ConditionType("fooCondition") + testCases := []struct { + name string + machine *clusterv1.Machine + expectErr bool + }{ + { + name: "missing condition should return error", + machine: &clusterv1.Machine{}, + expectErr: true, + }, + { + name: "false condition should return error", + machine: &clusterv1.Machine{ + Status: clusterv1.MachineStatus{ + Conditions: clusterv1.Conditions{ + *conditions.FalseCondition(condition, "fooReason", clusterv1.ConditionSeverityError, ""), + }, + }, + }, + expectErr: true, + }, + { + name: "unknown condition should return error", + machine: &clusterv1.Machine{ + Status: clusterv1.MachineStatus{ + Conditions: clusterv1.Conditions{ + *conditions.UnknownCondition(condition, "fooReason", ""), + }, + }, + }, + expectErr: true, + }, + { + name: "true condition should not return error", + machine: &clusterv1.Machine{ + Status: clusterv1.MachineStatus{ + Conditions: clusterv1.Conditions{ + *conditions.TrueCondition(condition), + }, + }, + }, + expectErr: false, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + err := preflightCheckCondition("machine", tt.machine, condition) + + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).NotTo(HaveOccurred()) + }) + } +} + func failureDomain(controlPlane bool) clusterv1.FailureDomainSpec { return clusterv1.FailureDomainSpec{ ControlPlane: controlPlane, diff --git a/controlplane/kubeadm/controllers/upgrade_test.go b/controlplane/kubeadm/controllers/upgrade_test.go index 4a3df31ae55b..dd6b4a19c613 100644 --- a/controlplane/kubeadm/controllers/upgrade_test.go +++ b/controlplane/kubeadm/controllers/upgrade_test.go @@ -41,6 +41,7 @@ func TestKubeadmControlPlaneReconciler_upgradeControlPlane(t *testing.T) { kcp.Spec.Version = "v1.17.3" kcp.Spec.KubeadmConfigSpec.ClusterConfiguration = nil kcp.Spec.Replicas = pointer.Int32Ptr(1) + setKCPHealthy(kcp) fakeClient := newFakeClient(g, cluster.DeepCopy(), kcp.DeepCopy(), genericMachineTemplate.DeepCopy()) @@ -51,17 +52,13 @@ func TestKubeadmControlPlaneReconciler_upgradeControlPlane(t *testing.T) { managementCluster: &fakeManagementCluster{ Management: &internal.Management{Client: fakeClient}, Workload: fakeWorkloadCluster{ - Status: internal.ClusterStatus{Nodes: 1}, - ControlPlaneHealthy: true, - EtcdHealthy: true, + Status: internal.ClusterStatus{Nodes: 1}, }, }, managementClusterUncached: &fakeManagementCluster{ Management: &internal.Management{Client: fakeClient}, Workload: fakeWorkloadCluster{ - Status: internal.ClusterStatus{Nodes: 1}, - ControlPlaneHealthy: true, - EtcdHealthy: true, + Status: internal.ClusterStatus{Nodes: 1}, }, }, } @@ -79,6 +76,9 @@ func TestKubeadmControlPlaneReconciler_upgradeControlPlane(t *testing.T) { initialMachine := &clusterv1.MachineList{} g.Expect(fakeClient.List(context.Background(), initialMachine, client.InNamespace(cluster.Namespace))).To(Succeed()) g.Expect(initialMachine.Items).To(HaveLen(1)) + for i := range initialMachine.Items { + setMachineHealthy(&initialMachine.Items[i]) + } // change the KCP spec so the machine becomes outdated kcp.Spec.Version = "v1.17.4" @@ -94,18 +94,20 @@ func TestKubeadmControlPlaneReconciler_upgradeControlPlane(t *testing.T) { g.Expect(bothMachines.Items).To(HaveLen(2)) // run upgrade a second time, simulate that the node has not appeared yet but the machine exists - r.managementCluster.(*fakeManagementCluster).Workload.ControlPlaneHealthy = false + // Unhealthy control plane will be detected during reconcile loop and upgrade will never be called. - _, err = r.reconcile(context.Background(), cluster, kcp) - g.Expect(err).To(HaveOccurred()) + result, err = r.reconcile(context.Background(), cluster, kcp) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: preflightFailedRequeueAfter})) g.Expect(fakeClient.List(context.Background(), bothMachines, client.InNamespace(cluster.Namespace))).To(Succeed()) g.Expect(bothMachines.Items).To(HaveLen(2)) - controlPlane.Machines = internal.NewFilterableMachineCollectionFromMachineList(bothMachines) - // manually increase number of nodes, make control plane healthy again r.managementCluster.(*fakeManagementCluster).Workload.Status.Nodes++ - r.managementCluster.(*fakeManagementCluster).Workload.ControlPlaneHealthy = true + for i := range bothMachines.Items { + setMachineHealthy(&bothMachines.Items[i]) + } + controlPlane.Machines = internal.NewFilterableMachineCollectionFromMachineList(bothMachines) // run upgrade the second time, expect we scale down result, err = r.upgradeControlPlane(context.Background(), cluster, kcp, controlPlane, controlPlane.Machines) diff --git a/controlplane/kubeadm/internal/cluster_test.go b/controlplane/kubeadm/internal/cluster_test.go index 7411e8253ed9..db62c06132f8 100644 --- a/controlplane/kubeadm/internal/cluster_test.go +++ b/controlplane/kubeadm/internal/cluster_test.go @@ -44,86 +44,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func TestCheckStaticPodReadyCondition(t *testing.T) { - table := []checkStaticPodReadyConditionTest{ - { - name: "pod is ready", - conditions: []corev1.PodCondition{podReady(corev1.ConditionTrue)}, - }, - } - for _, test := range table { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - - pod := corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - }, - Spec: corev1.PodSpec{}, - Status: corev1.PodStatus{Conditions: test.conditions}, - } - g.Expect(checkStaticPodReadyCondition(pod)).To(Succeed()) - }) - } -} - -func TestCheckStaticPodNotReadyCondition(t *testing.T) { - table := []checkStaticPodReadyConditionTest{ - { - name: "no pod status", - }, - { - name: "not ready pod status", - conditions: []corev1.PodCondition{podReady(corev1.ConditionFalse)}, - }, - } - for _, test := range table { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - - pod := corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pod", - }, - Spec: corev1.PodSpec{}, - Status: corev1.PodStatus{Conditions: test.conditions}, - } - g.Expect(checkStaticPodReadyCondition(pod)).NotTo(Succeed()) - }) - } -} - -func TestControlPlaneIsHealthy(t *testing.T) { - g := NewWithT(t) - - readyStatus := corev1.PodStatus{ - Conditions: []corev1.PodCondition{ - { - Type: corev1.PodReady, - Status: corev1.ConditionTrue, - }, - }, - } - workloadCluster := &Workload{ - Client: &fakeClient{ - list: nodeListForTestControlPlaneIsHealthy(), - get: map[string]interface{}{ - "kube-system/kube-apiserver-first-control-plane": &corev1.Pod{Status: readyStatus}, - "kube-system/kube-apiserver-second-control-plane": &corev1.Pod{Status: readyStatus}, - "kube-system/kube-apiserver-third-control-plane": &corev1.Pod{Status: readyStatus}, - "kube-system/kube-controller-manager-first-control-plane": &corev1.Pod{Status: readyStatus}, - "kube-system/kube-controller-manager-second-control-plane": &corev1.Pod{Status: readyStatus}, - "kube-system/kube-controller-manager-third-control-plane": &corev1.Pod{Status: readyStatus}, - }, - }, - } - - health, err := workloadCluster.ControlPlaneIsHealthy(context.Background()) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(health).NotTo(HaveLen(0)) - g.Expect(health).To(HaveLen(len(nodeListForTestControlPlaneIsHealthy().Items))) -} - func TestGetMachinesForCluster(t *testing.T) { g := NewWithT(t) @@ -310,40 +230,6 @@ func getTestCACert(key *rsa.PrivateKey) (*x509.Certificate, error) { return c, err } -func podReady(isReady corev1.ConditionStatus) corev1.PodCondition { - return corev1.PodCondition{ - Type: corev1.PodReady, - Status: isReady, - } -} - -type checkStaticPodReadyConditionTest struct { - name string - conditions []corev1.PodCondition -} - -func nodeListForTestControlPlaneIsHealthy() *corev1.NodeList { - return &corev1.NodeList{ - Items: []corev1.Node{ - nodeNamed("first-control-plane"), - nodeNamed("second-control-plane"), - nodeNamed("third-control-plane"), - }, - } -} - -func nodeNamed(name string, options ...func(n corev1.Node) corev1.Node) corev1.Node { - node := corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - } - for _, opt := range options { - node = opt(node) - } - return node -} - func machineListForTestGetMachinesForCluster() *clusterv1.MachineList { owned := true ownedRef := []metav1.OwnerReference{ diff --git a/controlplane/kubeadm/internal/control_plane.go b/controlplane/kubeadm/internal/control_plane.go index 0c2c02dffd56..cb1568b8b120 100644 --- a/controlplane/kubeadm/internal/control_plane.go +++ b/controlplane/kubeadm/internal/control_plane.go @@ -25,12 +25,14 @@ 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/apiserver/pkg/storage/names" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3" "sigs.k8s.io/cluster-api/controllers/external" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" + "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -38,9 +40,10 @@ import ( // It should never need to connect to a service, that responsibility lies outside of this struct. // Going forward we should be trying to add more logic to here and reduce the amount of logic in the reconciler. type ControlPlane struct { - KCP *controlplanev1.KubeadmControlPlane - Cluster *clusterv1.Cluster - Machines FilterableMachineCollection + KCP *controlplanev1.KubeadmControlPlane + Cluster *clusterv1.Cluster + Machines FilterableMachineCollection + machinesPatchHelpers map[string]*patch.Helper // reconciliationTime is the time of the current reconciliation, and should be used for all "now" calculations reconciliationTime metav1.Time @@ -61,13 +64,23 @@ func NewControlPlane(ctx context.Context, client client.Client, cluster *cluster if err != nil { return nil, err } + patchHelpers := map[string]*patch.Helper{} + for _, machine := range ownedMachines { + patchHelper, err := patch.NewHelper(machine, client) + if err != nil { + return nil, errors.Wrapf(err, "failed to create patch helper for machine %s", machine.Name) + } + patchHelpers[machine.Name] = patchHelper + } + return &ControlPlane{ - KCP: kcp, - Cluster: cluster, - Machines: ownedMachines, - kubeadmConfigs: kubeadmConfigs, - infraResources: infraObjects, - reconciliationTime: metav1.Now(), + KCP: kcp, + Cluster: cluster, + Machines: ownedMachines, + machinesPatchHelpers: patchHelpers, + kubeadmConfigs: kubeadmConfigs, + infraResources: infraObjects, + reconciliationTime: metav1.Now(), }, nil } @@ -286,3 +299,24 @@ func getKubeadmConfigs(ctx context.Context, cl client.Client, machines Filterabl func (c *ControlPlane) IsEtcdManaged() bool { return c.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration == nil || c.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External == nil } + +func (c *ControlPlane) PatchMachines(ctx context.Context) error { + errList := []error{} + for i := range c.Machines { + machine := c.Machines[i] + if helper, ok := c.machinesPatchHelpers[machine.Name]; ok { + if err := helper.Patch(ctx, machine, patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ + controlplanev1.MachineAPIServerPodHealthyCondition, + controlplanev1.MachineControllerManagerPodHealthyCondition, + controlplanev1.MachineSchedulerPodHealthyCondition, + controlplanev1.MachineEtcdPodHealthyCondition, + controlplanev1.MachineEtcdMemberHealthyCondition, + }}); err != nil { + errList = append(errList, errors.Wrapf(err, "failed to patch machine %s", machine.Name)) + } + continue + } + errList = append(errList, errors.Errorf("failed to get patch helper for machine %s", machine.Name)) + } + return kerrors.NewAggregate(errList) +} diff --git a/controlplane/kubeadm/internal/etcd/etcd.go b/controlplane/kubeadm/internal/etcd/etcd.go index a71fbb898de7..9a9cb7614963 100644 --- a/controlplane/kubeadm/internal/etcd/etcd.go +++ b/controlplane/kubeadm/internal/etcd/etcd.go @@ -53,6 +53,7 @@ type Client struct { EtcdClient etcd Endpoint string LeaderID uint64 + Errors []string } // MemberAlarm represents an alarm type association with a cluster member. @@ -77,6 +78,13 @@ const ( AlarmCorrupt ) +// AlarmTypeName provides a text translation for AlarmType codes. +var AlarmTypeName = map[AlarmType]string{ + AlarmOk: "NONE", + AlarmNoSpace: "NOSPACE", + AlarmCorrupt: "CORRUPT", +} + // Adapted from kubeadm // Member struct defines an etcd member; it is used to avoid spreading @@ -154,6 +162,7 @@ func newEtcdClient(ctx context.Context, etcdClient etcd) (*Client, error) { Endpoint: endpoints[0], EtcdClient: etcdClient, LeaderID: status.Leader, + Errors: status.Errors, }, nil } diff --git a/controlplane/kubeadm/internal/etcd/util/set.go b/controlplane/kubeadm/internal/etcd/util/set.go deleted file mode 100644 index c5d941e93153..000000000000 --- a/controlplane/kubeadm/internal/etcd/util/set.go +++ /dev/null @@ -1,201 +0,0 @@ -/* -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. -*/ - -// Modified copy of k8s.io/apimachinery/pkg/util/sets/int64.go -// Modifications -// - int64 became uint64 -// - Empty type is added to this file and changed to a non exported symbol - -package util - -import ( - "sort" -) - -type empty struct{} - -// util.UInt64Set is a set of uint64s, implemented via map[uint64]struct{} for minimal memory consumption. -type UInt64Set map[uint64]empty - -// NewUInt64Set creates a UInt64Set from a list of values. -func NewUInt64Set(items ...uint64) UInt64Set { - ss := UInt64Set{} - ss.Insert(items...) - return ss -} - -// Insert adds items to the set. -func (s UInt64Set) Insert(items ...uint64) UInt64Set { - for _, item := range items { - s[item] = empty{} - } - return s -} - -// Delete removes all items from the set. -func (s UInt64Set) Delete(items ...uint64) UInt64Set { - for _, item := range items { - delete(s, item) - } - return s -} - -// Has returns true if and only if item is contained in the set. -func (s UInt64Set) Has(item uint64) bool { - _, contained := s[item] - return contained -} - -// HasAll returns true if and only if all items are contained in the set. -func (s UInt64Set) HasAll(items ...uint64) bool { - for _, item := range items { - if !s.Has(item) { - return false - } - } - return true -} - -// HasAny returns true if any items are contained in the set. -func (s UInt64Set) HasAny(items ...uint64) bool { - for _, item := range items { - if s.Has(item) { - return true - } - } - return false -} - -// Difference returns a set of objects that are not in s2 -// For example: -// s1 = {a1, a2, a3} -// s2 = {a1, a2, a4, a5} -// s1.Difference(s2) = {a3} -// s2.Difference(s1) = {a4, a5} -func (s UInt64Set) Difference(s2 UInt64Set) UInt64Set { - result := NewUInt64Set() - for key := range s { - if !s2.Has(key) { - result.Insert(key) - } - } - return result -} - -// Union returns a new set which includes items in either s1 or s2. -// For example: -// s1 = {a1, a2} -// s2 = {a3, a4} -// s1.Union(s2) = {a1, a2, a3, a4} -// s2.Union(s1) = {a1, a2, a3, a4} -func (s UInt64Set) Union(s2 UInt64Set) UInt64Set { - s1 := s - result := NewUInt64Set() - for key := range s1 { - result.Insert(key) - } - for key := range s2 { - result.Insert(key) - } - return result -} - -// Intersection returns a new set which includes the item in BOTH s1 and s2 -// For example: -// s1 = {a1, a2} -// s2 = {a2, a3} -// s1.Intersection(s2) = {a2} -func (s UInt64Set) Intersection(s2 UInt64Set) UInt64Set { - s1 := s - var walk, other UInt64Set - result := NewUInt64Set() - if s1.Len() < s2.Len() { - walk = s1 - other = s2 - } else { - walk = s2 - other = s1 - } - for key := range walk { - if other.Has(key) { - result.Insert(key) - } - } - return result -} - -// IsSuperset returns true if and only if s1 is a superset of s2. -func (s UInt64Set) IsSuperset(s2 UInt64Set) bool { - s1 := s - for item := range s2 { - if !s1.Has(item) { - return false - } - } - return true -} - -// Equal returns true if and only if s1 is equal (as a set) to s2. -// Two sets are equal if their membership is identical. -// (In practice, this means same elements, order doesn't matter) -func (s UInt64Set) Equal(s2 UInt64Set) bool { - s1 := s - return len(s1) == len(s2) && s1.IsSuperset(s2) -} - -type sortableSliceOfUInt64 []uint64 - -func (s sortableSliceOfUInt64) Len() int { return len(s) } -func (s sortableSliceOfUInt64) Less(i, j int) bool { return lessUInt64(s[i], s[j]) } -func (s sortableSliceOfUInt64) Swap(i, j int) { s[i], s[j] = s[j], s[i] } - -// List returns the contents as a sorted uint64 slice. -func (s UInt64Set) List() []uint64 { - res := make(sortableSliceOfUInt64, 0, len(s)) - for key := range s { - res = append(res, key) - } - sort.Sort(res) - return []uint64(res) -} - -// UnsortedList returns the slice with contents in random order. -func (s UInt64Set) UnsortedList() []uint64 { - res := make([]uint64, 0, len(s)) - for key := range s { - res = append(res, key) - } - return res -} - -// Returns a single element from the set. -func (s UInt64Set) PopAny() (uint64, bool) { - for key := range s { - s.Delete(key) - return key, true - } - var zeroValue uint64 - return zeroValue, false -} - -// Len returns the size of the set. -func (s UInt64Set) Len() int { - return len(s) -} - -func lessUInt64(lhs, rhs uint64) bool { - return lhs < rhs -} diff --git a/controlplane/kubeadm/internal/etcd/util/util.go b/controlplane/kubeadm/internal/etcd/util/util.go index eb3bcf39c927..9876aa7eac44 100644 --- a/controlplane/kubeadm/internal/etcd/util/util.go +++ b/controlplane/kubeadm/internal/etcd/util/util.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd" ) @@ -30,11 +31,16 @@ func MemberForName(members []*etcd.Member, name string) *etcd.Member { return nil } -// MemberIDSet returns a set of member IDs. -func MemberIDSet(members []*etcd.Member) UInt64Set { - set := UInt64Set{} +func MemberNames(members []*etcd.Member) []string { + names := make([]string, 0, len(members)) for _, m := range members { - set.Insert(m.ID) + names = append(names, m.Name) } - return set + return names +} + +func MemberEqual(members1, members2 []*etcd.Member) bool { + names1 := sets.NewString(MemberNames(members1)...) + names2 := sets.NewString(MemberNames(members2)...) + return names1.Equal(names2) } diff --git a/controlplane/kubeadm/internal/workload_cluster.go b/controlplane/kubeadm/internal/workload_cluster.go index 825cf7e13a76..d0ade2b14ec9 100644 --- a/controlplane/kubeadm/internal/workload_cluster.go +++ b/controlplane/kubeadm/internal/workload_cluster.go @@ -34,7 +34,6 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kerrors "k8s.io/apimachinery/pkg/util/errors" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" "sigs.k8s.io/cluster-api/util" @@ -58,8 +57,8 @@ var ( type WorkloadCluster interface { // Basic health and status checks. ClusterStatus(ctx context.Context) (ClusterStatus, error) - ControlPlaneIsHealthy(ctx context.Context) (HealthCheckResult, error) - EtcdIsHealthy(ctx context.Context) (HealthCheckResult, error) + UpdateStaticPodConditions(ctx context.Context, controlPlane *ControlPlane) + UpdateEtcdConditions(ctx context.Context, controlPlane *ControlPlane) // Upgrade related tasks. ReconcileKubeletRBACBinding(ctx context.Context, version semver.Version) error @@ -77,7 +76,7 @@ type WorkloadCluster interface { AllowBootstrapTokensToGetNodes(ctx context.Context) error // State recovery tasks. - ReconcileEtcdMembers(ctx context.Context) error + ReconcileEtcdMembers(ctx context.Context) ([]string, error) } // Workload defines operations on workload clusters. @@ -107,88 +106,6 @@ func (w *Workload) getConfigMap(ctx context.Context, configMap ctrlclient.Object return original.DeepCopy(), nil } -// HealthCheckResult maps nodes that are checked to any errors the node has related to the check. -type HealthCheckResult map[string]error - -// Aggregate will analyse HealthCheckResult and report any errors discovered. -// It also ensures there is a 1;1 match between nodes and machines. -func (h HealthCheckResult) Aggregate(controlPlane *ControlPlane) error { - var errorList []error - kcpMachines := controlPlane.Machines.UnsortedList() - // Make sure Cluster API is aware of all the nodes. - - for nodeName, err := range h { - if err != nil { - errorList = append(errorList, fmt.Errorf("node %q: %v", nodeName, err)) - } - } - if len(errorList) != 0 { - return kerrors.NewAggregate(errorList) - } - - // This check ensures there is a 1 to 1 correspondence of nodes and machines. - // If a machine was not checked this is considered an error. - for _, machine := range kcpMachines { - if machine.Status.NodeRef == nil { - // The condition for this case is set by the Machine controller - return errors.Errorf("control plane machine %s/%s has no status.nodeRef", machine.Namespace, machine.Name) - } - if _, ok := h[machine.Status.NodeRef.Name]; !ok { - return errors.Errorf("machine's (%s/%s) node (%s) was not checked", machine.Namespace, machine.Name, machine.Status.NodeRef.Name) - } - } - if len(h) != len(kcpMachines) { - // MachinesReadyCondition covers this health failure. - return errors.Errorf("number of nodes and machines in namespace %s did not match: %d nodes %d machines", controlPlane.Cluster.Namespace, len(h), len(kcpMachines)) - } - return nil -} - -// controlPlaneIsHealthy does a best effort check of the control plane components the kubeadm control plane cares about. -// The return map is a map of node names as keys to error that that node encountered. -// All nodes will exist in the map with nil errors if there were no errors for that node. -func (w *Workload) ControlPlaneIsHealthy(ctx context.Context) (HealthCheckResult, error) { - controlPlaneNodes, err := w.getControlPlaneNodes(ctx) - if err != nil { - return nil, err - } - - response := make(map[string]error) - for _, node := range controlPlaneNodes.Items { - name := node.Name - response[name] = nil - - if err := checkNodeNoExecuteCondition(node); err != nil { - response[name] = err - continue - } - - apiServerPodKey := ctrlclient.ObjectKey{ - Namespace: metav1.NamespaceSystem, - Name: staticPodName("kube-apiserver", name), - } - apiServerPod := corev1.Pod{} - if err := w.Client.Get(ctx, apiServerPodKey, &apiServerPod); err != nil { - response[name] = err - continue - } - response[name] = checkStaticPodReadyCondition(apiServerPod) - - controllerManagerPodKey := ctrlclient.ObjectKey{ - Namespace: metav1.NamespaceSystem, - Name: staticPodName("kube-controller-manager", name), - } - controllerManagerPod := corev1.Pod{} - if err := w.Client.Get(ctx, controllerManagerPodKey, &controllerManagerPod); err != nil { - response[name] = err - continue - } - response[name] = checkStaticPodReadyCondition(controllerManagerPod) - } - - return response, nil -} - // UpdateKubernetesVersionInKubeadmConfigMap updates the kubernetes version in the kubeadm config map. func (w *Workload) UpdateImageRepositoryInKubeadmConfigMap(ctx context.Context, imageRepository string) error { configMapKey := ctrlclient.ObjectKey{Name: "kubeadm-config", Namespace: metav1.NamespaceSystem} @@ -383,31 +300,6 @@ func staticPodName(component, nodeName string) string { return fmt.Sprintf("%s-%s", component, nodeName) } -func checkStaticPodReadyCondition(pod corev1.Pod) error { - found := false - for _, condition := range pod.Status.Conditions { - if condition.Type == corev1.PodReady { - found = true - } - if condition.Type == corev1.PodReady && condition.Status != corev1.ConditionTrue { - return errors.Errorf("static pod %s/%s is not ready", pod.Namespace, pod.Name) - } - } - if !found { - return errors.Errorf("pod does not have ready condition: %v", pod.Name) - } - return nil -} - -func checkNodeNoExecuteCondition(node corev1.Node) error { - for _, taint := range node.Spec.Taints { - if taint.Key == corev1.TaintNodeUnreachable && taint.Effect == corev1.TaintEffectNoExecute { - return errors.Errorf("node has NoExecute taint: %v", node.Name) - } - } - return nil -} - // UpdateKubeProxyImageInfo updates kube-proxy image in the kube-proxy DaemonSet. func (w *Workload) UpdateKubeProxyImageInfo(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane) error { // Return early if we've been asked to skip kube-proxy upgrades entirely. diff --git a/controlplane/kubeadm/internal/workload_cluster_conditions.go b/controlplane/kubeadm/internal/workload_cluster_conditions.go new file mode 100644 index 000000000000..ee5e4753d6ea --- /dev/null +++ b/controlplane/kubeadm/internal/workload_cluster_conditions.go @@ -0,0 +1,553 @@ +/* +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 internal + +import ( + "context" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + 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/etcd" + etcdutil "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd/util" + "sigs.k8s.io/cluster-api/util/conditions" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +// UpdateEtcdConditions is responsible for updating machine conditions reflecting the status of all the etcd members. +// This operation is best effort, in the sense that in case of problems in retrieving member status, it sets +// the condition to Unknown state without returning any error. +func (w *Workload) UpdateEtcdConditions(ctx context.Context, controlPlane *ControlPlane) { + if controlPlane.IsEtcdManaged() { + w.updateManagedEtcdConditions(ctx, controlPlane) + return + } + w.updateExternalEtcdConditions(ctx, controlPlane) +} + +func (w *Workload) updateExternalEtcdConditions(ctx context.Context, controlPlane *ControlPlane) { //nolint:unparam + // When KCP is not responsible for external etcd, we are reporting only health at KCP level. + conditions.MarkTrue(controlPlane.KCP, controlplanev1.EtcdClusterHealthyCondition) + + // TODO: check external etcd for alarms an possibly also for member errors + // this requires implementing an new type of etcd client generator given that it is not possible to use nodes + // as a source for the etcd endpoint address; the address of the external etcd should be available on the kubeadm configuration. +} + +func (w *Workload) updateManagedEtcdConditions(ctx context.Context, controlPlane *ControlPlane) { + // NOTE: This methods uses control plane nodes only to get in contact with etcd but then it relies on etcd + // as ultimate source of truth for the list of members and for their health. + controlPlaneNodes, err := w.getControlPlaneNodes(ctx) + if err != nil { + conditions.MarkUnknown(controlPlane.KCP, controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterInspectionFailedReason, "Failed to list nodes which are hosting the etcd members") + for _, m := range controlPlane.Machines { + conditions.MarkUnknown(m, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberInspectionFailedReason, "Failed to get the node which is hosting the etcd member") + } + return + } + + // Update conditions for etcd members on the nodes. + var ( + // kcpErrors is used to store errors that can't be reported on any machine. + kcpErrors []string + // clusterID is used to store and compare the etcd's cluster id. + clusterID *uint64 + // members is used to store the list of etcd members and compare with all the other nodes in the cluster. + members []*etcd.Member + ) + + for _, node := range controlPlaneNodes.Items { + // Search for the machine corresponding to the node. + var machine *clusterv1.Machine + for _, m := range controlPlane.Machines { + if m.Status.NodeRef != nil && m.Status.NodeRef.Name == node.Name { + machine = m + } + } + + if machine == nil { + // If there are machines still provisioning there is the chance that a chance that a node might be linked to a machine soon, + // otherwise report the error at KCP level given that there is no machine to report on. + if hasProvisioningMachine(controlPlane.Machines) { + continue + } + kcpErrors = append(kcpErrors, fmt.Sprintf("Control plane node %s does not have a corresponding machine", node.Name)) + continue + } + + // If the machine is deleting, report all the conditions as deleting + if !machine.ObjectMeta.DeletionTimestamp.IsZero() { + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "") + continue + } + + // Create the etcd Client for the etcd Pod scheduled on the Node + etcdClient, err := w.etcdClientGenerator.forNodes(ctx, []corev1.Node{node}) + if err != nil { + conditions.MarkUnknown(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberInspectionFailedReason, "Failed to connect to the etcd pod on the %s node", node.Name) + continue + } + defer etcdClient.Close() + + // While creating a new client, forNodes retrieves the status for the endpoint; check if the endpoint has errors. + if len(etcdClient.Errors) > 0 { + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Etcd member status reports errors: %s", strings.Join(etcdClient.Errors, ", ")) + continue + } + + // Gets the list etcd members known by this member. + currentMembers, err := etcdClient.Members(ctx) + if err != nil { + // NB. We should never be in here, given that we just received answer to the etcd calls included in forNodes; + // however, we are considering the calls to Members a signal of etcd not being stable. + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Failed get answer from the etcd member on the %s node", node.Name) + continue + } + + // Check if the list of members IDs reported is the same as all other members. + // NOTE: the first member reporting this information is the baseline for this information. + if members == nil { + members = currentMembers + } + if !etcdutil.MemberEqual(members, currentMembers) { + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "etcd member reports the cluster is composed by members %s, but all previously seen etcd members are reporting %s", etcdutil.MemberNames(currentMembers), etcdutil.MemberNames(members)) + continue + } + + // Retrieve the member and check for alarms. + // NB. The member for this node always exists given forNodes(node) used above + member := etcdutil.MemberForName(currentMembers, node.Name) + if len(member.Alarms) > 0 { + alarmList := []string{} + for _, alarm := range member.Alarms { + switch alarm { + case etcd.AlarmOk: + continue + default: + alarmList = append(alarmList, etcd.AlarmTypeName[alarm]) + } + } + if len(alarmList) > 0 { + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Etcd member reports alarms: %s", strings.Join(alarmList, ", ")) + continue + } + } + + // Check if the member belongs to the same cluster as all other members. + // NOTE: the first member reporting this information is the baseline for this information. + if clusterID == nil { + clusterID = &member.ClusterID + } + if *clusterID != member.ClusterID { + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "etcd member has cluster ID %d, but all previously seen etcd members have cluster ID %d", member.ClusterID, *clusterID) + continue + } + + conditions.MarkTrue(machine, controlplanev1.MachineEtcdMemberHealthyCondition) + } + + // Make sure that the list of etcd members and machines is consistent. + kcpErrors = compareMachinesAndMembers(controlPlane, members, kcpErrors) + + // Aggregate components error from machines at KCP level + aggregateFromMachinesToKCP(aggregateFromMachinesToKCPInput{ + controlPlane: controlPlane, + machineConditions: []clusterv1.ConditionType{controlplanev1.MachineEtcdMemberHealthyCondition}, + kcpErrors: kcpErrors, + condition: controlplanev1.EtcdClusterHealthyCondition, + unhealthyReason: controlplanev1.EtcdClusterUnhealthyReason, + unknownReason: controlplanev1.EtcdClusterUnknownReason, + note: "etcd member", + }) +} + +func compareMachinesAndMembers(controlPlane *ControlPlane, members []*etcd.Member, kcpErrors []string) []string { + // NOTE: We run this check only if we actually know the list of members, otherwise the first for loop + // could generate a false negative when reporting missing etcd members. + if members == nil { + return kcpErrors + } + + // Check Machine -> Etcd member. + for _, machine := range controlPlane.Machines { + if machine.Status.NodeRef == nil { + continue + } + found := false + for _, member := range members { + if machine.Status.NodeRef.Name == member.Name { + found = true + break + } + } + if !found { + conditions.MarkFalse(machine, controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Missing etcd member") + } + } + + // Check Etcd member -> Machine. + for _, member := range members { + found := false + for _, machine := range controlPlane.Machines { + if machine.Status.NodeRef != nil && machine.Status.NodeRef.Name == member.Name { + found = true + break + } + } + if !found { + name := member.Name + if name == "" { + name = fmt.Sprintf("%d (Name not yet assigned)", member.ID) + } + kcpErrors = append(kcpErrors, fmt.Sprintf("etcd member %s does not have a corresponding machine", name)) + } + } + return kcpErrors +} + +// UpdateStaticPodConditions is responsible for updating machine conditions reflecting the status of all the control plane +// components running in a static pod generated by kubeadm. This operation is best effort, in the sense that in case +// of problems in retrieving the pod status, it sets the condition to Unknown state without returning any error. +func (w *Workload) UpdateStaticPodConditions(ctx context.Context, controlPlane *ControlPlane) { + allMachinePodConditions := []clusterv1.ConditionType{ + controlplanev1.MachineAPIServerPodHealthyCondition, + controlplanev1.MachineControllerManagerPodHealthyCondition, + controlplanev1.MachineSchedulerPodHealthyCondition, + } + if controlPlane.IsEtcdManaged() { + allMachinePodConditions = append(allMachinePodConditions, controlplanev1.MachineEtcdPodHealthyCondition) + } + + // NOTE: this fun uses control plane nodes from the workload cluster as a source of truth for the current state. + controlPlaneNodes, err := w.getControlPlaneNodes(ctx) + if err != nil { + for i := range controlPlane.Machines { + machine := controlPlane.Machines[i] + for _, condition := range allMachinePodConditions { + conditions.MarkUnknown(machine, condition, controlplanev1.PodInspectionFailedReason, "Failed to get the node which is hosting this component") + } + } + conditions.MarkUnknown(controlPlane.KCP, controlplanev1.ControlPlaneComponentsHealthyCondition, controlplanev1.ControlPlaneComponentsInspectionFailedReason, "Failed to list nodes which are hosting control plane components") + return + } + + // Update conditions for control plane components hosted as static pods on the nodes. + var kcpErrors []string + + for _, node := range controlPlaneNodes.Items { + // Search for the machine corresponding to the node. + var machine *clusterv1.Machine + for _, m := range controlPlane.Machines { + if m.Status.NodeRef != nil && m.Status.NodeRef.Name == node.Name { + machine = m + break + } + } + + // If there is no machine corresponding to a node, determine if this is an error or not. + if machine == nil { + // If there are machines still provisioning there is the chance that a chance that a node might be linked to a machine soon, + // otherwise report the error at KCP level given that there is no machine to report on. + if hasProvisioningMachine(controlPlane.Machines) { + continue + } + kcpErrors = append(kcpErrors, fmt.Sprintf("Control plane node %s does not have a corresponding machine", node.Name)) + continue + } + + // If the machine is deleting, report all the conditions as deleting + if !machine.ObjectMeta.DeletionTimestamp.IsZero() { + for _, condition := range allMachinePodConditions { + conditions.MarkFalse(machine, condition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "") + } + continue + } + + // If the node is Unreachable, information about static pods could be stale so set all conditions to unknown. + if nodeHasUnreachableTaint(node) { + // NOTE: We are assuming unreachable as a temporary condition, leaving to MHC + // the responsibility to determine if the node is unhealthy or not. + for _, condition := range allMachinePodConditions { + conditions.MarkUnknown(machine, condition, controlplanev1.PodInspectionFailedReason, "Node is unreachable") + } + continue + } + + // Otherwise updates static pod based conditions reflecting the status of the underlying object generated by kubeadm. + w.updateStaticPodCondition(ctx, machine, node, "kube-apiserver", controlplanev1.MachineAPIServerPodHealthyCondition) + w.updateStaticPodCondition(ctx, machine, node, "kube-controller-manager", controlplanev1.MachineControllerManagerPodHealthyCondition) + w.updateStaticPodCondition(ctx, machine, node, "kube-scheduler", controlplanev1.MachineSchedulerPodHealthyCondition) + if controlPlane.IsEtcdManaged() { + w.updateStaticPodCondition(ctx, machine, node, "etcd", controlplanev1.MachineEtcdPodHealthyCondition) + } + } + + // If there are provisioned machines without corresponding nodes, report this as a failing conditions with SeverityError. + for i := range controlPlane.Machines { + machine := controlPlane.Machines[i] + if machine.Status.NodeRef == nil { + continue + } + found := false + for _, node := range controlPlaneNodes.Items { + if machine.Status.NodeRef.Name == node.Name { + found = true + break + } + } + if !found { + for _, condition := range allMachinePodConditions { + conditions.MarkFalse(machine, condition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "Missing node") + } + } + } + + // Aggregate components error from machines at KCP level. + aggregateFromMachinesToKCP(aggregateFromMachinesToKCPInput{ + controlPlane: controlPlane, + machineConditions: allMachinePodConditions, + kcpErrors: kcpErrors, + condition: controlplanev1.ControlPlaneComponentsHealthyCondition, + unhealthyReason: controlplanev1.ControlPlaneComponentsUnhealthyReason, + unknownReason: controlplanev1.ControlPlaneComponentsUnknownReason, + note: "control plane", + }) +} + +func hasProvisioningMachine(machines FilterableMachineCollection) bool { + for _, machine := range machines { + if machine.Status.NodeRef == nil { + return true + } + } + return false +} + +// nodeHasUnreachableTaint returns true if the node has is unreachable from the node controller. +func nodeHasUnreachableTaint(node corev1.Node) bool { + for _, taint := range node.Spec.Taints { + if taint.Key == corev1.TaintNodeUnreachable && taint.Effect == corev1.TaintEffectNoExecute { + return true + } + } + return false +} + +// updateStaticPodCondition is responsible for updating machine conditions reflecting the status of a component running +// in a static pod generated by kubeadm. This operation is best effort, in the sense that in case of problems +// in retrieving the pod status, it sets the condition to Unknown state without returning any error. +func (w *Workload) updateStaticPodCondition(ctx context.Context, machine *clusterv1.Machine, node corev1.Node, component string, staticPodCondition clusterv1.ConditionType) { + podKey := ctrlclient.ObjectKey{ + Namespace: metav1.NamespaceSystem, + Name: staticPodName(component, node.Name), + } + + pod := corev1.Pod{} + if err := w.Client.Get(ctx, podKey, &pod); err != nil { + // If there is an error getting the Pod, do not set any conditions. + if apierrors.IsNotFound(err) { + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodMissingReason, clusterv1.ConditionSeverityError, "Pod %s is missing", podKey.Name) + return + } + conditions.MarkUnknown(machine, staticPodCondition, controlplanev1.PodInspectionFailedReason, "Failed to get pod status") + return + } + + switch pod.Status.Phase { + case corev1.PodPending: + // PodPending means the pod has been accepted by the system, but one or more of the containers + // has not been started. This logic is trying to surface more details about what is happening in this phase. + + // Check if the container is still to be scheduled + // NOTE: This should never happen for static pods, however this check is implemented for completeness. + if podCondition(pod, corev1.PodScheduled) != corev1.ConditionTrue { + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Waiting to be scheduled") + return + } + + // Check if the container is still running init containers + // NOTE: As of today there are not init containers in static pods generated by kubeadm, however this check is implemented for completeness. + if podCondition(pod, corev1.PodInitialized) != corev1.ConditionTrue { + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Running init containers") + return + } + + // If there are no error from containers, report provisioning without further details. + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "") + case corev1.PodRunning: + // PodRunning means the pod has been bound to a node and all of the containers have been started. + // At least one container is still running or is in the process of being restarted. + // This logic is trying to determine if we are actually running or if we are in an intermediate state + // like e.g. a container is retarted. + + // PodReady condition means the pod is able to service requests + if podCondition(pod, corev1.PodReady) == corev1.ConditionTrue { + conditions.MarkTrue(machine, staticPodCondition) + return + } + + // Surface wait message from containers. + // Exception: Since default "restartPolicy" = "Always", a container that exited with error will be in waiting state (not terminated state) + // with "CrashLoopBackOff" reason and its LastTerminationState will be non-nil. + var containerWaitingMessages []string + terminatedWithError := false + for _, containerStatus := range pod.Status.ContainerStatuses { + if containerStatus.LastTerminationState.Terminated != nil && containerStatus.LastTerminationState.Terminated.ExitCode != 0 { + terminatedWithError = true + } + if containerStatus.State.Waiting != nil { + containerWaitingMessages = append(containerWaitingMessages, containerStatus.State.Waiting.Reason) + } + } + if len(containerWaitingMessages) > 0 { + if terminatedWithError { + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, strings.Join(containerWaitingMessages, ", ")) + return + } + // Note: Some error cases cannot be caught when container state == "Waiting", + // e.g., "waiting.reason: ErrImagePull" is an error, but since LastTerminationState does not exist, this cannot be differentiated from "PodProvisioningReason" + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, strings.Join(containerWaitingMessages, ", ")) + return + } + + // Surface errors message from containers. + var containerTerminatedMessages []string + for _, containerStatus := range pod.Status.ContainerStatuses { + if containerStatus.State.Terminated != nil { + containerTerminatedMessages = append(containerTerminatedMessages, containerStatus.State.Terminated.Reason) + } + } + if len(containerTerminatedMessages) > 0 { + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, strings.Join(containerTerminatedMessages, ", ")) + return + } + + // If the pod is not yet ready, most probably it is waiting for startup or readiness probes. + // Report this as part of the provisioning process because the corresponding control plane component is not ready yet. + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Waiting for startup or readiness probes") + case corev1.PodSucceeded: + // PodSucceeded means that all containers in the pod have voluntarily terminated + // with a container exit code of 0, and the system is not going to restart any of these containers. + // NOTE: This should never happen for the static pods running control plane components. + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "All the containers have been terminated") + case corev1.PodFailed: + // PodFailed means that all containers in the pod have terminated, and at least one container has + // terminated in a failure (exited with a non-zero exit code or was stopped by the system). + // NOTE: This should never happen for the static pods running control plane components. + conditions.MarkFalse(machine, staticPodCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "All the containers have been terminated") + case corev1.PodUnknown: + // PodUnknown means that for some reason the state of the pod could not be obtained, typically due + // to an error in communicating with the host of the pod. + conditions.MarkUnknown(machine, staticPodCondition, controlplanev1.PodInspectionFailedReason, "Pod is reporting unknown status") + } +} + +func podCondition(pod corev1.Pod, condition corev1.PodConditionType) corev1.ConditionStatus { + for _, c := range pod.Status.Conditions { + if c.Type == condition { + return c.Status + } + } + return corev1.ConditionUnknown +} + +type aggregateFromMachinesToKCPInput struct { + controlPlane *ControlPlane + machineConditions []clusterv1.ConditionType + kcpErrors []string + condition clusterv1.ConditionType + unhealthyReason string + unknownReason string + note string +} + +// aggregateFromMachinesToKCP aggregates a group of conditions from machines to KCP. +// NOTE: this func follows the same aggregation rules used by conditions.Merge thus giving priority to +// errors, then warning, info down to unknown. +func aggregateFromMachinesToKCP(input aggregateFromMachinesToKCPInput) { + // Aggregates machines for condition status. + // NB. A machine could be assigned to many groups, but only the group with the highest severity will be reported. + kcpMachinesWithErrors := sets.NewString() + kcpMachinesWithWarnings := sets.NewString() + kcpMachinesWithInfo := sets.NewString() + kcpMachinesWithTrue := sets.NewString() + kcpMachinesWithUnknown := sets.NewString() + + for i := range input.controlPlane.Machines { + machine := input.controlPlane.Machines[i] + for _, condition := range input.machineConditions { + if machineCondition := conditions.Get(machine, condition); machineCondition != nil { + switch machineCondition.Status { + case corev1.ConditionTrue: + kcpMachinesWithTrue.Insert(machine.Name) + case corev1.ConditionFalse: + switch machineCondition.Severity { + case clusterv1.ConditionSeverityInfo: + kcpMachinesWithInfo.Insert(machine.Name) + case clusterv1.ConditionSeverityWarning: + kcpMachinesWithWarnings.Insert(machine.Name) + case clusterv1.ConditionSeverityError: + kcpMachinesWithErrors.Insert(machine.Name) + } + case corev1.ConditionUnknown: + kcpMachinesWithUnknown.Insert(machine.Name) + } + } + } + } + + // In case of at least one machine with errors or KCP level errors (nodes without machines), report false, error. + if len(kcpMachinesWithErrors) > 0 { + input.kcpErrors = append(input.kcpErrors, fmt.Sprintf("Following machines are reporting %s errors: %s", input.note, strings.Join(kcpMachinesWithErrors.List(), ", "))) + } + if len(input.kcpErrors) > 0 { + conditions.MarkFalse(input.controlPlane.KCP, input.condition, input.unhealthyReason, clusterv1.ConditionSeverityError, strings.Join(input.kcpErrors, "; ")) + return + } + + // In case of no errors and at least one machine with warnings, report false, warnings. + if len(kcpMachinesWithWarnings) > 0 { + conditions.MarkFalse(input.controlPlane.KCP, input.condition, input.unhealthyReason, clusterv1.ConditionSeverityWarning, "Following machines are reporting %s warnings: %s", input.note, strings.Join(kcpMachinesWithWarnings.List(), ", ")) + return + } + + // In case of no errors, no warning, and at least one machine with info, report false, info. + if len(kcpMachinesWithWarnings) > 0 { + conditions.MarkFalse(input.controlPlane.KCP, input.condition, input.unhealthyReason, clusterv1.ConditionSeverityWarning, "Following machines are reporting %s info: %s", input.note, strings.Join(kcpMachinesWithInfo.List(), ", ")) + return + } + + // In case of no errors, no warning, no Info, and at least one machine with true conditions, report true. + if len(kcpMachinesWithTrue) > 0 { + conditions.MarkTrue(input.controlPlane.KCP, input.condition) + return + } + + // Otherwise, if there is at least one machine with unknown, report unknown. + if len(kcpMachinesWithUnknown) > 0 { + conditions.MarkUnknown(input.controlPlane.KCP, input.condition, input.unknownReason, "Following machines are reporting unknown %s status: %s", input.note, strings.Join(kcpMachinesWithUnknown.List(), ", ")) + return + } + + // This last case should happen only if there are no provisioned machines, and thus without conditions. + // So there will be no condition at KCP level too. +} diff --git a/controlplane/kubeadm/internal/workload_cluster_conditions_test.go b/controlplane/kubeadm/internal/workload_cluster_conditions_test.go new file mode 100644 index 000000000000..c3986fb90928 --- /dev/null +++ b/controlplane/kubeadm/internal/workload_cluster_conditions_test.go @@ -0,0 +1,1042 @@ +/* +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 internal + +import ( + "testing" + + . "github.com/onsi/gomega" + "github.com/pkg/errors" + "go.etcd.io/etcd/clientv3" + pb "go.etcd.io/etcd/etcdserver/etcdserverpb" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3" + "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" + "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd" + fake2 "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd/fake" + "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestUpdateEtcdConditions(t *testing.T) { + tests := []struct { + name string + kcp *controlplanev1.KubeadmControlPlane + machines []*clusterv1.Machine + injectClient client.Client // This test is injecting a fake client because it is required to create nodes with a controlled Status or to fail with a specific error. + injectEtcdClientGenerator etcdClientFor // This test is injecting a fake etcdClientGenerator because it is required to nodes with a controlled Status or to fail with a specific error. + expectedKCPCondition *clusterv1.Condition + expectedMachineConditions map[string]clusterv1.Conditions + }{ + { + name: "if list nodes return an error should report all the conditions Unknown", + machines: []*clusterv1.Machine{ + fakeMachine("m1"), + }, + injectClient: &fakeClient{ + listErr: errors.New("failed to list nodes"), + }, + expectedKCPCondition: conditions.UnknownCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterInspectionFailedReason, "Failed to list nodes which are hosting the etcd members"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.UnknownCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberInspectionFailedReason, "Failed to get the node which is hosting the etcd member"), + }, + }, + }, + { + name: "node without machine should be ignored if there are provisioning machines", + machines: []*clusterv1.Machine{ + fakeMachine("m1"), // without NodeRef (provisioning) + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + expectedKCPCondition: nil, + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": {}, + }, + }, + { + name: "node without machine should report a problem at KCP level if there are no provisioning machines", + machines: []*clusterv1.Machine{}, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnhealthyReason, clusterv1.ConditionSeverityError, "Control plane node %s does not have a corresponding machine", "n1"), + }, + { + name: "failure creating the etcd client should report unknown condition", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesErr: errors.New("failed to get client for node"), + }, + expectedKCPCondition: conditions.UnknownCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnknownReason, "Following machines are reporting unknown etcd member status: m1"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.UnknownCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberInspectionFailedReason, "Failed to connect to the etcd pod on the %s node", "n1"), + }, + }, + }, + { + name: "etcd client reporting status errors should be reflected into a false condition", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesClient: &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + }, + Errors: []string{"some errors"}, + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting etcd member errors: %s", "m1"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.FalseCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Etcd member status reports errors: %s", "some errors"), + }, + }, + }, + { + name: "failure listing members should report false condition", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesClient: &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + ErrorResponse: errors.New("failed to list members"), + }, + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting etcd member errors: %s", "m1"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.FalseCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Failed get answer from the etcd member on the %s node", "n1"), + }, + }, + }, + { + name: "an etcd member with alarms should report false condition", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesClient: &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Members: []*pb.Member{ + {Name: "n1", ID: uint64(1)}, + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{ + {MemberID: uint64(1), Alarm: 1}, // NOSPACE + }, + }, + }, + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting etcd member errors: %s", "m1"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.FalseCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Etcd member reports alarms: %s", "NOSPACE"), + }, + }, + }, + { + name: "etcd members with different Cluster ID should report false condition", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + fakeMachine("m2", withNodeRef("n2")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{ + *fakeNode("n1"), + *fakeNode("n2"), + }, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesClientFunc: func(n []corev1.Node) (*etcd.Client, error) { + switch n[0].Name { + case "n1": + return &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Header: &pb.ResponseHeader{ + ClusterId: uint64(1), + }, + Members: []*pb.Member{ + {Name: "n1", ID: uint64(1)}, + {Name: "n2", ID: uint64(2)}, + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{}, + }, + }, + }, nil + case "n2": + return &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Header: &pb.ResponseHeader{ + ClusterId: uint64(2), // different Cluster ID + }, + Members: []*pb.Member{ + {Name: "n1", ID: uint64(1)}, + {Name: "n2", ID: uint64(2)}, + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{}, + }, + }, + }, nil + default: + return nil, errors.New("no client for this node") + } + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting etcd member errors: %s", "m2"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + "m2": { + *conditions.FalseCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "etcd member has cluster ID %d, but all previously seen etcd members have cluster ID %d", uint64(2), uint64(1)), + }, + }, + }, + { + name: "etcd members with different member list should report false condition", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + fakeMachine("m2", withNodeRef("n2")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{ + *fakeNode("n1"), + *fakeNode("n2"), + }, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesClientFunc: func(n []corev1.Node) (*etcd.Client, error) { + switch n[0].Name { + case "n1": + return &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Header: &pb.ResponseHeader{ + ClusterId: uint64(1), + }, + Members: []*pb.Member{ + {Name: "n1", ID: uint64(1)}, + {Name: "n2", ID: uint64(2)}, + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{}, + }, + }, + }, nil + case "n2": + return &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Header: &pb.ResponseHeader{ + ClusterId: uint64(1), + }, + Members: []*pb.Member{ // different member list + {Name: "n2", ID: uint64(2)}, + {Name: "n3", ID: uint64(3)}, + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{}, + }, + }, + }, nil + default: + return nil, errors.New("no client for this node") + } + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting etcd member errors: %s", "m2"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + "m2": { + *conditions.FalseCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "etcd member reports the cluster is composed by members [n2 n3], but all previously seen etcd members are reporting [n1 n2]"), + }, + }, + }, + { + name: "a machine without a member should report false condition", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + fakeMachine("m2", withNodeRef("n2")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{ + *fakeNode("n1"), + *fakeNode("n2"), + }, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesClientFunc: func(n []corev1.Node) (*etcd.Client, error) { + switch n[0].Name { + case "n1": + return &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Header: &pb.ResponseHeader{ + ClusterId: uint64(1), + }, + Members: []*pb.Member{ + {Name: "n1", ID: uint64(1)}, + // member n2 is missing + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{}, + }, + }, + }, nil + default: + return nil, errors.New("no client for this node") + } + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.EtcdClusterHealthyCondition, controlplanev1.EtcdClusterUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting etcd member errors: %s", "m2"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + "m2": { + *conditions.FalseCondition(controlplanev1.MachineEtcdMemberHealthyCondition, controlplanev1.EtcdMemberUnhealthyReason, clusterv1.ConditionSeverityError, "Missing etcd member"), + }, + }, + }, + { + name: "healthy etcd members should report true", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + fakeMachine("m2", withNodeRef("n2")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{ + *fakeNode("n1"), + *fakeNode("n2"), + }, + }, + }, + injectEtcdClientGenerator: &fakeEtcdClientGenerator{ + forNodesClientFunc: func(n []corev1.Node) (*etcd.Client, error) { + switch n[0].Name { + case "n1": + return &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Header: &pb.ResponseHeader{ + ClusterId: uint64(1), + }, + Members: []*pb.Member{ + {Name: "n1", ID: uint64(1)}, + {Name: "n2", ID: uint64(2)}, + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{}, + }, + }, + }, nil + case "n2": + return &etcd.Client{ + EtcdClient: &fake2.FakeEtcdClient{ + EtcdEndpoints: []string{}, + MemberListResponse: &clientv3.MemberListResponse{ + Header: &pb.ResponseHeader{ + ClusterId: uint64(1), + }, + Members: []*pb.Member{ + {Name: "n1", ID: uint64(1)}, + {Name: "n2", ID: uint64(2)}, + }, + }, + AlarmResponse: &clientv3.AlarmResponse{ + Alarms: []*pb.AlarmMember{}, + }, + }, + }, nil + default: + return nil, errors.New("no client for this node") + } + }, + }, + expectedKCPCondition: conditions.TrueCondition(controlplanev1.EtcdClusterHealthyCondition), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + "m2": { + *conditions.TrueCondition(controlplanev1.MachineEtcdMemberHealthyCondition), + }, + }, + }, + { + name: "Eternal etcd should set a condition at KCP level", + kcp: &controlplanev1.KubeadmControlPlane{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &v1beta1.ClusterConfiguration{ + Etcd: v1beta1.Etcd{ + External: &v1beta1.ExternalEtcd{}, + }, + }, + }, + }, + }, + expectedKCPCondition: conditions.TrueCondition(controlplanev1.EtcdClusterHealthyCondition), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + if tt.kcp == nil { + tt.kcp = &controlplanev1.KubeadmControlPlane{} + } + w := &Workload{ + Client: tt.injectClient, + etcdClientGenerator: tt.injectEtcdClientGenerator, + } + controlPane := &ControlPlane{ + KCP: tt.kcp, + Machines: NewFilterableMachineCollection(tt.machines...), + } + w.UpdateEtcdConditions(ctx, controlPane) + + if tt.expectedKCPCondition != nil { + g.Expect(*conditions.Get(tt.kcp, controlplanev1.EtcdClusterHealthyCondition)).To(conditions.MatchCondition(*tt.expectedKCPCondition)) + } + for _, m := range tt.machines { + g.Expect(tt.expectedMachineConditions).To(HaveKey(m.Name)) + g.Expect(m.GetConditions()).To(conditions.MatchConditions(tt.expectedMachineConditions[m.Name]), "unexpected conditions for machine %s", m.Name) + } + }) + } +} + +func TestUpdateStaticPodConditions(t *testing.T) { + n1APIServerPodName := staticPodName("kube-apiserver", "n1") + n1APIServerPodkey := client.ObjectKey{ + Namespace: metav1.NamespaceSystem, + Name: n1APIServerPodName, + }.String() + n1ControllerManagerPodName := staticPodName("kube-controller-manager", "n1") + n1ControllerManagerPodNKey := client.ObjectKey{ + Namespace: metav1.NamespaceSystem, + Name: n1ControllerManagerPodName, + }.String() + n1SchedulerPodName := staticPodName("kube-scheduler", "n1") + n1SchedulerPodKey := client.ObjectKey{ + Namespace: metav1.NamespaceSystem, + Name: n1SchedulerPodName, + }.String() + n1EtcdPodName := staticPodName("etcd", "n1") + n1EtcdPodKey := client.ObjectKey{ + Namespace: metav1.NamespaceSystem, + Name: n1EtcdPodName, + }.String() + tests := []struct { + name string + kcp *controlplanev1.KubeadmControlPlane + machines []*clusterv1.Machine + injectClient client.Client // This test is injecting a fake client because it is required to create nodes with a controlled Status or to fail with a specific error. + expectedKCPCondition *clusterv1.Condition + expectedMachineConditions map[string]clusterv1.Conditions + }{ + { + name: "if list nodes return an error, it should report all the conditions Unknown", + machines: []*clusterv1.Machine{ + fakeMachine("m1"), + }, + injectClient: &fakeClient{ + listErr: errors.New("failed to list nodes"), + }, + expectedKCPCondition: conditions.UnknownCondition(controlplanev1.ControlPlaneComponentsHealthyCondition, controlplanev1.ControlPlaneComponentsInspectionFailedReason, "Failed to list nodes which are hosting control plane components"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.UnknownCondition(controlplanev1.MachineAPIServerPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Failed to get the node which is hosting this component"), + *conditions.UnknownCondition(controlplanev1.MachineControllerManagerPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Failed to get the node which is hosting this component"), + *conditions.UnknownCondition(controlplanev1.MachineSchedulerPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Failed to get the node which is hosting this component"), + *conditions.UnknownCondition(controlplanev1.MachineEtcdPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Failed to get the node which is hosting this component"), + }, + }, + }, + { + name: "If there are provisioning machines, a node without machine should be ignored", + machines: []*clusterv1.Machine{ + fakeMachine("m1"), // without NodeRef (provisioning) + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + expectedKCPCondition: nil, + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": {}, + }, + }, + { + name: "If there are no provisioning machines, a node without machine should be reported as False condition at KCP level", + machines: []*clusterv1.Machine{}, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.ControlPlaneComponentsHealthyCondition, controlplanev1.ControlPlaneComponentsUnhealthyReason, clusterv1.ConditionSeverityError, "Control plane node %s does not have a corresponding machine", "n1"), + }, + { + name: "A node with unreachable taint should report all the conditions Unknown", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1", withUnreachableTaint())}, + }, + }, + expectedKCPCondition: conditions.UnknownCondition(controlplanev1.ControlPlaneComponentsHealthyCondition, controlplanev1.ControlPlaneComponentsUnknownReason, "Following machines are reporting unknown control plane status: m1"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.UnknownCondition(controlplanev1.MachineAPIServerPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Node is unreachable"), + *conditions.UnknownCondition(controlplanev1.MachineControllerManagerPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Node is unreachable"), + *conditions.UnknownCondition(controlplanev1.MachineSchedulerPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Node is unreachable"), + *conditions.UnknownCondition(controlplanev1.MachineEtcdPodHealthyCondition, controlplanev1.PodInspectionFailedReason, "Node is unreachable"), + }, + }, + }, + { + name: "A provisioning machine without node should be ignored", + machines: []*clusterv1.Machine{ + fakeMachine("m1"), // without NodeRef (provisioning) + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{}, + }, + expectedKCPCondition: nil, + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": {}, + }, + }, + { + name: "A provisioned machine without node should report all the conditions as false", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{}, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.ControlPlaneComponentsHealthyCondition, controlplanev1.ControlPlaneComponentsUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting control plane errors: %s", "m1"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.FalseCondition(controlplanev1.MachineAPIServerPodHealthyCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "Missing node"), + *conditions.FalseCondition(controlplanev1.MachineControllerManagerPodHealthyCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "Missing node"), + *conditions.FalseCondition(controlplanev1.MachineEtcdPodHealthyCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "Missing node"), + *conditions.FalseCondition(controlplanev1.MachineSchedulerPodHealthyCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "Missing node"), + }, + }, + }, + { + name: "Should surface control plane components errors", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + get: map[string]interface{}{ + n1APIServerPodkey: fakePod(n1APIServerPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + n1ControllerManagerPodNKey: fakePod(n1ControllerManagerPodName, + withPhase(corev1.PodPending), + withCondition(corev1.PodScheduled, corev1.ConditionFalse), + ), + n1SchedulerPodKey: fakePod(n1SchedulerPodName, + withPhase(corev1.PodFailed), + ), + n1EtcdPodKey: fakePod(n1EtcdPodName, + withPhase(corev1.PodSucceeded), + ), + }, + }, + expectedKCPCondition: conditions.FalseCondition(controlplanev1.ControlPlaneComponentsHealthyCondition, controlplanev1.ControlPlaneComponentsUnhealthyReason, clusterv1.ConditionSeverityError, "Following machines are reporting control plane errors: %s", "m1"), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.TrueCondition(controlplanev1.MachineAPIServerPodHealthyCondition), + *conditions.FalseCondition(controlplanev1.MachineControllerManagerPodHealthyCondition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Waiting to be scheduled"), + *conditions.FalseCondition(controlplanev1.MachineSchedulerPodHealthyCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "All the containers have been terminated"), + *conditions.FalseCondition(controlplanev1.MachineEtcdPodHealthyCondition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "All the containers have been terminated"), + }, + }, + }, + { + name: "Should surface control plane components health", + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + get: map[string]interface{}{ + n1APIServerPodkey: fakePod(n1APIServerPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + n1ControllerManagerPodNKey: fakePod(n1ControllerManagerPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + n1SchedulerPodKey: fakePod(n1SchedulerPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + n1EtcdPodKey: fakePod(n1EtcdPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + }, + }, + expectedKCPCondition: conditions.TrueCondition(controlplanev1.ControlPlaneComponentsHealthyCondition), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.TrueCondition(controlplanev1.MachineAPIServerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineControllerManagerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineSchedulerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineEtcdPodHealthyCondition), + }, + }, + }, + { + name: "Should surface control plane components health with eternal etcd", + kcp: &controlplanev1.KubeadmControlPlane{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &v1beta1.ClusterConfiguration{ + Etcd: v1beta1.Etcd{ + External: &v1beta1.ExternalEtcd{}, + }, + }, + }, + }, + }, + machines: []*clusterv1.Machine{ + fakeMachine("m1", withNodeRef("n1")), + }, + injectClient: &fakeClient{ + list: &corev1.NodeList{ + Items: []corev1.Node{*fakeNode("n1")}, + }, + get: map[string]interface{}{ + n1APIServerPodkey: fakePod(n1APIServerPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + n1ControllerManagerPodNKey: fakePod(n1ControllerManagerPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + n1SchedulerPodKey: fakePod(n1SchedulerPodName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + // no static pod for etcd + }, + }, + expectedKCPCondition: conditions.TrueCondition(controlplanev1.ControlPlaneComponentsHealthyCondition), + expectedMachineConditions: map[string]clusterv1.Conditions{ + "m1": { + *conditions.TrueCondition(controlplanev1.MachineAPIServerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineControllerManagerPodHealthyCondition), + *conditions.TrueCondition(controlplanev1.MachineSchedulerPodHealthyCondition), + // no condition for etcd Pod + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + if tt.kcp == nil { + tt.kcp = &controlplanev1.KubeadmControlPlane{} + } + w := &Workload{ + Client: tt.injectClient, + } + controlPane := &ControlPlane{ + KCP: tt.kcp, + Machines: NewFilterableMachineCollection(tt.machines...), + } + w.UpdateStaticPodConditions(ctx, controlPane) + + if tt.expectedKCPCondition != nil { + g.Expect(*conditions.Get(tt.kcp, controlplanev1.ControlPlaneComponentsHealthyCondition)).To(conditions.MatchCondition(*tt.expectedKCPCondition)) + } + for _, m := range tt.machines { + g.Expect(tt.expectedMachineConditions).To(HaveKey(m.Name)) + g.Expect(m.GetConditions()).To(conditions.MatchConditions(tt.expectedMachineConditions[m.Name])) + } + }) + } +} + +func TestUpdateStaticPodCondition(t *testing.T) { + machine := &clusterv1.Machine{} + node := corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + } + component := "kube-component" + condition := clusterv1.ConditionType("kubeComponentHealthy") + podName := staticPodName(component, node.Name) + podkey := client.ObjectKey{ + Namespace: metav1.NamespaceSystem, + Name: podName, + }.String() + + tests := []struct { + name string + injectClient client.Client // This test is injecting a fake client because it is required to create pods with a controlled Status or to fail with a specific error. + expectedCondition clusterv1.Condition + }{ + { + name: "if gets pod return a NotFound error should report PodCondition=False, PodMissing", + injectClient: &fakeClient{ + getErr: apierrors.NewNotFound(schema.ParseGroupResource("Pod"), component), + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodMissingReason, clusterv1.ConditionSeverityError, "Pod kube-component-node is missing"), + }, + { + name: "if gets pod return a generic error should report PodCondition=Unknown, PodInspectionFailed", + injectClient: &fakeClient{ + getErr: errors.New("get failure"), + }, + expectedCondition: *conditions.UnknownCondition(condition, controlplanev1.PodInspectionFailedReason, "Failed to get pod status"), + }, + { + name: "pending pod not yet scheduled should report PodCondition=False, PodProvisioning", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodPending), + withCondition(corev1.PodScheduled, corev1.ConditionFalse), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Waiting to be scheduled"), + }, + { + name: "pending pod running init containers should report PodCondition=False, PodProvisioning", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodPending), + withCondition(corev1.PodScheduled, corev1.ConditionTrue), + withCondition(corev1.PodInitialized, corev1.ConditionFalse), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Running init containers"), + }, + { + name: "pending pod with PodScheduled and PodInitialized report PodCondition=False, PodProvisioning", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodPending), + withCondition(corev1.PodScheduled, corev1.ConditionTrue), + withCondition(corev1.PodInitialized, corev1.ConditionTrue), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, ""), + }, + { + name: "running pod with podReady should report PodCondition=true", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodRunning), + withCondition(corev1.PodReady, corev1.ConditionTrue), + ), + }, + }, + expectedCondition: *conditions.TrueCondition(condition), + }, + { + name: "running pod with ContainerStatus Waiting should report PodCondition=False, PodProvisioning", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodRunning), + withContainerStatus(corev1.ContainerStatus{ + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{Reason: "Waiting something"}, + }, + }), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Waiting something"), + }, + { + name: "running pod with ContainerStatus Waiting but with exit code != 0 should report PodCondition=False, PodFailed", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodRunning), + withContainerStatus(corev1.ContainerStatus{ + State: corev1.ContainerState{ + Waiting: &corev1.ContainerStateWaiting{Reason: "Waiting something"}, + }, + LastTerminationState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: 1, + }, + }, + }), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "Waiting something"), + }, + { + name: "running pod with ContainerStatus Terminated should report PodCondition=False, PodFailed", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodRunning), + withContainerStatus(corev1.ContainerStatus{ + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{Reason: "Something failed"}, + }, + }), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "Something failed"), + }, + { + name: "running pod without podReady and without Container status messages should report PodCondition=False, PodProvisioning", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodRunning), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodProvisioningReason, clusterv1.ConditionSeverityInfo, "Waiting for startup or readiness probes"), + }, + { + name: "failed pod should report PodCondition=False, PodFailed", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodFailed), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "All the containers have been terminated"), + }, + { + name: "succeeded pod should report PodCondition=False, PodFailed", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodSucceeded), + ), + }, + }, + expectedCondition: *conditions.FalseCondition(condition, controlplanev1.PodFailedReason, clusterv1.ConditionSeverityError, "All the containers have been terminated"), + }, + { + name: "pod in unknown phase should report PodCondition=Unknown, PodInspectionFailed", + injectClient: &fakeClient{ + get: map[string]interface{}{ + podkey: fakePod(podName, + withPhase(corev1.PodUnknown), + ), + }, + }, + expectedCondition: *conditions.UnknownCondition(condition, controlplanev1.PodInspectionFailedReason, "Pod is reporting unknown status"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + w := &Workload{ + Client: tt.injectClient, + } + w.updateStaticPodCondition(ctx, machine, node, component, condition) + + g.Expect(*conditions.Get(machine, condition)).To(conditions.MatchCondition(tt.expectedCondition)) + }) + } +} + +type fakeNodeOption func(*corev1.Node) + +func fakeNode(name string, options ...fakeNodeOption) *corev1.Node { + p := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } + for _, opt := range options { + opt(p) + } + return p +} + +func withUnreachableTaint() fakeNodeOption { + return func(node *corev1.Node) { + node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{ + Key: corev1.TaintNodeUnreachable, + Effect: corev1.TaintEffectNoExecute, + }) + } +} + +type fakeMachineOption func(*clusterv1.Machine) + +func fakeMachine(name string, options ...fakeMachineOption) *clusterv1.Machine { + p := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } + for _, opt := range options { + opt(p) + } + return p +} + +func withNodeRef(ref string) fakeMachineOption { + return func(machine *clusterv1.Machine) { + machine.Status.NodeRef = &corev1.ObjectReference{ + Kind: "Node", + Name: ref, + } + } +} + +type fakePodOption func(*corev1.Pod) + +func fakePod(name string, options ...fakePodOption) *corev1.Pod { + p := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceSystem, + }, + } + for _, opt := range options { + opt(p) + } + return p +} + +func withPhase(phase corev1.PodPhase) fakePodOption { + return func(pod *corev1.Pod) { + pod.Status.Phase = phase + } +} + +func withContainerStatus(status corev1.ContainerStatus) fakePodOption { + return func(pod *corev1.Pod) { + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, status) + } +} + +func withCondition(condition corev1.PodConditionType, status corev1.ConditionStatus) fakePodOption { + return func(pod *corev1.Pod) { + c := corev1.PodCondition{ + Type: condition, + Status: status, + } + pod.Status.Conditions = append(pod.Status.Conditions, c) + } +} diff --git a/controlplane/kubeadm/internal/workload_cluster_etcd.go b/controlplane/kubeadm/internal/workload_cluster_etcd.go index 78261db27911..93fed7348ef8 100644 --- a/controlplane/kubeadm/internal/workload_cluster_etcd.go +++ b/controlplane/kubeadm/internal/workload_cluster_etcd.go @@ -34,113 +34,15 @@ type etcdClientFor interface { forLeader(ctx context.Context, nodes []corev1.Node) (*etcd.Client, error) } -// EtcdIsHealthy runs checks for every etcd member in the cluster to satisfy our definition of healthy. -// This is a best effort check and nodes can become unhealthy after the check is complete. It is not a guarantee. -// It's used a signal for if we should allow a target cluster to scale up, scale down or upgrade. -// It returns a map of nodes checked along with an error for a given node. -func (w *Workload) EtcdIsHealthy(ctx context.Context) (HealthCheckResult, error) { - var knownClusterID uint64 - var knownMemberIDSet etcdutil.UInt64Set - - controlPlaneNodes, err := w.getControlPlaneNodes(ctx) - if err != nil { - return nil, err - } - - expectedMembers := 0 - response := make(map[string]error) - for _, node := range controlPlaneNodes.Items { - name := node.Name - response[name] = nil - if node.Spec.ProviderID == "" { - response[name] = errors.New("empty provider ID") - continue - } - - // Check to see if the pod is ready - etcdPodKey := ctrlclient.ObjectKey{ - Namespace: metav1.NamespaceSystem, - Name: staticPodName("etcd", name), - } - pod := corev1.Pod{} - if err := w.Client.Get(ctx, etcdPodKey, &pod); err != nil { - response[name] = errors.Wrap(err, "failed to get etcd pod") - continue - } - if err := checkStaticPodReadyCondition(pod); err != nil { - // Nothing wrong here, etcd on this node is just not running. - // If it's a true failure the healthcheck will fail since it won't have checked enough members. - continue - } - // Only expect a member reports healthy if its pod is ready. - // This fixes the known state where the control plane has a crash-looping etcd pod that is not part of the - // etcd cluster. - expectedMembers++ - - // Create the etcd Client for the etcd Pod scheduled on the Node - etcdClient, err := w.etcdClientGenerator.forNodes(ctx, []corev1.Node{node}) - if err != nil { - response[name] = errors.Wrap(err, "failed to create etcd client") - continue - } - defer etcdClient.Close() - - // List etcd members. This checks that the member is healthy, because the request goes through consensus. - members, err := etcdClient.Members(ctx) - if err != nil { - response[name] = errors.Wrap(err, "failed to list etcd members using etcd client") - continue - } - - member := etcdutil.MemberForName(members, name) - - // Check that the member reports no alarms. - if len(member.Alarms) > 0 { - response[name] = errors.Errorf("etcd member reports alarms: %v", member.Alarms) - continue - } - - // Check that the member belongs to the same cluster as all other members. - clusterID := member.ClusterID - if knownClusterID == 0 { - knownClusterID = clusterID - } else if knownClusterID != clusterID { - response[name] = errors.Errorf("etcd member has cluster ID %d, but all previously seen etcd members have cluster ID %d", clusterID, knownClusterID) - continue - } - - // Check that the member list is stable. - memberIDSet := etcdutil.MemberIDSet(members) - if knownMemberIDSet.Len() == 0 { - knownMemberIDSet = memberIDSet - } else { - unknownMembers := memberIDSet.Difference(knownMemberIDSet) - if unknownMembers.Len() > 0 { - response[name] = errors.Errorf("etcd member reports members IDs %v, but all previously seen etcd members reported member IDs %v", memberIDSet.UnsortedList(), knownMemberIDSet.UnsortedList()) - } - continue - } - } - - // TODO: ensure that each pod is owned by a node that we're managing. That would ensure there are no out-of-band etcd members - - // Check that there is exactly one etcd member for every healthy pod. - // This allows us to handle the expected case where there is a failing pod but it's been removed from the member list. - if expectedMembers != len(knownMemberIDSet) { - return response, errors.Errorf("there are %d healthy etcd pods, but %d etcd members", expectedMembers, len(knownMemberIDSet)) - } - - return response, nil -} - // ReconcileEtcdMembers iterates over all etcd members and finds members that do not have corresponding nodes. // If there are any such members, it deletes them from etcd and removes their nodes from the kubeadm configmap so that kubeadm does not run etcd health checks on them. -func (w *Workload) ReconcileEtcdMembers(ctx context.Context) error { +func (w *Workload) ReconcileEtcdMembers(ctx context.Context) ([]string, error) { controlPlaneNodes, err := w.getControlPlaneNodes(ctx) if err != nil { - return err + return nil, err } + removedMembers := []string{} errs := []error{} for _, node := range controlPlaneNodes.Items { // Create the etcd Client for the etcd Pod scheduled on the Node @@ -168,6 +70,7 @@ func (w *Workload) ReconcileEtcdMembers(ctx context.Context) error { if isFound { continue } + removedMembers = append(removedMembers, member.Name) if err := w.removeMemberForNode(ctx, member.Name); err != nil { errs = append(errs, err) } @@ -177,7 +80,7 @@ func (w *Workload) ReconcileEtcdMembers(ctx context.Context) error { } } } - return kerrors.NewAggregate(errs) + return removedMembers, kerrors.NewAggregate(errs) } // UpdateEtcdVersionInKubeadmConfigMap sets the imageRepository or the imageTag or both in the kubeadm config map. diff --git a/controlplane/kubeadm/internal/workload_cluster_etcd_test.go b/controlplane/kubeadm/internal/workload_cluster_etcd_test.go index d4384d4bbec7..4deb3a4f36bc 100644 --- a/controlplane/kubeadm/internal/workload_cluster_etcd_test.go +++ b/controlplane/kubeadm/internal/workload_cluster_etcd_test.go @@ -36,53 +36,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func TestWorkload_EtcdIsHealthy(t *testing.T) { - g := NewWithT(t) - - workload := &Workload{ - Client: &fakeClient{ - get: map[string]interface{}{ - "kube-system/etcd-test-1": etcdPod("etcd-test-1", withReadyOption), - "kube-system/etcd-test-2": etcdPod("etcd-test-2", withReadyOption), - "kube-system/etcd-test-3": etcdPod("etcd-test-3", withReadyOption), - "kube-system/etcd-test-4": etcdPod("etcd-test-4"), - }, - list: &corev1.NodeList{ - Items: []corev1.Node{ - nodeNamed("test-1", withProviderID("my-provider-id-1")), - nodeNamed("test-2", withProviderID("my-provider-id-2")), - nodeNamed("test-3", withProviderID("my-provider-id-3")), - nodeNamed("test-4", withProviderID("my-provider-id-4")), - }, - }, - }, - etcdClientGenerator: &fakeEtcdClientGenerator{ - forNodesClient: &etcd.Client{ - EtcdClient: &fake2.FakeEtcdClient{ - EtcdEndpoints: []string{}, - MemberListResponse: &clientv3.MemberListResponse{ - Members: []*pb.Member{ - {Name: "test-1", ID: uint64(1)}, - {Name: "test-2", ID: uint64(2)}, - {Name: "test-3", ID: uint64(3)}, - }, - }, - AlarmResponse: &clientv3.AlarmResponse{ - Alarms: []*pb.AlarmMember{}, - }, - }, - }, - }, - } - ctx := context.Background() - health, err := workload.EtcdIsHealthy(ctx) - g.Expect(err).NotTo(HaveOccurred()) - - for _, err := range health { - g.Expect(err).NotTo(HaveOccurred()) - } -} - func TestUpdateEtcdVersionInKubeadmConfigMap(t *testing.T) { kubeadmConfig := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -603,7 +556,7 @@ kind: ClusterStatus`, etcdClientGenerator: tt.etcdClientGenerator, } ctx := context.TODO() - err := w.ReconcileEtcdMembers(ctx) + _, err := w.ReconcileEtcdMembers(ctx) if tt.expectErr { g.Expect(err).To(HaveOccurred()) return @@ -619,13 +572,17 @@ kind: ClusterStatus`, } type fakeEtcdClientGenerator struct { - forNodesClient *etcd.Client - forLeaderClient *etcd.Client - forNodesErr error - forLeaderErr error + forNodesClient *etcd.Client + forNodesClientFunc func([]corev1.Node) (*etcd.Client, error) + forLeaderClient *etcd.Client + forNodesErr error + forLeaderErr error } -func (c *fakeEtcdClientGenerator) forNodes(_ context.Context, _ []corev1.Node) (*etcd.Client, error) { +func (c *fakeEtcdClientGenerator) forNodes(_ context.Context, n []corev1.Node) (*etcd.Client, error) { + if c.forNodesClientFunc != nil { + return c.forNodesClientFunc(n) + } return c.forNodesClient, c.forNodesErr } @@ -633,35 +590,6 @@ func (c *fakeEtcdClientGenerator) forLeader(_ context.Context, _ []corev1.Node) return c.forLeaderClient, c.forLeaderErr } -type podOption func(*corev1.Pod) - -func etcdPod(name string, options ...podOption) *corev1.Pod { - p := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: metav1.NamespaceSystem, - }, - } - for _, opt := range options { - opt(p) - } - return p -} -func withReadyOption(pod *corev1.Pod) { - readyCondition := corev1.PodCondition{ - Type: corev1.PodReady, - Status: corev1.ConditionTrue, - } - pod.Status.Conditions = append(pod.Status.Conditions, readyCondition) -} - -func withProviderID(pi string) func(corev1.Node) corev1.Node { - return func(node corev1.Node) corev1.Node { - node.Spec.ProviderID = pi - return node - } -} - func defaultMachine(transforms ...func(m *clusterv1.Machine)) *clusterv1.Machine { m := &clusterv1.Machine{ Status: clusterv1.MachineStatus{ @@ -675,3 +603,12 @@ func defaultMachine(transforms ...func(m *clusterv1.Machine)) *clusterv1.Machine } return m } + +func nodeNamed(name string) corev1.Node { + node := corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } + return node +} diff --git a/controlplane/kubeadm/internal/workload_cluster_test.go b/controlplane/kubeadm/internal/workload_cluster_test.go index 4cee568a4500..995f3d47967f 100644 --- a/controlplane/kubeadm/internal/workload_cluster_test.go +++ b/controlplane/kubeadm/internal/workload_cluster_test.go @@ -28,7 +28,6 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" cabpkv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3" @@ -618,165 +617,3 @@ func newKubeProxyDSWithImage(image string) appsv1.DaemonSet { ds.Spec.Template.Spec.Containers[0].Image = image return ds } - -func TestHealthCheck_NoError(t *testing.T) { - threeMachines := []*clusterv1.Machine{ - controlPlaneMachine("one"), - controlPlaneMachine("two"), - controlPlaneMachine("three"), - } - controlPlane := createControlPlane(threeMachines) - tests := []struct { - name string - checkResult HealthCheckResult - controlPlaneName string - controlPlane *ControlPlane - }{ - { - name: "simple", - checkResult: HealthCheckResult{ - "one": nil, - "two": nil, - "three": nil, - }, - controlPlane: controlPlane, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - g.Expect(tt.checkResult.Aggregate(controlPlane)).To(Succeed()) - }) - } -} - -func TestManagementCluster_healthCheck_Errors(t *testing.T) { - tests := []struct { - name string - checkResult HealthCheckResult - clusterKey ctrlclient.ObjectKey - controlPlaneName string - controlPlane *ControlPlane - // expected errors will ensure the error contains this list of strings. - // If not supplied, no check on the error's value will occur. - expectedErrors []string - }{ - { - name: "machine's node was not checked for health", - controlPlane: createControlPlane([]*clusterv1.Machine{ - controlPlaneMachine("one"), - controlPlaneMachine("two"), - controlPlaneMachine("three"), - }), - checkResult: HealthCheckResult{ - "one": nil, - }, - }, - { - name: "two nodes error on the check but no overall error occurred", - controlPlane: createControlPlane([]*clusterv1.Machine{ - controlPlaneMachine("one"), - controlPlaneMachine("two"), - controlPlaneMachine("three")}), - checkResult: HealthCheckResult{ - "one": nil, - "two": errors.New("two"), - "three": errors.New("three"), - }, - expectedErrors: []string{"two", "three"}, - }, - { - name: "more nodes than machines were checked (out of band control plane nodes)", - controlPlane: createControlPlane([]*clusterv1.Machine{ - controlPlaneMachine("one")}), - checkResult: HealthCheckResult{ - "one": nil, - "two": nil, - "three": nil, - }, - }, - { - name: "a machine that has a nil node reference", - controlPlane: createControlPlane([]*clusterv1.Machine{ - controlPlaneMachine("one"), - controlPlaneMachine("two"), - nilNodeRef(controlPlaneMachine("three"))}), - checkResult: HealthCheckResult{ - "one": nil, - "two": nil, - "three": nil, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - err := tt.checkResult.Aggregate(tt.controlPlane) - g.Expect(err).To(HaveOccurred()) - - for _, expectedError := range tt.expectedErrors { - g.Expect(err).To(MatchError(ContainSubstring(expectedError))) - } - }) - } -} -func createControlPlane(machines []*clusterv1.Machine) *ControlPlane { - defaultInfra := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "kind": "InfrastructureMachine", - "apiVersion": "infrastructure.cluster.x-k8s.io/v1alpha3", - "metadata": map[string]interface{}{ - "name": "infra-config1", - "namespace": "default", - }, - "spec": map[string]interface{}{}, - "status": map[string]interface{}{}, - }, - } - - fakeClient := fake.NewFakeClientWithScheme(runtime.NewScheme(), defaultInfra.DeepCopy()) - - controlPlane, _ := NewControlPlane(ctx, fakeClient, &clusterv1.Cluster{}, &v1alpha3.KubeadmControlPlane{}, NewFilterableMachineCollection(machines...)) - return controlPlane -} - -func controlPlaneMachine(name string) *clusterv1.Machine { - t := true - infraRef := &corev1.ObjectReference{ - Kind: "InfraKind", - APIVersion: "infrastructure.cluster.x-k8s.io/v1alpha3", - Name: "infra", - Namespace: "default", - } - - return &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: name, - Labels: ControlPlaneLabelsForCluster("cluster-name"), - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "KubeadmControlPlane", - Name: "control-plane-name", - Controller: &t, - }, - }, - }, - Spec: clusterv1.MachineSpec{ - InfrastructureRef: *infraRef.DeepCopy(), - }, - Status: clusterv1.MachineStatus{ - NodeRef: &corev1.ObjectReference{ - Name: name, - }, - }, - } -} - -func nilNodeRef(machine *clusterv1.Machine) *clusterv1.Machine { - machine.Status.NodeRef = nil - return machine -}