diff --git a/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go b/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go index 089842967c..e118da8d9f 100644 --- a/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go +++ b/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go @@ -3,6 +3,7 @@ package machinehealthcheck import ( "context" golangerrors "errors" + "fmt" "time" "github.com/golang/glog" @@ -37,11 +38,12 @@ const ( // Add creates a new MachineHealthCheck Controller and adds it to the Manager. The Manager will set fields on the Controller // and start it when the Manager is started. func Add(mgr manager.Manager, opts manager.Options) error { - return add(mgr, newReconciler(mgr, opts)) + r := newReconciler(mgr, opts) + return add(mgr, r, r.nodeRequestsFromMachineHealthCheck) } // newReconciler returns a new reconcile.Reconciler -func newReconciler(mgr manager.Manager, opts manager.Options) reconcile.Reconciler { +func newReconciler(mgr manager.Manager, opts manager.Options) *ReconcileMachineHealthCheck { return &ReconcileMachineHealthCheck{ client: mgr.GetClient(), scheme: mgr.GetScheme(), @@ -50,12 +52,21 @@ func newReconciler(mgr manager.Manager, opts manager.Options) reconcile.Reconcil } // add adds a new Controller to mgr with r as the reconcile.Reconciler -func add(mgr manager.Manager, r reconcile.Reconciler) error { +func add(mgr manager.Manager, r reconcile.Reconciler, mapFn handler.ToRequestsFunc) error { // Create a new controller c, err := controller.New("machinehealthcheck-controller", mgr, controller.Options{Reconciler: r}) if err != nil { return err } + + // Watch MachineHealthChecks and enqueue reconcile.Request for the backed nodes. + // This is useful to trigger remediation when a machineHealCheck is created against + // a node which is already unhealthy and is not able to receive status updates. + err = c.Watch(&source.Kind{Type: &healthcheckingv1alpha1.MachineHealthCheck{}}, &handler.EnqueueRequestsFromMapFunc{ToRequests: mapFn}) + if err != nil { + return err + } + return c.Watch(&source.Kind{Type: &corev1.Node{}}, &handler.EnqueueRequestForObject{}) } @@ -142,6 +153,78 @@ func (r *ReconcileMachineHealthCheck) Reconcile(request reconcile.Request) (reco return reconcile.Result{}, nil } +func (r *ReconcileMachineHealthCheck) nodeRequestsFromMachineHealthCheck(o handler.MapObject) []reconcile.Request { + glog.V(3).Infof("Watched machineHealthCheck event, finding nodes to reconcile.Request...") + mhc := &healthcheckingv1alpha1.MachineHealthCheck{} + if err := r.client.Get( + context.Background(), + client.ObjectKey{ + Namespace: o.Meta.GetNamespace(), + Name: o.Meta.GetName(), + }, + mhc, + ); err != nil { + glog.Errorf("No-op: Unable to retrieve mhc %s/%s from store: %v", o.Meta.GetNamespace(), o.Meta.GetName(), err) + return []reconcile.Request{} + } + + if mhc.DeletionTimestamp != nil { + glog.V(3).Infof("No-op: mhc %q is being deleted", o.Meta.GetName()) + return []reconcile.Request{} + } + + // get nodes covered by then mhc + nodeNames, err := r.getNodeNamesForMHC(*mhc) + if err != nil { + glog.Errorf("No-op: failed to get nodes for mhc %q", o.Meta.GetName()) + return []reconcile.Request{} + } + if nodeNames != nil { + var requests []reconcile.Request + for _, nodeName := range nodeNames { + // convert to namespacedName to satisfy type Request struct + nodeNamespacedName := client.ObjectKey{ + Name: string(nodeName), + } + requests = append(requests, reconcile.Request{NamespacedName: nodeNamespacedName}) + } + return requests + } + return []reconcile.Request{} +} + +func (r *ReconcileMachineHealthCheck) getNodeNamesForMHC(mhc healthcheckingv1alpha1.MachineHealthCheck) ([]types.NodeName, error) { + machineList := &mapiv1.MachineList{} + selector, err := metav1.LabelSelectorAsSelector(&mhc.Spec.Selector) + if err != nil { + return nil, fmt.Errorf("failed to build selector") + } + options := client.ListOptions{ + LabelSelector: selector, + } + + if err := r.client.List(context.Background(), + machineList, + client.UseListOptions(options.InNamespace(mhc.GetNamespace()))); err != nil { + return nil, fmt.Errorf("failed to list machines: %v", err) + } + + if len(machineList.Items) < 1 { + return nil, nil + } + + var nodeNames []types.NodeName + for _, machine := range machineList.Items { + if machine.Status.NodeRef != nil { + nodeNames = append(nodeNames, types.NodeName(machine.Status.NodeRef.Name)) + } + } + if len(nodeNames) < 1 { + return nil, nil + } + return nodeNames, nil +} + // This is set so the fake client can be used for unit test. See: // https://github.com/kubernetes-sigs/controller-runtime/issues/168 func getMachineHealthCheckListOptions() *client.ListOptions { diff --git a/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go b/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go index 97471b0eb3..46672695e9 100644 --- a/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go +++ b/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go @@ -7,7 +7,11 @@ import ( "testing" "time" - mapiv1alpha1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "sigs.k8s.io/controller-runtime/pkg/handler" + + mapiv1beta1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1" healthcheckingv1alpha1 "github.com/openshift/machine-api-operator/pkg/apis/healthchecking/v1alpha1" "github.com/openshift/machine-api-operator/pkg/util/conditions" maotesting "github.com/openshift/machine-api-operator/pkg/util/testing" @@ -31,14 +35,14 @@ const ( func init() { // Add types to scheme - mapiv1alpha1.AddToScheme(scheme.Scheme) + mapiv1beta1.AddToScheme(scheme.Scheme) healthcheckingv1alpha1.AddToScheme(scheme.Scheme) } func TestHasMatchingLabels(t *testing.T) { machine := maotesting.NewMachine("machine", "node") testsCases := []struct { - machine *mapiv1alpha1.Machine + machine *mapiv1beta1.Machine machineHealthCheck *healthcheckingv1alpha1.MachineHealthCheck expected bool }{ @@ -180,7 +184,7 @@ func testReconcile(t *testing.T, remediationWaitTime time.Duration, initObjects machineUnhealthyForTooLong := maotesting.NewMachine("machineUnhealthyForTooLong", nodeUnhealthyForTooLong.Name) testsCases := []struct { - machine *mapiv1alpha1.Machine + machine *mapiv1beta1.Machine node *corev1.Node remediationStrategy healthcheckingv1alpha1.RemediationStrategyType expected expectedReconcile @@ -312,7 +316,7 @@ func TestHasMachineSetOwner(t *testing.T) { machineWithNoMachineSet.OwnerReferences = nil testsCases := []struct { - machine *mapiv1alpha1.Machine + machine *mapiv1beta1.Machine expected bool }{ { @@ -385,3 +389,145 @@ func TestApplyRemediationReboot(t *testing.T) { t.Errorf("Expected: node to have reboot annotion %s, got: %v", machineRebootAnnotationKey, node.Annotations) } } + +func TestGetNodeNamesForMHC(t *testing.T) { + testCases := []struct { + mhc healthcheckingv1alpha1.MachineHealthCheck + machines []*mapiv1beta1.Machine + expectedNodeNames []types.NodeName + }{ + { + mhc: *maotesting.NewMachineHealthCheck("matchNodes"), + machines: []*mapiv1beta1.Machine{ + maotesting.NewMachine("test", "node1"), + maotesting.NewMachine("test2", "node2"), + }, + expectedNodeNames: []types.NodeName{ + "node1", + "node2", + }, + }, + { + mhc: *&healthcheckingv1alpha1.MachineHealthCheck{ + ObjectMeta: metav1.ObjectMeta{ + Name: "noMatchingMachines", + Namespace: namespace, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "MachineHealthCheck", + }, + Spec: healthcheckingv1alpha1.MachineHealthCheckSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "no": "match", + }, + }, + }, + Status: healthcheckingv1alpha1.MachineHealthCheckStatus{}, + }, + machines: []*mapiv1beta1.Machine{ + maotesting.NewMachine("test", "node1"), + maotesting.NewMachine("test2", "node2"), + }, + expectedNodeNames: nil, + }, + { + mhc: *maotesting.NewMachineHealthCheck("noNodeRefs"), + machines: []*mapiv1beta1.Machine{ + maotesting.NewMachine("test", ""), + maotesting.NewMachine("test2", ""), + }, + expectedNodeNames: nil, + }, + } + for _, tc := range testCases { + objects := []runtime.Object{} + for i := range tc.machines { + objects = append(objects, runtime.Object(tc.machines[i])) + } + r := newFakeReconciler(objects...) + nodeNames, err := r.getNodeNamesForMHC(tc.mhc) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(nodeNames, tc.expectedNodeNames) { + t.Errorf("Expected: %v, got: %v", tc.expectedNodeNames, nodeNames) + } + } +} + +func TestNodeRequestsFromMachineHealthCheck(t *testing.T) { + testCases := []struct { + mhc healthcheckingv1alpha1.MachineHealthCheck + machines []*mapiv1beta1.Machine + expectedRequests []reconcile.Request + }{ + { + mhc: *maotesting.NewMachineHealthCheck("matchNodes"), + machines: []*mapiv1beta1.Machine{ + maotesting.NewMachine("test", "node1"), + maotesting.NewMachine("test2", "node2"), + }, + expectedRequests: []reconcile.Request{ + { + NamespacedName: client.ObjectKey{ + Name: string("node1"), + }, + }, + { + NamespacedName: client.ObjectKey{ + Name: string("node2"), + }, + }, + }, + }, + { + mhc: *&healthcheckingv1alpha1.MachineHealthCheck{ + ObjectMeta: metav1.ObjectMeta{ + Name: "noMatchingMachines", + Namespace: namespace, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "MachineHealthCheck", + }, + Spec: healthcheckingv1alpha1.MachineHealthCheckSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "no": "match", + }, + }, + }, + Status: healthcheckingv1alpha1.MachineHealthCheckStatus{}, + }, + machines: []*mapiv1beta1.Machine{ + maotesting.NewMachine("test", "node1"), + maotesting.NewMachine("test2", "node2"), + }, + expectedRequests: []reconcile.Request{}, + }, + { + mhc: *maotesting.NewMachineHealthCheck("noNodeRefs"), + machines: []*mapiv1beta1.Machine{ + maotesting.NewMachine("test", ""), + maotesting.NewMachine("test2", ""), + }, + expectedRequests: []reconcile.Request{}, + }, + } + for _, tc := range testCases { + objects := []runtime.Object{} + for i := range tc.machines { + objects = append(objects, runtime.Object(tc.machines[i])) + } + objects = append(objects, runtime.Object(&tc.mhc)) + r := newFakeReconciler(objects...) + o := handler.MapObject{ + Meta: tc.mhc.GetObjectMeta(), + Object: &tc.mhc, + } + requests := r.nodeRequestsFromMachineHealthCheck(o) + if !reflect.DeepEqual(requests, tc.expectedRequests) { + t.Errorf("Expected: %v, got: %v", tc.expectedRequests, requests) + } + } +} diff --git a/pkg/util/testing/testing.go b/pkg/util/testing/testing.go index a0cae8cf3c..2e9fe5effc 100644 --- a/pkg/util/testing/testing.go +++ b/pkg/util/testing/testing.go @@ -103,7 +103,7 @@ func NewNode(name string, ready bool) *corev1.Node { // NewMachine returns new machine object that can be used for testing func NewMachine(name string, nodeName string) *mapiv1.Machine { - return &mapiv1.Machine{ + m := &mapiv1.Machine{ TypeMeta: metav1.TypeMeta{Kind: "Machine"}, ObjectMeta: metav1.ObjectMeta{ Annotations: make(map[string]string), @@ -114,13 +114,16 @@ func NewMachine(name string, nodeName string) *mapiv1.Machine { OwnerReferences: []metav1.OwnerReference{{Kind: "MachineSet"}}, }, Spec: mapiv1.MachineSpec{}, - Status: mapiv1.MachineStatus{ + } + if nodeName != "" { + m.Status = mapiv1.MachineStatus{ NodeRef: &corev1.ObjectReference{ Name: nodeName, Namespace: metav1.NamespaceNone, }, - }, + } } + return m } // NewMachineHealthCheck returns new MachineHealthCheck object that can be used for testing