From 1f087aa3240e24e20c37d35f6cfabae88ed82aa8 Mon Sep 17 00:00:00 2001 From: Aditya Bhatia <7274741+adityabhatia@users.noreply.github.com> Date: Sun, 21 Jan 2024 14:54:42 +0100 Subject: [PATCH] Watch for Cluster resources in topology MachineSet controller --- .../machinedeployment_controller.go | 1 - .../machinedeployment_controller.go | 32 +++++++++++++++---- .../machineset/machineset_controller.go | 32 +++++++++++++++---- test/e2e/quick_start_test.go | 2 +- test/framework/finalizers_helpers.go | 21 ------------ 5 files changed, 53 insertions(+), 35 deletions(-) diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go index a5d5fdbe733e..64ac90c58d0f 100644 --- 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 index 6ab485f4c745..b12a66826811 100644 --- 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 index 5437b9489415..f5f30bac1fd8 100644 --- 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/e2e/quick_start_test.go b/test/e2e/quick_start_test.go index bc0a545d6a55..245ea4ba589b 100644 --- a/test/e2e/quick_start_test.go +++ b/test/e2e/quick_start_test.go @@ -198,7 +198,7 @@ var _ = Describe("When following the Cluster API quick-start check finalizers re }) }) -var _ = Describe("When following the Cluster API quick-start with ClusterClass check finalizers resilience after deletion [ClusterClass]", func() { +var _ = Describe("When following the Cluster API quick-start with ClusterClass check finalizers resilience after deletion [ClusterClass] [Aditya]", func() { QuickStartSpec(ctx, func() QuickStartSpecInput { return QuickStartSpecInput{ E2EConfig: e2eConfig, diff --git a/test/framework/finalizers_helpers.go b/test/framework/finalizers_helpers.go index 2b4c91e0a436..cdc544769a92 100644 --- 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()) - } - } -}