Let MHC to remediate any machine owned by a controller

Fix https://bugzilla.redhat.com/show_bug.cgi?id=1816398
We want to relax the hard dependency on machineSets and let remediation to happen for machines that are owned by any controller.
This will enable a possible integration for a self healing control plane.

Author: enxebre

pkg/controller/machinehealthcheck/machinehealthcheck_controller.go

@@ -33,7 +33,6 @@ import (
 const (
 	machineAnnotationKey          = "machine.openshift.io/machine"
 	machineExternalAnnotationKey  = "host.metal3.io/external-remediation"
-	ownerControllerKind           = "MachineSet"
 	nodeMasterLabel               = "node-role.kubernetes.io/master"
 	machineRoleLabel              = "machine.openshift.io/cluster-api-machine-role"
 	machineMasterRole             = "master"
@@ -436,8 +435,8 @@ func (r *ReconcileMachineHealthCheck) mhcRequestsFromMachine(o handler.MapObject
 
 func (t *target) remediate(r *ReconcileMachineHealthCheck) error {
 	glog.Infof(" %s: start remediation logic", t.string())
-	if !t.hasMachineSetOwner() {
-		glog.Infof("%s: no machineSet controller owner, skipping remediation", t.string())
+	if !t.hasControllerOwner() {
+		glog.Infof("%s: no controller owner, skipping remediation", t.string())
 		return nil
 	}
 
@@ -595,14 +594,8 @@ func (t *target) needsRemediation(timeoutForMachineToHaveNode time.Duration) (bo
 	return false, minDuration(nextCheckTimes), nil
 }
 
-func (t *target) hasMachineSetOwner() bool {
-	ownerRefs := t.Machine.ObjectMeta.GetOwnerReferences()
-	for _, or := range ownerRefs {
-		if or.Kind == ownerControllerKind {
-			return true
-		}
-	}
-	return false
+func (t *target) hasControllerOwner() bool {
+	return metav1.GetControllerOf(&t.Machine) != nil
 }
 
 func derefStringPointer(stringPointer *string) string {

pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go

@@ -20,6 +20,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -364,11 +365,27 @@ func TestReconcile(t *testing.T) {
 	}
 }
 
-func TestHasMachineSetOwner(t *testing.T) {
+func TestHasControllerOwner(t *testing.T) {
 	machineWithMachineSet := maotesting.NewMachine("machineWithMachineSet", "node")
+
 	machineWithNoMachineSet := maotesting.NewMachine("machineWithNoMachineSet", "node")
 	machineWithNoMachineSet.OwnerReferences = nil
 
+	machineWithAnyControllerOwner := maotesting.NewMachine("machineWithAnyControllerOwner", "node")
+	machineWithAnyControllerOwner.OwnerReferences = []metav1.OwnerReference{
+		{
+			Kind:       "Any",
+			Controller: pointer.BoolPtr(true),
+		},
+	}
+
+	machineWithNoControllerOwner := maotesting.NewMachine("machineWithNoControllerOwner", "node")
+	machineWithNoControllerOwner.OwnerReferences = []metav1.OwnerReference{
+		{
+			Kind: "Any",
+		},
+	}
+
 	testsCases := []struct {
 		target   target
 		expected bool
@@ -385,10 +402,22 @@ func TestHasMachineSetOwner(t *testing.T) {
 			},
 			expected: true,
 		},
+		{
+			target: target{
+				Machine: *machineWithAnyControllerOwner,
+			},
+			expected: true,
+		},
+		{
+			target: target{
+				Machine: *machineWithNoControllerOwner,
+			},
+			expected: false,
+		},
 	}
 
 	for _, tc := range testsCases {
-		if got := tc.target.hasMachineSetOwner(); got != tc.expected {
+		if got := tc.target.hasControllerOwner(); got != tc.expected {
 			t.Errorf("Test case: Machine %s. Expected: %t, got: %t", tc.target.Machine.Name, tc.expected, got)
 		}
 	}
@@ -1820,11 +1849,16 @@ func TestRemediate(t *testing.T) {
 						APIVersion: "machine.openshift.io/v1beta1",
 					},
 					ObjectMeta: metav1.ObjectMeta{
-						Annotations:     make(map[string]string),
-						Name:            "test",
-						Namespace:       namespace,
-						Labels:          map[string]string{"foo": "bar"},
-						OwnerReferences: []metav1.OwnerReference{{Kind: "MachineSet"}},
+						Annotations: make(map[string]string),
+						Name:        "test",
+						Namespace:   namespace,
+						Labels:      map[string]string{"foo": "bar"},
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind:       "MachineSet",
+								Controller: pointer.BoolPtr(true),
+							},
+						},
 					},
 					Spec:   mapiv1beta1.MachineSpec{},
 					Status: mapiv1beta1.MachineStatus{},
@@ -1858,12 +1892,17 @@ func TestRemediate(t *testing.T) {
 						APIVersion: "machine.openshift.io/v1beta1",
 					},
 					ObjectMeta: metav1.ObjectMeta{
-						Annotations:     make(map[string]string),
-						Name:            "test",
-						Namespace:       namespace,
-						Labels:          map[string]string{"foo": "bar"},
-						OwnerReferences: []metav1.OwnerReference{{Kind: "MachineSet"}},
-						UID:             "uid",
+						Annotations: make(map[string]string),
+						Name:        "test",
+						Namespace:   namespace,
+						Labels:      map[string]string{"foo": "bar"},
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind:       "MachineSet",
+								Controller: pointer.BoolPtr(true),
+							},
+						},
+						UID: "uid",
 					},
 					//Spec:   mapiv1beta1.MachineSpec{},
 					//Status: mapiv1beta1.MachineStatus{},
@@ -1905,7 +1944,12 @@ func TestRemediate(t *testing.T) {
 						Labels: map[string]string{
 							machineRoleLabel: machineMasterRole,
 						},
-						OwnerReferences: []metav1.OwnerReference{{Kind: "MachineSet"}},
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind:       "MachineSet",
+								Controller: pointer.BoolPtr(true),
+							},
+						},
 					},
 					Spec:   mapiv1beta1.MachineSpec{},
 					Status: mapiv1beta1.MachineStatus{},

pkg/util/testing/testing.go

@@ -8,6 +8,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/utils/pointer"
 )
 
 const (
@@ -74,12 +75,17 @@ func NewMachine(name string, nodeName string) *mapiv1.Machine {
 	m := &mapiv1.Machine{
 		TypeMeta: metav1.TypeMeta{Kind: "Machine"},
 		ObjectMeta: metav1.ObjectMeta{
-			Annotations:     make(map[string]string),
-			Name:            name,
-			Namespace:       Namespace,
-			Labels:          FooBar(),
-			UID:             uuid.NewUUID(),
-			OwnerReferences: []metav1.OwnerReference{{Kind: "MachineSet"}},
+			Annotations: make(map[string]string),
+			Name:        name,
+			Namespace:   Namespace,
+			Labels:      FooBar(),
+			UID:         uuid.NewUUID(),
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					Kind:       "MachineSet",
+					Controller: pointer.BoolPtr(true),
+				},
+			},
 		},
 		Spec: mapiv1.MachineSpec{},
 	}