review fixes

internal/controllers/machine/machine_controller.go

@@ -104,6 +104,10 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 	if err != nil {
 		return err
 	}
+	mdToMachines, err := util.MachineDeploymentToObjectsMapper(mgr.GetClient(), &clusterv1.MachineList{}, mgr.GetScheme())
+	if err != nil {
+		return err
+	}
 
 	if r.nodeDeletionRetryTimeout.Nanoseconds() == 0 {
 		r.nodeDeletionRetryTimeout = 10 * time.Second
@@ -130,6 +134,10 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 			&clusterv1.MachineSet{},
 			handler.EnqueueRequestsFromMapFunc(msToMachines),
 		).
+		Watches(
+			&clusterv1.MachineSet{},
+			handler.EnqueueRequestsFromMapFunc(mdToMachines),
+		).
 		Build(r)
 	if err != nil {
 		return errors.Wrap(err, "failed setting up with a controller manager")

internal/controllers/machine/machine_controller_noderef.go

@@ -25,6 +25,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -257,8 +258,6 @@ func (r *Reconciler) getNode(ctx context.Context, c client.Reader, providerID st
 // PatchNode is required to workaround an issue on Node.Status.Address which is incorrectly annotated as patchStrategy=merge
 // and this causes SSA patch to fail in case there are two addresses with the same key https://github.com/kubernetes-sigs/cluster-api/issues/8417
 func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, node *corev1.Node, newLabels, newAnnotations map[string]string, m *clusterv1.Machine) error {
-	log := ctrl.LoggerFrom(ctx)
-
 	newNode := node.DeepCopy()
 
 	// Adds the annotations CAPI sets on the node.
@@ -298,9 +297,9 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client,
 	// Set Taint to a node in an old machineDeployment and unset Taint from a node in a new machineDeployment
 	// Ignore errors as it's not critical for the reconcile.
 	// To avoid an unnecessary Taint remaining due to the error remove Taint when errors occur.
-	isOutdated, err := isNodeOutdated(ctx, r.Client, m)
+	isOutdated, err := shouldNodeHaveOutdatedTaint(ctx, r.Client, m)
 	if err != nil {
-		log.V(2).Info("Failed to check if Node %s is outdated", "err", err, "node name", node.Name)
+		return errors.Wrapf(err, "failed to check if Node %s is outdated", klog.KRef("", node.Name))
 	}
 	if isOutdated {
 		hasTaintChanges = taints.EnsureNodeTaint(newNode, clusterv1.NodeOutdatedRevisionTaint) || hasTaintChanges
@@ -315,21 +314,30 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client,
 	return remoteClient.Patch(ctx, newNode, client.StrategicMergeFrom(node))
 }
 
-func isNodeOutdated(ctx context.Context, c client.Client, m *clusterv1.Machine) (bool, error) {
-	ms := &clusterv1.MachineSet{}
-	objKey := client.ObjectKey{
-		Namespace: m.ObjectMeta.Namespace,
-		Name:      m.Labels[clusterv1.MachineSetNameLabel],
+func shouldNodeHaveOutdatedTaint(ctx context.Context, c client.Client, m *clusterv1.Machine) (bool, error) {
+	if _, hasLabel := m.Labels[clusterv1.MachineSetNameLabel]; !hasLabel {
+		return false, nil
+	}
+	if _, hasLabel := m.Labels[clusterv1.MachineDeploymentNameLabel]; !hasLabel {
+		return false, nil
+	}
+
+	// Resolve the MachineSet name via owner references because the label value
+	// could also be a hash.
+	objKey, err := getOwnerMachineSetObjectKey(m.ObjectMeta)
+	if err != nil {
+		return false, err
 	}
-	if err := c.Get(ctx, objKey, ms); err != nil {
+	ms := &clusterv1.MachineSet{}
+	if err := c.Get(ctx, *objKey, ms); err != nil {
 		return false, err
 	}
 	md := &clusterv1.MachineDeployment{}
-	objKey = client.ObjectKey{
+	objKey = &client.ObjectKey{
 		Namespace: m.ObjectMeta.Namespace,
 		Name:      m.Labels[clusterv1.MachineDeploymentNameLabel],
 	}
-	if err := c.Get(ctx, objKey, md); err != nil {
+	if err := c.Get(ctx, *objKey, md); err != nil {
 		return false, err
 	}
 	msRev, err := mdutil.Revision(ms)
@@ -345,3 +353,16 @@ func isNodeOutdated(ctx context.Context, c client.Client, m *clusterv1.Machine)
 	}
 	return false, nil
 }
+
+func getOwnerMachineSetObjectKey(obj metav1.ObjectMeta) (*client.ObjectKey, error) {
+	for _, ref := range obj.GetOwnerReferences() {
+		gv, err := schema.ParseGroupVersion(ref.APIVersion)
+		if err != nil {
+			return nil, err
+		}
+		if ref.Kind == "MachineSet" && gv.Group == clusterv1.GroupVersion.Group {
+			return &client.ObjectKey{Namespace: obj.Namespace, Name: ref.Name}, nil
+		}
+	}
+	return nil, errors.Errorf("failed to find MachineSet owner reference for Machine %s", klog.KRef(obj.GetNamespace(), obj.GetName()))
+}

internal/controllers/machine/machine_controller_noderef_test.go

@@ -793,6 +793,12 @@ func TestPatchNode(t *testing.T) {
 						clusterv1.MachineSetNameLabel:        "test-ms-outdated",
 						clusterv1.MachineDeploymentNameLabel: "test-md-outdated",
 					},
+					OwnerReferences: []metav1.OwnerReference{{
+						Kind:       "MachineSet",
+						Name:       "test-ms-outdated",
+						APIVersion: clusterv1.GroupVersion.String(),
+						UID:        "uid",
+					}},
 				},
 				Spec: newFakeMachineSpec(metav1.NamespaceDefault, clusterName),
 			},
@@ -853,6 +859,12 @@ func TestPatchNode(t *testing.T) {
 						clusterv1.MachineSetNameLabel:        "test-ms-not-outdated",
 						clusterv1.MachineDeploymentNameLabel: "test-md-not-outdated",
 					},
+					OwnerReferences: []metav1.OwnerReference{{
+						Kind:       "MachineSet",
+						Name:       "test-ms-not-outdated",
+						APIVersion: clusterv1.GroupVersion.String(),
+						UID:        "uid",
+					}},
 				},
 				Spec: newFakeMachineSpec(metav1.NamespaceDefault, clusterName),
 			},

internal/test/builder/builders.go

@@ -1776,6 +1776,7 @@ func (m *MachineSetBuilder) Build() *clusterv1.MachineSet {
 		},
 	}
 	obj.Spec.ClusterName = m.clusterName
+	obj.Spec.Template.Spec.ClusterName = m.clusterName
 	obj.Spec.Replicas = m.replicas
 	if m.bootstrapTemplate != nil {
 		obj.Spec.Template.Spec.Bootstrap.ConfigRef = objToRef(m.bootstrapTemplate)