Merge pull request kubernetes-sigs#9985 from adityabhatia/revertClusterWatcherForTopologyMD

🐛 Revert "Watch for Cluster resources in topology MD controller"

Author: k8s-ci-robot

internal/controllers/machinedeployment/machinedeployment_controller.go

@@ -92,9 +92,12 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 		Watches(
 			&clusterv1.Cluster{},
 			handler.EnqueueRequestsFromMapFunc(clusterToMachineDeployments),
-			// TODO: should this wait for Cluster.Status.InfrastructureReady similar to Infra Machine resources?
 			builder.WithPredicates(
-				predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
+				// TODO: should this wait for Cluster.Status.InfrastructureReady similar to Infra Machine resources?
+				predicates.All(ctrl.LoggerFrom(ctx),
+					predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
+					predicates.ResourceHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue),
+				),
 			),
 		).Complete(r)
 	if err != nil {

internal/controllers/topology/machinedeployment/machinedeployment_controller.go

@@ -24,11 +24,9 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/internal/controllers/topology/machineset"
@@ -57,27 +55,14 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	clusterToMachineDeployments, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &clusterv1.MachineDeploymentList{}, mgr.GetScheme())
-	if err != nil {
-		return err
-	}
-
-	err = ctrl.NewControllerManagedBy(mgr).
-		For(&clusterv1.MachineDeployment{},
-			builder.WithPredicates(
-				predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)),
-				predicates.ResourceNotPaused(ctrl.LoggerFrom(ctx)))).
+	err := ctrl.NewControllerManagedBy(mgr).
+		For(&clusterv1.MachineDeployment{}).
 		Named("topology/machinedeployment").
-		WithEventFilter(predicates.ResourceHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue)).
 		WithOptions(options).
-		Watches(
-			&clusterv1.Cluster{},
-			handler.EnqueueRequestsFromMapFunc(clusterToMachineDeployments),
-			builder.WithPredicates(
-				predicates.ClusterHasTopology(ctrl.LoggerFrom(ctx)),
-				predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
-			),
-		).
+		WithEventFilter(predicates.All(ctrl.LoggerFrom(ctx),
+			predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue),
+			predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)),
+		)).
 		Complete(r)
 	if err != nil {
 		return errors.Wrap(err, "failed setting up with a controller manager")

test/framework/finalizers_helpers.go

@@ -24,6 +24,7 @@ import (
 
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -66,6 +67,10 @@ func ValidateFinalizersResilience(ctx context.Context, proxy ClusterProxy, names
 	// Unpause the cluster.
 	setClusterPause(ctx, proxy.GetClient(), clusterKey, false)
 
+	// Annotate the MachineDeployment  to speed up reconciliation. This ensures MachineDeployment topology Finalizers are re-reconciled.
+	// TODO: Remove this as part of https://github.com/kubernetes-sigs/cluster-api/issues/9532
+	forceMachineDeploymentTopologyReconcile(ctx, proxy.GetClient(), clusterKey)
+
 	// Check that the Finalizers are as expected after further reconciliations.
 	assertFinalizersExist(ctx, proxy, namespace, objectsWithFinalizers)
 }
@@ -143,3 +148,19 @@ func assertFinalizersExist(ctx context.Context, proxy ClusterProxy, namespace st
 		return kerrors.NewAggregate(allErrs)
 	}).WithTimeout(1 * time.Minute).WithPolling(2 * time.Second).Should(Succeed())
 }
+
+// forceMachineDeploymentTopologyReconcile forces reconciliation of the MachineDeployment.
+func forceMachineDeploymentTopologyReconcile(ctx context.Context, cli client.Client, clusterKey types.NamespacedName) {
+	mdList := &clusterv1.MachineDeploymentList{}
+	clientOptions := (&client.ListOptions{}).ApplyOptions([]client.ListOption{
+		client.MatchingLabels{clusterv1.ClusterNameLabel: clusterKey.Name},
+	})
+	Expect(cli.List(ctx, mdList, clientOptions)).To(Succeed())
+
+	for i := range mdList.Items {
+		if _, ok := mdList.Items[i].GetLabels()[clusterv1.ClusterTopologyOwnedLabel]; ok {
+			annotationPatch := client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf("{\"metadata\":{\"annotations\":{\"cluster.x-k8s.io/modifiedAt\":\"%v\"}}}", time.Now().Format(time.RFC3339))))
+			Expect(cli.Patch(ctx, &mdList.Items[i], annotationPatch)).To(Succeed())
+		}
+	}
+}

util/util.go

@@ -522,7 +522,7 @@ func ClusterToTypedObjectsMapper(c client.Client, ro client.ObjectList, scheme *
 
 		results := []ctrl.Request{}
 		for _, obj := range objects {
-			// Note: We don't check if the type cast succeeds as all items in a client.ObjectList
+			// Note: We don't check if the type cast succeeds as all items in an client.ObjectList
 			// are client.Objects.
 			o := obj.(client.Object)
 			results = append(results, ctrl.Request{