From 8d6e9fb68b05fc955b625dd48cdd6929668ed9e4 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 13 Feb 2020 14:25:41 +0000 Subject: [PATCH] Add remediation logic for Machine HealthChecking --- api/v1alpha3/machinehealthcheck_webhook.go | 9 + .../machinehealthcheck_webhook_test.go | 44 +++ controllers/machinehealthcheck_controller.go | 62 +++- .../machinehealthcheck_controller_test.go | 86 +++++ controllers/machinehealthcheck_targets.go | 101 ++++++ .../machinehealthcheck_targets_test.go | 319 +++++++++++++++++- 6 files changed, 619 insertions(+), 2 deletions(-) diff --git a/api/v1alpha3/machinehealthcheck_webhook.go b/api/v1alpha3/machinehealthcheck_webhook.go index 4c32b7a0085b..bcab8b39512a 100644 --- a/api/v1alpha3/machinehealthcheck_webhook.go +++ b/api/v1alpha3/machinehealthcheck_webhook.go @@ -106,6 +106,15 @@ func (m *MachineHealthCheck) validate(old *MachineHealthCheck) error { ) } + if m.Spec.MaxUnhealthy != nil { + if _, err := intstr.GetValueFromIntOrPercent(m.Spec.MaxUnhealthy, 0, false); err != nil { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", "maxUnhealthy"), m.Spec.MaxUnhealthy, "must be either an int or a percentage"), + ) + } + } + if len(allErrs) == 0 { return nil } diff --git a/api/v1alpha3/machinehealthcheck_webhook_test.go b/api/v1alpha3/machinehealthcheck_webhook_test.go index e838f3a4cf12..f9b4b3550cf9 100644 --- a/api/v1alpha3/machinehealthcheck_webhook_test.go +++ b/api/v1alpha3/machinehealthcheck_webhook_test.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) func TestMachineHealthCheckDefault(t *testing.T) { @@ -181,3 +182,46 @@ func TestMachineHealthCheckNodeStartupTimeout(t *testing.T) { } } } + +func TestMachineHealthCheckMaxUnhealthy(t *testing.T) { + tests := []struct { + name string + value intstr.IntOrString + expectErr bool + }{ + { + name: "when the value is an integer", + value: intstr.Parse("10"), + expectErr: false, + }, + { + name: "when the value is a percentage", + value: intstr.Parse("10%"), + expectErr: false, + }, + { + name: "when the value is a random string", + value: intstr.Parse("abcdef"), + expectErr: true, + }, + } + + for _, tt := range tests { + g := NewWithT(t) + + maxUnhealthy := tt.value + mhc := &MachineHealthCheck{ + Spec: MachineHealthCheckSpec{ + MaxUnhealthy: &maxUnhealthy, + }, + } + + if tt.expectErr { + g.Expect(mhc.ValidateCreate()).NotTo(Succeed()) + g.Expect(mhc.ValidateUpdate(mhc)).NotTo(Succeed()) + } else { + g.Expect(mhc.ValidateCreate()).To(Succeed()) + g.Expect(mhc.ValidateUpdate(mhc)).To(Succeed()) + } + } +} diff --git a/controllers/machinehealthcheck_controller.go b/controllers/machinehealthcheck_controller.go index 299609713f51..4732c9250386 100644 --- a/controllers/machinehealthcheck_controller.go +++ b/controllers/machinehealthcheck_controller.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" @@ -49,6 +50,12 @@ import ( const ( mhcClusterNameIndex = "spec.clusterName" machineNodeNameIndex = "status.nodeRef.name" + + // Event types + + // EventRemediationRestricted is emitted in case when machine remediation + // is restricted by remediation circuit shorting logic + EventRemediationRestricted string = "RemediationRestricted" ) // MachineHealthCheckReconciler reconciles a MachineHealthCheck object @@ -206,10 +213,47 @@ func (r *MachineHealthCheckReconciler) reconcile(ctx context.Context, cluster *c currentHealthy, needRemediationTargets, nextCheckTimes := r.healthCheckTargets(targets, logger, m.Spec.NodeStartupTimeout.Duration) m.Status.CurrentHealthy = int32(currentHealthy) + // check MHC current health against MaxUnhealthy + if !isAllowedRemediation(m) { + logger.V(3).Info( + "Short-circuiting remediation", + "total target", totalTargets, + "max unhealthy", m.Spec.MaxUnhealthy, + "unhealthy targets", totalTargets-currentHealthy, + ) + + r.recorder.Eventf( + m, + corev1.EventTypeWarning, + EventRemediationRestricted, + "Remediation restricted due to exceeded number of unhealthy machines (total: %v, unhealthy: %v, maxUnhealthy: %v)", + totalTargets, + totalTargets-currentHealthy, + m.Spec.MaxUnhealthy, + ) + return reconcile.Result{Requeue: true}, nil + } + logger.V(3).Info( + "Remediations are allowed", + "total target", totalTargets, + "max unhealthy", m.Spec.MaxUnhealthy, + "unhealthy targets", totalTargets-currentHealthy, + ) + // remediate + errList := []error{} for _, t := range needRemediationTargets { logger.V(3).Info("Target meets unhealthy criteria, triggers remediation", "target", t.string()) - // TODO(JoelSpeed): Implement remediation logic + if err := t.remediate(ctx, logger, r.Client, r.recorder); err != nil { + logger.Error(err, "Error remediating target", "target", t.string()) + errList = append(errList, err) + } + } + + // handle remediation errors + if len(errList) > 0 { + logger.V(3).Info("Error(s) remediating request, requeueing") + return reconcile.Result{}, kerrors.NewAggregate(errList) } if minNextCheck := minDuration(nextCheckTimes); minNextCheck > 0 { @@ -391,3 +435,19 @@ func (r *MachineHealthCheckReconciler) indexMachineByNodeName(object runtime.Obj return nil } + +// isAllowedRemediation checks the value of the MaxUnhealthy field to determine +// whether remediation should be allowed or not +func isAllowedRemediation(mhc *clusterv1.MachineHealthCheck) bool { + if mhc.Spec.MaxUnhealthy == nil { + return true + } + maxUnhealthy, err := intstr.GetValueFromIntOrPercent(mhc.Spec.MaxUnhealthy, int(mhc.Status.ExpectedMachines), false) + if err != nil { + return false + } + + // If unhealthy is above maxUnhealthy, short circuit any further remediation + unhealthy := mhc.Status.ExpectedMachines - mhc.Status.CurrentHealthy + return int(unhealthy) <= maxUnhealthy +} diff --git a/controllers/machinehealthcheck_controller_test.go b/controllers/machinehealthcheck_controller_test.go index 91551a6998bc..48f532da52b9 100644 --- a/controllers/machinehealthcheck_controller_test.go +++ b/controllers/machinehealthcheck_controller_test.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/cluster-api/controllers/external" @@ -777,3 +778,88 @@ func TestIndexMachineByNodeName(t *testing.T) { }) } } + +func TestIsAllowedRedmediation(t *testing.T) { + testCases := []struct { + name string + maxUnhealthy *intstr.IntOrString + expectedMachines int32 + currentHealthy int32 + allowed bool + }{ + { + name: "when maxUnhealthy is not set", + maxUnhealthy: nil, + expectedMachines: int32(3), + currentHealthy: int32(0), + allowed: true, + }, + { + name: "when maxUnhealthy is not an int or percentage", + maxUnhealthy: &intstr.IntOrString{Type: intstr.String, StrVal: "abcdef"}, + expectedMachines: int32(5), + currentHealthy: int32(2), + allowed: false, + }, + { + name: "when maxUnhealthy is an int less than current unhealthy", + maxUnhealthy: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(1)}, + expectedMachines: int32(3), + currentHealthy: int32(1), + allowed: false, + }, + { + name: "when maxUnhealthy is an int equal to current unhealthy", + maxUnhealthy: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(2)}, + expectedMachines: int32(3), + currentHealthy: int32(1), + allowed: true, + }, + { + name: "when maxUnhealthy is an int greater than current unhealthy", + maxUnhealthy: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(3)}, + expectedMachines: int32(3), + currentHealthy: int32(1), + allowed: true, + }, + { + name: "when maxUnhealthy is a percentage less than current unhealthy", + maxUnhealthy: &intstr.IntOrString{Type: intstr.String, StrVal: "50%"}, + expectedMachines: int32(5), + currentHealthy: int32(2), + allowed: false, + }, + { + name: "when maxUnhealthy is a percentage equal to current unhealthy", + maxUnhealthy: &intstr.IntOrString{Type: intstr.String, StrVal: "60%"}, + expectedMachines: int32(5), + currentHealthy: int32(2), + allowed: true, + }, + { + name: "when maxUnhealthy is a percentage greater than current unhealthy", + maxUnhealthy: &intstr.IntOrString{Type: intstr.String, StrVal: "70%"}, + expectedMachines: int32(5), + currentHealthy: int32(2), + allowed: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mhc := &clusterv1.MachineHealthCheck{ + Spec: clusterv1.MachineHealthCheckSpec{ + MaxUnhealthy: tc.maxUnhealthy, + }, + Status: clusterv1.MachineHealthCheckStatus{ + ExpectedMachines: tc.expectedMachines, + CurrentHealthy: tc.currentHealthy, + }, + } + + g.Expect(isAllowedRemediation(mhc)).To(Equal(tc.allowed)) + }) + } +} diff --git a/controllers/machinehealthcheck_targets.go b/controllers/machinehealthcheck_targets.go index b474ec8c4a01..99a364b7a42e 100644 --- a/controllers/machinehealthcheck_targets.go +++ b/controllers/machinehealthcheck_targets.go @@ -26,12 +26,29 @@ 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/labels" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/controller-runtime/pkg/client" ) const ( + ownerControllerKind = "MachineSet" + nodeControlPlaneLabel = "node-role.kubernetes.io/master" + + // Event types + + // EventSkippedControlPlane is emitted in case an unhealthy node (or a machine + // associated with the node) has the `master` role + EventSkippedControlPlane string = "SkippedControlPlane" + // EventMachineDeletionFailed is emitted in case remediation of a machine + // is required but deletion of its Machine object failed + EventMachineDeletionFailed string = "MachineDeletionFailed" + // EventMachineDeleted is emitted when machine was successfully remediated + // by deleting its Machine object + EventMachineDeleted string = "MachineDeleted" // EventDetectedUnhealthy is emitted in case a node associated with a // machine was detected unhealthy EventDetectedUnhealthy string = "DetectedUnhealthy" @@ -256,3 +273,87 @@ func minDuration(durations []time.Duration) time.Duration { } return minDuration } + +// remediate deletes the Machine if it is owned by a MachineSet and is not part of the cluster's control plane +func (t *healthCheckTarget) remediate(ctx context.Context, logger logr.Logger, c client.Client, r record.EventRecorder) error { + logger = logger.WithValues("target", t.string()) + logger.Info("Starting remediation for target") + + // If the machine is not owned by a MachineSet, it should be skipped + hasOwner, err := t.hasMachineSetOwner() + if err != nil { + return fmt.Errorf("%s: unable to determine Machine owners: %v", t.string(), err) + } + if !hasOwner { + logger.Info("Target has no machineset owner, skipping remediation") + return nil + } + + // If the machine is a control plane node, it should be skipped + if t.isControlPlane() { + r.Eventf( + t.Machine, + corev1.EventTypeNormal, + EventSkippedControlPlane, + "Machine %v is a control plane node, skipping remediation", + t.string(), + ) + logger.Info("Target is a control plane node, skipping remediation") + return nil + } + + logger.Info("Deleting target machine") + if err := c.Delete(ctx, t.Machine); err != nil { + r.Eventf( + t.Machine, + corev1.EventTypeWarning, + EventMachineDeletionFailed, + "Machine %v remediation failed: unable to delete Machine object: %v", + t.string(), + err, + ) + return fmt.Errorf("%s: failed to delete machine: %v", t.string(), err) + } + r.Eventf( + t.Machine, + corev1.EventTypeNormal, + EventMachineDeleted, + "Machine %v has been remediated by requesting to delete Machine object", + t.string(), + ) + + return nil +} + +// hasMachineSetOwner checks whether the target's Machine is owned by a MachineSet +func (t *healthCheckTarget) hasMachineSetOwner() (bool, error) { + ownerRefs := t.Machine.ObjectMeta.GetOwnerReferences() + for _, or := range ownerRefs { + if or.Kind == ownerControllerKind { + // The Kind matches so check the Group matches as well + gv, err := schema.ParseGroupVersion(or.APIVersion) + if err != nil { + return false, err + } + if gv.Group == clusterv1.GroupVersion.Group { + return true, nil + } + } + } + return false, nil +} + +// isControlPlane checks whether the target refers to a machine that is part of the +// cluster's control plane +func (t *healthCheckTarget) isControlPlane() bool { + if t.Node != nil && labels.Set(t.Node.Labels).Has(nodeControlPlaneLabel) { + return true + } + + // if the node is not found, fallback to checking the machine + if t.Machine != nil && labels.Set(t.Machine.Labels).Has(clusterv1.MachineControlPlaneLabelName) { + return true + } + + return false +} diff --git a/controllers/machinehealthcheck_targets_test.go b/controllers/machinehealthcheck_targets_test.go index cef69134e1dd..0ce8bbb39fdf 100644 --- a/controllers/machinehealthcheck_targets_test.go +++ b/controllers/machinehealthcheck_targets_test.go @@ -17,16 +17,22 @@ limitations under the License. package controllers import ( + "context" + "fmt" "testing" "time" . "github.com/onsi/gomega" + gtypes "github.com/onsi/gomega/types" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -327,7 +333,318 @@ func TestHealthCheckTargets(t *testing.T) { } } +func TestRemediate(t *testing.T) { + namespace := "test-mhc" + clusterName := "test-cluster" + labels := map[string]string{"cluster": clusterName, "machine-group": "foo"} + + machineSetORs := []metav1.OwnerReference{{APIVersion: clusterv1.GroupVersion.String(), Kind: "MachineSet"}} + kubeadmControlPlaneORs := []metav1.OwnerReference{{APIVersion: controlplanev1.GroupVersion.String(), Kind: "KubeadmControlPlane"}} + + workerNode := newTestNode("worker-node") + workerMachine := newTestMachine("worker-machine", namespace, clusterName, workerNode.Name, labels) + workerMachine.SetOwnerReferences(machineSetORs) + workerMachineUnowned := newTestMachine("worker-machine", namespace, clusterName, workerNode.Name, labels) + + controlPlaneNode := newTestNode("control-plane-node") + if controlPlaneNode.Labels == nil { + controlPlaneNode.Labels = make(map[string]string) + } + controlPlaneNode.Labels[nodeControlPlaneLabel] = "" + + controlPlaneMachine := newTestMachine("control-plane-machine", namespace, clusterName, controlPlaneNode.Name, labels) + controlPlaneMachine.SetOwnerReferences(kubeadmControlPlaneORs) + if controlPlaneMachine.Labels == nil { + controlPlaneMachine.Labels = make(map[string]string) + } + controlPlaneMachine.Labels[clusterv1.MachineControlPlaneLabelName] = "" + + testCases := []struct { + name string + node *corev1.Node + machine *clusterv1.Machine + expectErr bool + expectDeleted bool + expectEvents []string + }{ + { + name: "when the machine is not owned by a machineset", + node: workerNode, + machine: workerMachineUnowned, + expectErr: false, + expectDeleted: false, + expectEvents: []string{}, + }, + { + name: "when the node is a worker with a machine owned by a machineset", + node: workerNode, + machine: workerMachine, + expectErr: false, + expectDeleted: true, + expectEvents: []string{EventMachineDeleted}, + }, + { + name: "when the node is a control plane node", + node: controlPlaneNode, + machine: controlPlaneMachine, + expectErr: false, + expectDeleted: false, + expectEvents: []string{}, + }, + { + name: "when the machine is a control plane machine", + node: nil, + machine: controlPlaneMachine, + expectErr: false, + expectDeleted: false, + expectEvents: []string{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewGomegaWithT(t) + + target := &healthCheckTarget{ + Node: tc.node, + Machine: tc.machine, + MHC: newTestMachineHealthCheck("mhc", namespace, clusterName, labels), + } + + fakeRecorder := record.NewFakeRecorder(2) + gs.Expect(clusterv1.AddToScheme(scheme.Scheme)).To(Succeed()) + k8sClient := fake.NewFakeClientWithScheme(scheme.Scheme, target.Machine) + if target.Node != nil { + gs.Expect(k8sClient.Create(context.Background(), target.Node)).To(Succeed()) + } + + // Run rememdiation + err := target.remediate(context.Background(), log.Log, k8sClient, fakeRecorder) + gs.Expect(err != nil).To(Equal(tc.expectErr)) + + machine := &clusterv1.Machine{} + key := types.NamespacedName{Namespace: tc.machine.Namespace, Name: tc.machine.Name} + err = k8sClient.Get(context.Background(), key, machine) + + // Check if the machine was deleted or not + if tc.expectDeleted { + gs.Expect(errors.IsNotFound(err)).To(BeTrue()) + } else { + gs.Expect(err).ToNot(HaveOccurred()) + gs.Expect(machine).To(Equal(target.Machine)) + } + + // Check which event types were sent + gs.Expect(fakeRecorder.Events).To(HaveLen(len(tc.expectEvents))) + receivedEvents := []string{} + eventMatchers := []gtypes.GomegaMatcher{} + for _, ev := range tc.expectEvents { + receivedEvents = append(receivedEvents, <-fakeRecorder.Events) + eventMatchers = append(eventMatchers, ContainSubstring(fmt.Sprintf(" %s ", ev))) + } + gs.Expect(receivedEvents).To(ConsistOf(eventMatchers)) + }) + } +} + +func TestHasMachineSetOwner(t *testing.T) { + machineSetOR := metav1.OwnerReference{ + Kind: "MachineSet", + APIVersion: clusterv1.GroupVersion.String(), + } + + machineDeploymentOR := metav1.OwnerReference{ + Kind: "MachineDeployment", + APIVersion: clusterv1.GroupVersion.String(), + } + + differentGroupOR := metav1.OwnerReference{ + Kind: "MachineSet", + APIVersion: "different-group/v1", + } + + invalidGroupOR := metav1.OwnerReference{ + Kind: "MachineSet", + APIVersion: "invalid/group/", + } + + testCases := []struct { + name string + ownerReferences []metav1.OwnerReference + hasOwner bool + expectErr bool + }{ + { + name: "with no owner references", + ownerReferences: []metav1.OwnerReference{}, + hasOwner: false, + expectErr: false, + }, + { + name: "with a MachineDeployment owner reference", + ownerReferences: []metav1.OwnerReference{machineDeploymentOR}, + hasOwner: false, + expectErr: false, + }, + { + name: "with a MachineSet owner reference", + ownerReferences: []metav1.OwnerReference{machineSetOR}, + hasOwner: true, + expectErr: false, + }, + { + name: "with a MachineSet and MachineDeployment owner reference", + ownerReferences: []metav1.OwnerReference{machineSetOR, machineDeploymentOR}, + hasOwner: true, + expectErr: false, + }, + { + name: "with a MachineSet owner reference from a different API Group", + ownerReferences: []metav1.OwnerReference{differentGroupOR}, + hasOwner: false, + expectErr: false, + }, + { + name: "with a MachineSet owner reference from an invalid API Group", + ownerReferences: []metav1.OwnerReference{invalidGroupOR}, + hasOwner: false, + expectErr: true, + }, + { + name: "with MachineSet owner references from two API Groups", + ownerReferences: []metav1.OwnerReference{differentGroupOR, machineSetOR}, + hasOwner: true, + expectErr: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewGomegaWithT(t) + + machine := newTestMachine("machine", "test-mhc", "test-cluster", "node", map[string]string{}) + machine.SetOwnerReferences(tc.ownerReferences) + + target := &healthCheckTarget{ + Machine: machine, + } + + hasOwner, err := target.hasMachineSetOwner() + gs.Expect(err != nil).To(Equal(tc.expectErr)) + gs.Expect(hasOwner).To(Equal(tc.hasOwner)) + }) + } +} + +func TestIsControlPlane(t *testing.T) { + namespace := "test-mhc" + clusterName := "test-cluster" + labels := map[string]string{"cluster": clusterName, "machine-group": "foo"} + + workerNode := newTestNode("worker-node") + workerMachine := newTestMachine("worker-machine", namespace, clusterName, workerNode.Name, labels) + + controlPlaneNode1 := newTestNode("control-plane-node") + if controlPlaneNode1.Labels == nil { + controlPlaneNode1.Labels = make(map[string]string) + } + controlPlaneNode1.Labels[nodeControlPlaneLabel] = "" + + controlPlaneMachine1 := newTestMachine("control-plane-machine", namespace, clusterName, controlPlaneNode1.Name, labels) + if controlPlaneMachine1.Labels == nil { + controlPlaneMachine1.Labels = make(map[string]string) + } + controlPlaneMachine1.Labels[clusterv1.MachineControlPlaneLabelName] = "" + + controlPlaneNode2 := newTestNode("control-plane-node") + if controlPlaneNode2.Labels == nil { + controlPlaneNode2.Labels = make(map[string]string) + } + controlPlaneNode2.Labels[nodeControlPlaneLabel] = "abcdef" + + controlPlaneMachine2 := newTestMachine("control-plane-machine", namespace, clusterName, controlPlaneNode2.Name, labels) + if controlPlaneMachine2.Labels == nil { + controlPlaneMachine2.Labels = make(map[string]string) + } + controlPlaneMachine2.Labels[clusterv1.MachineControlPlaneLabelName] = "abcdef" + + testCases := []struct { + name string + node *corev1.Node + machine *clusterv1.Machine + isControlPlane bool + }{ + { + name: "when the node and machine are nil", + node: nil, + machine: nil, + isControlPlane: false, + }, + { + name: "when the node is not a control plane node", + node: workerNode, + machine: workerMachine, + isControlPlane: false, + }, + { + name: "when the node is missing and the machine is not a control plane machine", + node: nil, + machine: workerMachine, + isControlPlane: false, + }, + { + name: "when the node is a control plane node", + node: controlPlaneNode1, + machine: controlPlaneMachine1, + isControlPlane: true, + }, + { + name: "when the node is missing and the machine is a control plane machine", + node: nil, + machine: controlPlaneMachine1, + isControlPlane: true, + }, + { + name: "when the node control plane label has a value", + node: controlPlaneNode2, + machine: controlPlaneMachine2, + isControlPlane: true, + }, + { + name: "when the machine control plane label has a value", + node: nil, + machine: controlPlaneMachine2, + isControlPlane: true, + }, + { + name: "when the node is not a control plane node, but the machine is a control plane machine", + node: workerNode, + machine: controlPlaneMachine1, + isControlPlane: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := NewGomegaWithT(t) + + target := &healthCheckTarget{ + Node: tc.node, + Machine: tc.machine, + } + + gs.Expect(target.isControlPlane()).To(Equal(tc.isControlPlane)) + }) + } +} + func newTestMachine(name, namespace, clusterName, nodeName string, labels map[string]string) *clusterv1.Machine { + // Copy the labels so that the map is unique to each test Machine + l := make(map[string]string) + for k, v := range labels { + l[k] = v + } + bootstrap := "bootstrap" return &clusterv1.Machine{ TypeMeta: metav1.TypeMeta{ @@ -337,7 +654,7 @@ func newTestMachine(name, namespace, clusterName, nodeName string, labels map[st ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, - Labels: labels, + Labels: l, }, Spec: clusterv1.MachineSpec{ ClusterName: clusterName,