diff --git a/controllers/machinehealthcheck_controller_test.go b/controllers/machinehealthcheck_controller_test.go index e0c4eb3865ae..64f96189a665 100644 --- a/controllers/machinehealthcheck_controller_test.go +++ b/controllers/machinehealthcheck_controller_test.go @@ -22,6 +22,8 @@ import ( "time" . "github.com/onsi/gomega" + capierrors "sigs.k8s.io/cluster-api/errors" + "sigs.k8s.io/cluster-api/util/patch" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -175,6 +177,54 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { )) }) + t.Run("it ignores Machines not matching the label selector", func(t *testing.T) { + g := NewWithT(t) + ctx := context.TODO() + cluster := createNamespaceAndCluster(g) + + mhc := newMachineHealthCheck(cluster.Namespace, cluster.Name) + + g.Expect(testEnv.Create(ctx, mhc)).To(Succeed()) + defer func(do ...runtime.Object) { + g.Expect(testEnv.Cleanup(ctx, do...)).To(Succeed()) + }(cluster, mhc) + + // Healthy nodes and machines matching the MHC's label selector. + _, machines, cleanup := createMachinesWithNodes(g, cluster, + count(2), + createNodeRefForMachine(true), + nodeStatus(corev1.ConditionTrue), + machineLabels(mhc.Spec.Selector.MatchLabels), + ) + defer cleanup() + targetMachines := make([]string, len(machines)) + for i, m := range machines { + targetMachines[i] = m.Name + } + + // Healthy nodes and machines NOT matching the MHC's label selector. + _, _, cleanup2 := createMachinesWithNodes(g, cluster, + count(2), + createNodeRefForMachine(true), + nodeStatus(corev1.ConditionTrue), + ) + defer cleanup2() + + // Make sure the status matches. + g.Eventually(func() *clusterv1.MachineHealthCheckStatus { + err := testEnv.Get(ctx, util.ObjectKey(mhc), mhc) + if err != nil { + return nil + } + return &mhc.Status + }).Should(MatchMachineHealthCheckStatus(&clusterv1.MachineHealthCheckStatus{ + ExpectedMachines: 2, + CurrentHealthy: 2, + ObservedGeneration: 1, + Targets: targetMachines}, + )) + }) + t.Run("it doesn't mark anything unhealthy when all Machines are healthy", func(t *testing.T) { g := NewWithT(t) ctx := context.TODO() @@ -191,7 +241,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, machines, cleanup := createMachinesWithNodes(g, cluster, count(2), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup() @@ -214,7 +264,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { )) }) - t.Run("it marks unhealthy machines for remediation when there is one unhealthy Machine", func(t *testing.T) { + t.Run("it marks unhealthy machines for remediation when there is one unhealthy Node", func(t *testing.T) { g := NewWithT(t) ctx := context.TODO() cluster := createNamespaceAndCluster(g) @@ -230,7 +280,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, machines, cleanup1 := createMachinesWithNodes(g, cluster, count(2), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup1() @@ -238,8 +288,108 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, unhealthyMachines, cleanup2 := createMachinesWithNodes(g, cluster, count(1), createNodeRefForMachine(true), - markNodeAsHealthy(false), + nodeStatus(corev1.ConditionUnknown), + machineLabels(mhc.Spec.Selector.MatchLabels), + ) + defer cleanup2() + machines = append(machines, unhealthyMachines...) + targetMachines := make([]string, len(machines)) + for i, m := range machines { + targetMachines[i] = m.Name + } + + // Make sure the status matches. + g.Eventually(func() *clusterv1.MachineHealthCheckStatus { + err := testEnv.Get(ctx, util.ObjectKey(mhc), mhc) + if err != nil { + return nil + } + return &mhc.Status + }).Should(MatchMachineHealthCheckStatus(&clusterv1.MachineHealthCheckStatus{ + ExpectedMachines: 3, + CurrentHealthy: 2, + ObservedGeneration: 1, + Targets: targetMachines, + })) + }) + + t.Run("it marks unhealthy machines for remediation when there a Machine has a failure reason", func(t *testing.T) { + g := NewWithT(t) + ctx := context.TODO() + cluster := createNamespaceAndCluster(g) + + mhc := newMachineHealthCheck(cluster.Namespace, cluster.Name) + + g.Expect(testEnv.Create(ctx, mhc)).To(Succeed()) + defer func(do ...runtime.Object) { + g.Expect(testEnv.Cleanup(ctx, do...)).To(Succeed()) + }(cluster, mhc) + + // Healthy nodes and machines. + _, machines, cleanup1 := createMachinesWithNodes(g, cluster, + count(2), + createNodeRefForMachine(true), + nodeStatus(corev1.ConditionTrue), + machineLabels(mhc.Spec.Selector.MatchLabels), + ) + defer cleanup1() + // Machine with failure reason. + _, unhealthyMachines, cleanup2 := createMachinesWithNodes(g, cluster, + count(1), + createNodeRefForMachine(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), + machineFailureReason("some failure"), + ) + defer cleanup2() + machines = append(machines, unhealthyMachines...) + targetMachines := make([]string, len(machines)) + for i, m := range machines { + targetMachines[i] = m.Name + } + + // Make sure the status matches. + g.Eventually(func() *clusterv1.MachineHealthCheckStatus { + err := testEnv.Get(ctx, util.ObjectKey(mhc), mhc) + if err != nil { + return nil + } + return &mhc.Status + }).Should(MatchMachineHealthCheckStatus(&clusterv1.MachineHealthCheckStatus{ + ExpectedMachines: 3, + CurrentHealthy: 2, + ObservedGeneration: 1, + Targets: targetMachines, + })) + }) + + t.Run("it marks unhealthy machines for remediation when there a Machine has a failure message", func(t *testing.T) { + g := NewWithT(t) + ctx := context.TODO() + cluster := createNamespaceAndCluster(g) + + mhc := newMachineHealthCheck(cluster.Namespace, cluster.Name) + + g.Expect(testEnv.Create(ctx, mhc)).To(Succeed()) + defer func(do ...runtime.Object) { + g.Expect(testEnv.Cleanup(ctx, do...)).To(Succeed()) + }(cluster, mhc) + + // Healthy nodes and machines. + _, machines, cleanup1 := createMachinesWithNodes(g, cluster, + count(2), + createNodeRefForMachine(true), + nodeStatus(corev1.ConditionTrue), + machineLabels(mhc.Spec.Selector.MatchLabels), + ) + defer cleanup1() + // Machine with failure message. + _, unhealthyMachines, cleanup2 := createMachinesWithNodes(g, cluster, + count(1), + createNodeRefForMachine(true), + nodeStatus(corev1.ConditionTrue), + machineLabels(mhc.Spec.Selector.MatchLabels), + machineFailureMessage("some failure"), ) defer cleanup2() machines = append(machines, unhealthyMachines...) @@ -281,7 +431,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, machines, cleanup1 := createMachinesWithNodes(g, cluster, count(1), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup1() @@ -289,7 +439,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, unhealthyMachines, cleanup2 := createMachinesWithNodes(g, cluster, count(2), createNodeRefForMachine(true), - markNodeAsHealthy(false), + nodeStatus(corev1.ConditionUnknown), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup2() @@ -342,7 +492,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { } for i := range machines.Items { - if conditions.Get(&machines.Items[i], clusterv1.MachineOwnerRemediatedCondition) != nil { + if conditions.IsTrue(&machines.Items[i], clusterv1.MachineOwnerRemediatedCondition) { remediated++ } } @@ -355,6 +505,14 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { ctx := context.TODO() cluster := createNamespaceAndCluster(g) + // After the cluster exists, we have to set the infrastructure ready condition; otherwise, MachineHealthChecks + // will never fail when nodeStartupTimeout is exceeded. + patchHelper, err := patch.NewHelper(cluster, testEnv.GetClient()) + g.Expect(err).ToNot(HaveOccurred()) + + conditions.MarkTrue(cluster, clusterv1.InfrastructureReadyCondition) + g.Expect(patchHelper.Patch(ctx, cluster)).To(Succeed()) + mhc := newMachineHealthCheck(cluster.Namespace, cluster.Name) mhc.Spec.NodeStartupTimeout = &metav1.Duration{Duration: 5 * time.Hour} @@ -367,7 +525,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, machines, cleanup1 := createMachinesWithNodes(g, cluster, count(2), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup1() @@ -375,7 +533,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, unhealthyMachines, cleanup2 := createMachinesWithNodes(g, cluster, count(1), createNodeRefForMachine(false), - markNodeAsHealthy(false), + nodeStatus(corev1.ConditionUnknown), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup2() @@ -428,7 +586,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { } for i := range machines.Items { - if conditions.Get(&machines.Items[i], clusterv1.MachineOwnerRemediatedCondition) != nil { + if conditions.IsTrue(&machines.Items[i], clusterv1.MachineOwnerRemediatedCondition) { remediated++ } } @@ -455,7 +613,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, machines, cleanup1 := createMachinesWithNodes(g, cluster, count(2), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup1() @@ -463,7 +621,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { _, unhealthyMachines, cleanup2 := createMachinesWithNodes(g, cluster, count(1), createNodeRefForMachine(false), - markNodeAsHealthy(false), + nodeStatus(corev1.ConditionUnknown), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup2() @@ -546,7 +704,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { nodes, machines, cleanup := createMachinesWithNodes(g, cluster, count(3), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup() @@ -631,7 +789,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { nodes, machines, cleanup := createMachinesWithNodes(g, cluster, count(1), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup() @@ -698,7 +856,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { return }).Should(Equal(1)) - // Calculate how many Machines have been remediated. + // Calculate how many Machines have been marked for remediation g.Eventually(func() (remediated int) { machines := &clusterv1.MachineList{} err := testEnv.List(ctx, machines, client.MatchingLabels{ @@ -709,7 +867,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { } for i := range machines.Items { - if conditions.Get(&machines.Items[i], clusterv1.MachineOwnerRemediatedCondition) != nil { + if conditions.IsFalse(&machines.Items[i], clusterv1.MachineOwnerRemediatedCondition) { remediated++ } } @@ -722,6 +880,14 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { ctx := context.TODO() cluster := createNamespaceAndCluster(g) + // After the cluster exists, we have to set the infrastructure ready condition; otherwise, MachineHealthChecks + // will never fail when nodeStartupTimeout is exceeded. + patchHelper, err := patch.NewHelper(cluster, testEnv.GetClient()) + g.Expect(err).ToNot(HaveOccurred()) + + conditions.MarkTrue(cluster, clusterv1.InfrastructureReadyCondition) + g.Expect(patchHelper.Patch(ctx, cluster)).To(Succeed()) + mhc := newMachineHealthCheck(cluster.Namespace, cluster.Name) // Create infrastructure template resource. infraResource := map[string]interface{}{ @@ -875,7 +1041,7 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { nodes, machines, cleanup := createMachinesWithNodes(g, cluster, count(1), createNodeRefForMachine(true), - markNodeAsHealthy(true), + nodeStatus(corev1.ConditionTrue), machineLabels(mhc.Spec.Selector.MatchLabels), ) defer cleanup() @@ -1459,12 +1625,14 @@ func ownerReferenceForCluster(ctx context.Context, g *WithT, c *clusterv1.Cluste func createNamespaceAndCluster(g *WithT) *clusterv1.Cluster { ns, err := testEnv.CreateNamespace(ctx, "test-mhc") g.Expect(err).ToNot(HaveOccurred()) + cluster := &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-cluster-", Namespace: ns.Name, }, } + g.Expect(testEnv.Create(ctx, cluster)).To(Succeed()) g.Eventually(func() error { var cl clusterv1.Cluster @@ -1503,28 +1671,6 @@ func newRunningMachine(c *clusterv1.Cluster, labels map[string]string) *clusterv } } -// newNode creaetes a Node object with node condition Ready == True -func newNode() *corev1.Node { - return &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-mhc-node-", - }, - Status: corev1.NodeStatus{ - Conditions: []corev1.NodeCondition{ - {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, - }, - }, - } -} - -func setNodeUnhealthy(node *corev1.Node) { - node.Status.Conditions[0] = corev1.NodeCondition{ - Type: corev1.NodeReady, - Status: corev1.ConditionUnknown, - LastTransitionTime: metav1.NewTime(time.Now().Add(-10 * time.Minute)), - } -} - func newInfraMachine(machine *clusterv1.Machine) (*unstructured.Unstructured, string) { providerID := fmt.Sprintf("test:////%v", uuid.NewUUID()) return &unstructured.Unstructured{ @@ -1544,9 +1690,11 @@ func newInfraMachine(machine *clusterv1.Machine) (*unstructured.Unstructured, st type machinesWithNodes struct { count int - markNodeAsHealthy bool + nodeStatus corev1.ConditionStatus createNodeRefForMachine bool labels map[string]string + failureReason string + failureMessage string } type machineWithNodesOption func(m *machinesWithNodes) @@ -1557,9 +1705,9 @@ func count(n int) machineWithNodesOption { } } -func markNodeAsHealthy(b bool) machineWithNodesOption { +func nodeStatus(s corev1.ConditionStatus) machineWithNodesOption { return func(m *machinesWithNodes) { - m.markNodeAsHealthy = b + m.nodeStatus = s } } @@ -1575,6 +1723,18 @@ func machineLabels(l map[string]string) machineWithNodesOption { } } +func machineFailureReason(s string) machineWithNodesOption { + return func(m *machinesWithNodes) { + m.failureReason = s + } +} + +func machineFailureMessage(s string) machineWithNodesOption { + return func(m *machinesWithNodes) { + m.failureMessage = s + } +} + func createMachinesWithNodes( g *WithT, c *clusterv1.Cluster, @@ -1627,36 +1787,61 @@ func createMachinesWithNodes( return machine.Status.LastUpdated }, timeout, 100*time.Millisecond).ShouldNot(BeNil()) + machinePatchHelper, err := patch.NewHelper(machine, testEnv.Client) + g.Expect(err).To(BeNil()) + if o.createNodeRefForMachine { - node := newNode() - if !o.markNodeAsHealthy { - setNodeUnhealthy(node) + // Create node + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-mhc-node-", + }, + Spec: corev1.NodeSpec{ + ProviderID: providerID, + }, } - machineStatus := machine.Status - node.Spec.ProviderID = providerID - nodeStatus := node.Status + g.Expect(testEnv.Create(ctx, node)).To(Succeed()) fmt.Printf("node created: %s\n", node.GetName()) - nodePatch := client.MergeFrom(node.DeepCopy()) - node.Status = nodeStatus - g.Expect(testEnv.Status().Patch(ctx, node, nodePatch)).To(Succeed()) + // Patch node status + nodePatchHelper, err := patch.NewHelper(node, testEnv.Client) + g.Expect(err).To(BeNil()) + + node.Status.Conditions = []corev1.NodeCondition{ + { + Type: corev1.NodeReady, + Status: o.nodeStatus, + LastTransitionTime: metav1.NewTime(time.Now().Add(-10 * time.Minute)), + }, + } + + g.Expect(nodePatchHelper.Patch(ctx, node)).To(Succeed()) + nodes = append(nodes, node) - machinePatch := client.MergeFrom(machine.DeepCopy()) - machine.Status = machineStatus machine.Status.NodeRef = &corev1.ObjectReference{ Name: node.Name, } + } - // Adding one second to ensure there is a difference from the - // original time so that the patch works. That is, ensure the - // precision isn't lost during conversions. - lastUp := metav1.NewTime(machine.Status.LastUpdated.Add(time.Second)) - machine.Status.LastUpdated = &lastUp - g.Expect(testEnv.Status().Patch(ctx, machine, machinePatch)).To(Succeed()) + if o.failureReason != "" { + failureReason := capierrors.MachineStatusError(o.failureReason) + machine.Status.FailureReason = &failureReason + } + if o.failureMessage != "" { + machine.Status.FailureMessage = pointer.StringPtr(o.failureMessage) } + // Adding one second to ensure there is a difference from the + // original time so that the patch works. That is, ensure the + // precision isn't lost during conversions. + lastUp := metav1.NewTime(machine.Status.LastUpdated.Add(time.Second)) + machine.Status.LastUpdated = &lastUp + + // Patch the machine to record the status changes + g.Expect(machinePatchHelper.Patch(ctx, machine)).To(Succeed()) + machines = append(machines, machine) } diff --git a/controllers/machinehealthcheck_targets.go b/controllers/machinehealthcheck_targets.go index 0069cdea1790..960d7f581dd4 100644 --- a/controllers/machinehealthcheck_targets.go +++ b/controllers/machinehealthcheck_targets.go @@ -58,6 +58,7 @@ const ( // healthCheckTarget contains the information required to perform a health check // on the node to determine if any remediation is required. type healthCheckTarget struct { + Cluster *clusterv1.Cluster Machine *clusterv1.Machine Node *corev1.Node MHC *clusterv1.MachineHealthCheck @@ -116,16 +117,22 @@ func (t *healthCheckTarget) needsRemediation(logger logr.Logger, timeoutForMachi // the node has not been set yet if t.Node == nil { - // status not updated yet - if t.Machine.Status.LastUpdated == nil { - return false, timeoutForMachineToHaveNode + // TODO change to checking ControlPlaneReadyCondition in v1alpha4, when cluster.spec.controlPlaneRef will be required. + // We can't do this yet because ControlPlaneReadyCondition is only set when you're using a control plane provider, + // and that is optional in v1alpha3. + if !conditions.Has(t.Cluster, clusterv1.InfrastructureReadyCondition) || conditions.IsFalse(t.Cluster, clusterv1.InfrastructureReadyCondition) { + // Cluster infrastructure is not ready yet, return a nextCheck time of 0 because we'll get requeued when + // the Cluster is updated. + return false, 0 } - if t.Machine.Status.LastUpdated.Add(timeoutForMachineToHaveNode).Before(now) { + + infraReadyTime := conditions.GetLastTransitionTime(t.Cluster, clusterv1.InfrastructureReadyCondition) + if infraReadyTime.Add(timeoutForMachineToHaveNode).Before(now) { conditions.MarkFalse(t.Machine, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.NodeStartupTimeoutReason, clusterv1.ConditionSeverityWarning, "Node failed to report startup in %s", timeoutForMachineToHaveNode.String()) logger.V(3).Info("Target is unhealthy: machine has no node", "duration", timeoutForMachineToHaveNode.String()) return true, time.Duration(0) } - durationUnhealthy := now.Sub(t.Machine.Status.LastUpdated.Time) + durationUnhealthy := now.Sub(infraReadyTime.Time) nextCheck := timeoutForMachineToHaveNode - durationUnhealthy + time.Second return false, nextCheck } @@ -168,6 +175,11 @@ func (r *MachineHealthCheckReconciler) getTargetsFromMHC(clusterClient client.Re return nil, nil } + var cluster clusterv1.Cluster + if err := clusterClient.Get(context.TODO(), client.ObjectKey{Namespace: mhc.Namespace, Name: mhc.Spec.ClusterName}, &cluster); err != nil { + return nil, errors.Wrapf(err, "error getting Cluster %s/%s for MachineHealthCheck %s", mhc.Namespace, mhc.Spec.ClusterName, mhc.Name) + } + targets := []healthCheckTarget{} for k := range machines { patchHelper, err := patch.NewHelper(&machines[k], r.Client) @@ -175,6 +187,7 @@ func (r *MachineHealthCheckReconciler) getTargetsFromMHC(clusterClient client.Re return nil, errors.Wrap(err, "unable to initialize patch helper") } target := healthCheckTarget{ + Cluster: &cluster, MHC: mhc, Machine: &machines[k], patchHelper: patchHelper, @@ -194,7 +207,7 @@ func (r *MachineHealthCheckReconciler) getTargetsFromMHC(clusterClient client.Re return targets, nil } -//getMachinesFromMHC fetches Machines matched by the MachineHealthCheck's +// getMachinesFromMHC fetches Machines matched by the MachineHealthCheck's // label selector func (r *MachineHealthCheckReconciler) getMachinesFromMHC(mhc *clusterv1.MachineHealthCheck) ([]clusterv1.Machine, error) { selector, err := metav1.LabelSelectorAsSelector(metav1.CloneSelectorAndAddLabel( diff --git a/controllers/machinehealthcheck_targets_test.go b/controllers/machinehealthcheck_targets_test.go index c35e42e95511..b176a2712488 100644 --- a/controllers/machinehealthcheck_targets_test.go +++ b/controllers/machinehealthcheck_targets_test.go @@ -21,149 +21,31 @@ import ( "time" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" - "sigs.k8s.io/cluster-api/util/patch" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/controller-runtime/pkg/log" ) -func TestGetTargetsFromMHC(t *testing.T) { +func TestHealthCheckTargets(t *testing.T) { namespace := "test-mhc" clusterName := "test-cluster" - mhcSelector := map[string]string{"cluster": clusterName, "machine-group": "foo"} - // Create a namespace for the tests - testNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "mhc-test"}} - - // Create a test MHC - testMHC := &clusterv1.MachineHealthCheck{ + cluster := &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-mhc", Namespace: namespace, - }, - Spec: clusterv1.MachineHealthCheckSpec{ - ClusterName: clusterName, - Selector: metav1.LabelSelector{ - MatchLabels: mhcSelector, - }, - UnhealthyConditions: []clusterv1.UnhealthyCondition{ - { - Type: corev1.NodeReady, - Status: corev1.ConditionUnknown, - Timeout: metav1.Duration{Duration: 5 * time.Minute}, - }, - }, + Name: clusterName, }, } + conditions.MarkTrue(cluster, clusterv1.InfrastructureReadyCondition) - baseObjects := []runtime.Object{testNS, testMHC} - - // Initialise some test machines and nodes for use in the test cases - - testNode1 := newTestNode("node1") - testMachine1 := newTestMachine("machine1", namespace, clusterName, testNode1.Name, mhcSelector) - testNode2 := newTestNode("node2") - testMachine2 := newTestMachine("machine2", namespace, clusterName, testNode2.Name, map[string]string{"cluster": clusterName}) - testNode3 := newTestNode("node3") - testMachine3 := newTestMachine("machine3", namespace, clusterName, testNode3.Name, mhcSelector) - testNode4 := newTestNode("node4") - testMachine4 := newTestMachine("machine4", namespace, "other-cluster", testNode4.Name, mhcSelector) - - testCases := []struct { - desc string - toCreate []runtime.Object - expectedTargets []healthCheckTarget - }{ - { - desc: "with no matching machines", - toCreate: baseObjects, - expectedTargets: nil, - }, - { - desc: "when a machine's node is missing", - toCreate: append(baseObjects, testMachine1), - expectedTargets: []healthCheckTarget{ - { - Machine: testMachine1, - MHC: testMHC, - Node: nil, - nodeMissing: true, - }, - }, - }, - { - desc: "when a machine's labels do not match the selector", - toCreate: append(baseObjects, testMachine1, testMachine2, testNode1), - expectedTargets: []healthCheckTarget{ - { - Machine: testMachine1, - MHC: testMHC, - Node: testNode1, - }, - }, - }, - { - desc: "with multiple machines, should match correct nodes", - toCreate: append(baseObjects, testNode1, testMachine1, testNode3, testMachine3, testNode4, testMachine4), - expectedTargets: []healthCheckTarget{ - { - Machine: testMachine1, - MHC: testMHC, - Node: testNode1, - }, - { - Machine: testMachine3, - MHC: testMHC, - Node: testNode3, - }, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - gs := NewGomegaWithT(t) - - gs.Expect(clusterv1.AddToScheme(scheme.Scheme)).To(Succeed()) - k8sClient := fake.NewFakeClientWithScheme(scheme.Scheme, tc.toCreate...) - - // Create a test reconciler - reconciler := &MachineHealthCheckReconciler{ - Client: k8sClient, - Log: log.Log, - scheme: scheme.Scheme, - } - for _, t := range tc.expectedTargets { - patchHelper, err := patch.NewHelper(t.Machine, k8sClient) - gs.Expect(err).ToNot(HaveOccurred()) - t.patchHelper = patchHelper - } - - targets, err := reconciler.getTargetsFromMHC(k8sClient, testMHC) - gs.Expect(err).ToNot(HaveOccurred()) - - gs.Expect(len(targets)).To(Equal(len(tc.expectedTargets))) - for i, target := range targets { - expectedTarget := tc.expectedTargets[i] - gs.Expect(target.Machine).To(Equal(expectedTarget.Machine)) - gs.Expect(target.MHC).To(Equal(expectedTarget.MHC)) - gs.Expect(target.Node).To(Equal(expectedTarget.Node)) - } - }) - } -} - -func TestHealthCheckTargets(t *testing.T) { - namespace := "test-mhc" - clusterName := "test-cluster" mhcSelector := map[string]string{"cluster": clusterName, "machine-group": "foo"} + timeoutForMachineToHaveNode := 10 * time.Minute + // Create a test MHC testMHC := &clusterv1.MachineHealthCheck{ ObjectMeta: metav1.ObjectMeta{ @@ -198,6 +80,7 @@ func TestHealthCheckTargets(t *testing.T) { testMachineLastUpdated400s.Status.LastUpdated = &nowMinus400s nodeNotYetStartedTarget := healthCheckTarget{ + Cluster: cluster, MHC: testMHC, Machine: testMachineLastUpdated400s, Node: nil, @@ -205,6 +88,7 @@ func TestHealthCheckTargets(t *testing.T) { // Target for when the Node has been seen, but has now gone nodeGoneAway := healthCheckTarget{ + Cluster: cluster, MHC: testMHC, Machine: testMachine, Node: &corev1.Node{}, @@ -214,6 +98,7 @@ func TestHealthCheckTargets(t *testing.T) { // Target for when the node has been in an unknown state for shorter than the timeout testNodeUnknown200 := newTestUnhealthyNode("node1", corev1.NodeReady, corev1.ConditionUnknown, 200*time.Second) nodeUnknown200 := healthCheckTarget{ + Cluster: cluster, MHC: testMHC, Machine: testMachine, Node: testNodeUnknown200, @@ -223,6 +108,7 @@ func TestHealthCheckTargets(t *testing.T) { // Second Target for when the node has been in an unknown state for shorter than the timeout testNodeUnknown100 := newTestUnhealthyNode("node1", corev1.NodeReady, corev1.ConditionUnknown, 100*time.Second) nodeUnknown100 := healthCheckTarget{ + Cluster: cluster, MHC: testMHC, Machine: testMachine, Node: testNodeUnknown100, @@ -232,6 +118,7 @@ func TestHealthCheckTargets(t *testing.T) { // Target for when the node has been in an unknown state for longer than the timeout testNodeUnknown400 := newTestUnhealthyNode("node1", corev1.NodeReady, corev1.ConditionUnknown, 400*time.Second) nodeUnknown400 := healthCheckTarget{ + Cluster: cluster, MHC: testMHC, Machine: testMachine, Node: testNodeUnknown400, @@ -242,6 +129,7 @@ func TestHealthCheckTargets(t *testing.T) { testNodeHealthy := newTestNode("node1") testNodeHealthy.UID = "12345" nodeHealthy := healthCheckTarget{ + Cluster: cluster, MHC: testMHC, Machine: testMachine, Node: testNodeHealthy, @@ -260,7 +148,7 @@ func TestHealthCheckTargets(t *testing.T) { targets: []healthCheckTarget{nodeNotYetStartedTarget}, expectedHealthy: []healthCheckTarget{}, expectedNeedsRemediation: []healthCheckTarget{}, - expectedNextCheckTimes: []time.Duration{200 * time.Second}, + expectedNextCheckTimes: []time.Duration{timeoutForMachineToHaveNode}, }, { desc: "when the node has gone away", @@ -301,20 +189,15 @@ func TestHealthCheckTargets(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - gs := NewGomegaWithT(t) - - gs.Expect(clusterv1.AddToScheme(scheme.Scheme)).To(Succeed()) - k8sClient := fake.NewFakeClientWithScheme(scheme.Scheme) + gs := NewWithT(t) - // Create a test reconciler + // Create a test reconciler. reconciler := &MachineHealthCheckReconciler{ - Client: k8sClient, Log: log.Log, scheme: scheme.Scheme, recorder: record.NewFakeRecorder(5), } - timeoutForMachineToHaveNode := 10 * time.Minute healthy, unhealthy, nextCheckTimes := reconciler.healthCheckTargets(tc.targets, reconciler.Log, timeoutForMachineToHaveNode) // Round durations down to nearest second account for minute differences @@ -356,7 +239,7 @@ func newTestMachine(name, namespace, clusterName, nodeName string, labels map[st Spec: clusterv1.MachineSpec{ ClusterName: clusterName, Bootstrap: clusterv1.Bootstrap{ - Data: &bootstrap, + DataSecretName: &bootstrap, }, }, Status: clusterv1.MachineStatus{ diff --git a/test/helpers/envtest.go b/test/helpers/envtest.go index 88613a2eda04..f806934ce539 100644 --- a/test/helpers/envtest.go +++ b/test/helpers/envtest.go @@ -213,7 +213,7 @@ func appendWebhookConfiguration(mutatingWebhooks []runtime.Object, validatingWeb } } if o.GetKind() == validatingWebhookKind { - // update the name in metadata + // update the name in metadat if o.GetName() == validatingwebhook { o.SetName(strings.Join([]string{validatingwebhook, "-", tag}, "")) validatingWebhooks = append(validatingWebhooks, &o)