Merge pull request #10029 from adityabhatia/clusterWatcherInMdCtrl

🌱 Watch for Cluster resources in topology MachineSet & MachineDeployment controllers
kubernetes-sigs · Jan 23, 2024 · 28f0335 · 28f0335
2 parents 4174cd7 + ba855c9
commit 28f0335
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 34 deletions.
diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go
@@ -96,7 +96,6 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 				// 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)

diff --git a/internal/controllers/topology/machinedeployment/machinedeployment_controller.go b/internal/controllers/topology/machinedeployment/machinedeployment_controller.go
@@ -24,9 +24,11 @@ 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"
@@ -55,14 +57,32 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	err := ctrl.NewControllerManagedBy(mgr).
-		For(&clusterv1.MachineDeployment{}).
+	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.All(ctrl.LoggerFrom(ctx),
+					predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)),
+					predicates.ResourceNotPaused(ctrl.LoggerFrom(ctx))),
+			),
+		).
 		Named("topology/machinedeployment").
 		WithOptions(options).
-		WithEventFilter(predicates.All(ctrl.LoggerFrom(ctx),
-			predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue),
-			predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)),
-		)).
+		WithEventFilter(predicates.ResourceHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue)).
+		Watches(
+			&clusterv1.Cluster{},
+			handler.EnqueueRequestsFromMapFunc(clusterToMachineDeployments),
+			builder.WithPredicates(
+				predicates.All(ctrl.LoggerFrom(ctx),
+					predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
+					predicates.ClusterHasTopology(ctrl.LoggerFrom(ctx)),
+				),
+			),
+		).
 		Complete(r)
 	if err != nil {
 		return errors.Wrap(err, "failed setting up with a controller manager")

diff --git a/internal/controllers/topology/machineset/machineset_controller.go b/internal/controllers/topology/machineset/machineset_controller.go
@@ -26,9 +26,11 @@ 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"
 	tlog "sigs.k8s.io/cluster-api/internal/log"
@@ -57,14 +59,32 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	err := ctrl.NewControllerManagedBy(mgr).
-		For(&clusterv1.MachineSet{}).
+	clusterToMachineSets, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &clusterv1.MachineSetList{}, mgr.GetScheme())
+	if err != nil {
+		return err
+	}
+
+	err = ctrl.NewControllerManagedBy(mgr).
+		For(&clusterv1.MachineSet{},
+			builder.WithPredicates(
+				predicates.All(ctrl.LoggerFrom(ctx),
+					predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)),
+					predicates.ResourceNotPaused(ctrl.LoggerFrom(ctx))),
+			),
+		).
 		Named("topology/machineset").
 		WithOptions(options).
-		WithEventFilter(predicates.All(ctrl.LoggerFrom(ctx),
-			predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue),
-			predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)),
-		)).
+		WithEventFilter(predicates.ResourceHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue)).
+		Watches(
+			&clusterv1.Cluster{},
+			handler.EnqueueRequestsFromMapFunc(clusterToMachineSets),
+			builder.WithPredicates(
+				predicates.All(ctrl.LoggerFrom(ctx),
+					predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
+					predicates.ClusterHasTopology(ctrl.LoggerFrom(ctx)),
+				),
+			),
+		).
 		Complete(r)
 	if err != nil {
 		return errors.Wrap(err, "failed setting up with a controller manager")

diff --git a/test/framework/finalizers_helpers.go b/test/framework/finalizers_helpers.go
@@ -24,7 +24,6 @@ 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"
 
@@ -85,10 +84,6 @@ 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, allFinalizerAssertions)
 }
@@ -189,19 +184,3 @@ func concatenateFinalizerAssertions(finalizerAssertions ...map[string][]string)
 
 	return allFinalizerAssertions, kerrors.NewAggregate(allErrs)
 }
-
-// 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())
-		}
-	}
-}