From 508578553585e3bc9afc8afa259ccc08e6fb7e72 Mon Sep 17 00:00:00 2001 From: Karthik K N Date: Tue, 17 Sep 2024 05:03:52 +0000 Subject: [PATCH 1/3] Log resource kind in predicate logs --- .../controllers/kubeadmconfig_controller.go | 4 +- controllers/external/tracker.go | 5 +- controllers/external/tracker_test.go | 11 ++-- .../remote/cluster_cache_reconciler.go | 2 +- .../internal/controllers/controller.go | 4 +- .../clusterresourceset_controller.go | 2 +- .../clusterresourcesetbinding_controller.go | 2 +- .../controllers/machinepool_controller.go | 4 +- .../machinepool_controller_phases.go | 4 +- .../controllers/extensionconfig_controller.go | 2 +- .../controllers/cluster/cluster_controller.go | 2 +- .../cluster/cluster_controller_phases.go | 2 +- .../clusterclass/clusterclass_controller.go | 2 +- .../controllers/machine/machine_controller.go | 4 +- .../machine/machine_controller_phases.go | 2 +- .../machinedeployment_controller.go | 2 +- .../machinehealthcheck_controller.go | 4 +- .../machineset/machineset_controller.go | 4 +- .../topology/cluster/cluster_controller.go | 6 +-- .../machinedeployment_controller.go | 4 +- .../machineset/machineset_controller.go | 4 +- .../dockermachinepool_controller.go | 2 +- .../controllers/dockercluster_controller.go | 2 +- .../controllers/dockermachine_controller.go | 2 +- .../controllers/inmemorycluster_controller.go | 2 +- .../controllers/inmemorymachine_controller.go | 2 +- util/predicates/generic_predicates.go | 51 ++++++++++--------- 27 files changed, 73 insertions(+), 64 deletions(-) diff --git a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go index 54cecd3e7a27..3219033135bc 100644 --- a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go +++ b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go @@ -117,7 +117,7 @@ func (r *KubeadmConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl Watches( &clusterv1.Machine{}, handler.EnqueueRequestsFromMapFunc(r.MachineToBootstrapMapFunc), - ).WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)) + ).WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)) if feature.Gates.Enabled(feature.MachinePool) { b = b.Watches( @@ -132,7 +132,7 @@ func (r *KubeadmConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl builder.WithPredicates( predicates.All(predicateLog, predicates.ClusterUnpausedAndInfrastructureReady(predicateLog), - predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue), + predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), ) diff --git a/controllers/external/tracker.go b/controllers/external/tracker.go index 26a2a4206fd9..bdbc52bcdaa4 100644 --- a/controllers/external/tracker.go +++ b/controllers/external/tracker.go @@ -22,6 +22,7 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -41,7 +42,7 @@ type ObjectTracker struct { } // Watch uses the controller to issue a Watch only if the object hasn't been seen before. -func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error { +func (o *ObjectTracker) Watch(scheme *runtime.Scheme, log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error { // Consider this a no-op if the controller isn't present. if o.Controller == nil { return nil @@ -58,7 +59,7 @@ func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handle o.Cache, obj.DeepCopyObject().(client.Object), handler, - append(p, predicates.ResourceNotPaused(log))..., + append(p, predicates.ResourceNotPaused(scheme, log))..., )) if err != nil { o.m.Delete(key) diff --git a/controllers/external/tracker_test.go b/controllers/external/tracker_test.go index 553edb220513..d79737088804 100644 --- a/controllers/external/tracker_test.go +++ b/controllers/external/tracker_test.go @@ -23,6 +23,7 @@ import ( . "github.com/onsi/gomega" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/source" @@ -66,11 +67,12 @@ func TestRetryWatch(t *testing.T) { ctrl := newWatchCountController(true) tracker := ObjectTracker{Controller: ctrl} - err := tracker.Watch(logger, &clusterv1.Cluster{}, nil) + scheme := runtime.NewScheme() + err := tracker.Watch(scheme, logger, &clusterv1.Cluster{}, nil) g.Expect(err).To(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(1)) // Calling Watch on same Object kind that failed earlier should be retryable. - err = tracker.Watch(logger, &clusterv1.Cluster{}, nil) + err = tracker.Watch(scheme, logger, &clusterv1.Cluster{}, nil) g.Expect(err).To(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(2)) } @@ -86,11 +88,12 @@ func TestWatchMultipleTimes(t *testing.T) { APIVersion: clusterv1.GroupVersion.Version, }, } - err := tracker.Watch(logger, obj, nil) + scheme := runtime.NewScheme() + err := tracker.Watch(scheme, logger, obj, nil) g.Expect(err).ToNot(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(1)) // Calling Watch on same Object kind should not register watch again. - err = tracker.Watch(logger, obj, nil) + err = tracker.Watch(scheme, logger, obj, nil) g.Expect(err).ToNot(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(1)) } diff --git a/controllers/remote/cluster_cache_reconciler.go b/controllers/remote/cluster_cache_reconciler.go index 88111c9a3e30..caace4257af5 100644 --- a/controllers/remote/cluster_cache_reconciler.go +++ b/controllers/remote/cluster_cache_reconciler.go @@ -46,7 +46,7 @@ func (r *ClusterCacheReconciler) SetupWithManager(ctx context.Context, mgr ctrl. Named("remote/clustercache"). For(&clusterv1.Cluster{}). WithOptions(options). - WithEventFilter(predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Complete(r) if err != nil { diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go index dc80922248ea..b624d202f4a4 100644 --- a/controlplane/kubeadm/internal/controllers/controller.go +++ b/controlplane/kubeadm/internal/controllers/controller.go @@ -98,13 +98,13 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg For(&controlplanev1.KubeadmControlPlane{}). Owns(&clusterv1.Machine{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(r.ClusterToKubeadmControlPlane), builder.WithPredicates( predicates.All(predicateLog, - predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue), + predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), predicates.ClusterUnpausedAndInfrastructureReady(predicateLog), ), ), diff --git a/exp/addons/internal/controllers/clusterresourceset_controller.go b/exp/addons/internal/controllers/clusterresourceset_controller.go index 7b12c72ca625..f4d70c8df48a 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller.go @@ -98,7 +98,7 @@ func (r *ClusterResourceSetReconciler) SetupWithManager(ctx context.Context, mgr resourcepredicates.TypedResourceCreateOrUpdate[*metav1.PartialObjectMetadata](predicateLog), )). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Complete(r) if err != nil { return errors.Wrap(err, "failed setting up with a controller manager") diff --git a/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go b/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go index c4c219f59e0e..62d697b40192 100644 --- a/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go +++ b/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go @@ -56,7 +56,7 @@ func (r *ClusterResourceSetBindingReconciler) SetupWithManager(ctx context.Conte handler.EnqueueRequestsFromMapFunc(r.clusterToClusterResourceSetBinding), ). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Complete(r) if err != nil { return errors.Wrap(err, "failed setting up with a controller manager") diff --git a/exp/internal/controllers/machinepool_controller.go b/exp/internal/controllers/machinepool_controller.go index 67112e668a07..0ca3ea43d9ab 100644 --- a/exp/internal/controllers/machinepool_controller.go +++ b/exp/internal/controllers/machinepool_controller.go @@ -108,7 +108,7 @@ func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M c, err := ctrl.NewControllerManagedBy(mgr). For(&expv1.MachinePool{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToMachinePools), @@ -116,7 +116,7 @@ func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M builder.WithPredicates( predicates.All(predicateLog, predicates.ClusterUnpaused(predicateLog), - predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue), + predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), ). diff --git a/exp/internal/controllers/machinepool_controller_phases.go b/exp/internal/controllers/machinepool_controller_phases.go index e46b58f8818c..fab0ea37c126 100644 --- a/exp/internal/controllers/machinepool_controller_phases.go +++ b/exp/internal/controllers/machinepool_controller_phases.go @@ -123,7 +123,7 @@ func (r *MachinePoolReconciler) reconcileExternal(ctx context.Context, cluster * } // Ensure we add a watch to the external object, if there isn't one already. - if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &expv1.MachinePool{})); err != nil { + if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &expv1.MachinePool{})); err != nil { return external.ReconcileOutput{}, err } @@ -379,7 +379,7 @@ func (r *MachinePoolReconciler) reconcileMachines(ctx context.Context, s *scope, sampleInfraMachine.SetKind(infraMachineKind) // Add watcher for infraMachine, if there isn't one already. - if err := r.externalTracker.Watch(log, sampleInfraMachine, handler.EnqueueRequestsFromMapFunc(r.infraMachineToMachinePoolMapper)); err != nil { + if err := r.externalTracker.Watch(r.Client.Scheme(), log, sampleInfraMachine, handler.EnqueueRequestsFromMapFunc(r.infraMachineToMachinePoolMapper)); err != nil { return err } diff --git a/exp/runtime/internal/controllers/extensionconfig_controller.go b/exp/runtime/internal/controllers/extensionconfig_controller.go index 5d9f7641f558..e4c6eba808f7 100644 --- a/exp/runtime/internal/controllers/extensionconfig_controller.go +++ b/exp/runtime/internal/controllers/extensionconfig_controller.go @@ -79,7 +79,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt ), )). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Complete(r) if err != nil { return errors.Wrap(err, "failed setting up with a controller manager") diff --git a/internal/controllers/cluster/cluster_controller.go b/internal/controllers/cluster/cluster_controller.go index 23d1f6aa55d3..c3938dff5d93 100644 --- a/internal/controllers/cluster/cluster_controller.go +++ b/internal/controllers/cluster/cluster_controller.go @@ -88,7 +88,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt handler.EnqueueRequestsFromMapFunc(r.controlPlaneMachineToCluster), ). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Build(r) if err != nil { diff --git a/internal/controllers/cluster/cluster_controller_phases.go b/internal/controllers/cluster/cluster_controller_phases.go index 4afa3976f5e9..2860c91d2d80 100644 --- a/internal/controllers/cluster/cluster_controller_phases.go +++ b/internal/controllers/cluster/cluster_controller_phases.go @@ -93,7 +93,7 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.C } // Ensure we add a watcher to the external object. - if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{})); err != nil { + if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{})); err != nil { return external.ReconcileOutput{}, err } diff --git a/internal/controllers/clusterclass/clusterclass_controller.go b/internal/controllers/clusterclass/clusterclass_controller.go index 486dd26bf740..8a3cf2994c6a 100644 --- a/internal/controllers/clusterclass/clusterclass_controller.go +++ b/internal/controllers/clusterclass/clusterclass_controller.go @@ -79,7 +79,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt &runtimev1.ExtensionConfig{}, handler.EnqueueRequestsFromMapFunc(r.extensionConfigToClusterClass), ). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Complete(r) if err != nil { diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index 37e337bab1f1..a3a63104b38f 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -119,7 +119,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt c, err := ctrl.NewControllerManagedBy(mgr). For(&clusterv1.Machine{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToMachines), @@ -130,7 +130,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt predicates.ClusterUnpaused(predicateLog), predicates.ClusterControlPlaneInitialized(predicateLog), ), - predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue), + predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), )). Watches( diff --git a/internal/controllers/machine/machine_controller_phases.go b/internal/controllers/machine/machine_controller_phases.go index e922accef42a..1bb857dd1597 100644 --- a/internal/controllers/machine/machine_controller_phases.go +++ b/internal/controllers/machine/machine_controller_phases.go @@ -135,7 +135,7 @@ func (r *Reconciler) ensureExternalOwnershipAndWatch(ctx context.Context, cluste } // Ensure we add a watch to the external object, if there isn't one already. - if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Machine{})); err != nil { + if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Machine{})); err != nil { return external.ReconcileOutput{}, err } diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go index f9b55db02c4c..fac094e73e9d 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller.go @@ -91,7 +91,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt handler.EnqueueRequestsFromMapFunc(r.MachineSetToDeployments), ). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToMachineDeployments), diff --git a/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go b/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go index 733beff07764..05cd9e026c27 100644 --- a/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go +++ b/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go @@ -93,7 +93,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt handler.EnqueueRequestsFromMapFunc(r.machineToMachineHealthCheck), ). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(r.clusterToMachineHealthCheck), @@ -101,7 +101,7 @@ 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(predicateLog, predicates.ClusterUnpaused(predicateLog), - predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue), + predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), ).Build(r) diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index 360616e58412..4916bc170246 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -115,7 +115,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt handler.EnqueueRequestsFromMapFunc(r.MachineToMachineSets), ). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToMachineSets), @@ -123,7 +123,7 @@ 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(predicateLog, predicates.ClusterUnpaused(predicateLog), - predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue), + predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), ).Complete(r) diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index d38e969e0d10..0d1db575d477 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -111,7 +111,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt builder.WithPredicates(predicates.ResourceIsTopologyOwned(predicateLog)), ). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Build(r) if err != nil { @@ -295,7 +295,7 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope.Scope) (ctrl.Result // setupDynamicWatches create watches for InfrastructureCluster and ControlPlane CRs when they exist. func (r *Reconciler) setupDynamicWatches(ctx context.Context, s *scope.Scope) error { if s.Current.InfrastructureCluster != nil { - if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.InfrastructureCluster, + if err := r.externalTracker.Watch(r.Client.Scheme(), ctrl.LoggerFrom(ctx), s.Current.InfrastructureCluster, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}), // Only trigger Cluster reconciliation if the InfrastructureCluster is topology owned. predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil { @@ -303,7 +303,7 @@ func (r *Reconciler) setupDynamicWatches(ctx context.Context, s *scope.Scope) er } } if s.Current.ControlPlane.Object != nil { - if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.ControlPlane.Object, + if err := r.externalTracker.Watch(r.Client.Scheme(), ctrl.LoggerFrom(ctx), s.Current.ControlPlane.Object, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}), // Only trigger Cluster reconciliation if the ControlPlane is topology owned. predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil { diff --git a/internal/controllers/topology/machinedeployment/machinedeployment_controller.go b/internal/controllers/topology/machinedeployment/machinedeployment_controller.go index 0496907861dd..4ce5db3a4a53 100644 --- a/internal/controllers/topology/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/topology/machinedeployment/machinedeployment_controller.go @@ -69,12 +69,12 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt builder.WithPredicates( predicates.All(predicateLog, predicates.ResourceIsTopologyOwned(predicateLog), - predicates.ResourceNotPaused(predicateLog)), + predicates.ResourceNotPaused(mgr.GetScheme(), predicateLog)), ), ). Named("topology/machinedeployment"). WithOptions(options). - WithEventFilter(predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToMachineDeployments), diff --git a/internal/controllers/topology/machineset/machineset_controller.go b/internal/controllers/topology/machineset/machineset_controller.go index b73b45f64bba..35d66c60f9f3 100644 --- a/internal/controllers/topology/machineset/machineset_controller.go +++ b/internal/controllers/topology/machineset/machineset_controller.go @@ -71,12 +71,12 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt builder.WithPredicates( predicates.All(predicateLog, predicates.ResourceIsTopologyOwned(predicateLog), - predicates.ResourceNotPaused(predicateLog)), + predicates.ResourceNotPaused(mgr.GetScheme(), predicateLog)), ), ). Named("topology/machineset"). WithOptions(options). - WithEventFilter(predicates.ResourceHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToMachineSets), diff --git a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go index bbe336599f8d..16a4c25c375e 100644 --- a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go +++ b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go @@ -176,7 +176,7 @@ func (r *DockerMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr c, err := ctrl.NewControllerManagedBy(mgr). For(&infraexpv1.DockerMachinePool{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &expv1.MachinePool{}, handler.EnqueueRequestsFromMapFunc(utilexp.MachinePoolToInfrastructureMapFunc(ctx, diff --git a/test/infrastructure/docker/internal/controllers/dockercluster_controller.go b/test/infrastructure/docker/internal/controllers/dockercluster_controller.go index 6af29907877a..b9840fcca392 100644 --- a/test/infrastructure/docker/internal/controllers/dockercluster_controller.go +++ b/test/infrastructure/docker/internal/controllers/dockercluster_controller.go @@ -202,7 +202,7 @@ func (r *DockerClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl err := ctrl.NewControllerManagedBy(mgr). For(&infrav1.DockerCluster{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFunc(ctx, infrav1.GroupVersion.WithKind("DockerCluster"), mgr.GetClient(), &infrav1.DockerCluster{})), diff --git a/test/infrastructure/docker/internal/controllers/dockermachine_controller.go b/test/infrastructure/docker/internal/controllers/dockermachine_controller.go index d1b908d9f5f5..e5079ae86cdd 100644 --- a/test/infrastructure/docker/internal/controllers/dockermachine_controller.go +++ b/test/infrastructure/docker/internal/controllers/dockermachine_controller.go @@ -488,7 +488,7 @@ func (r *DockerMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl err = ctrl.NewControllerManagedBy(mgr). For(&infrav1.DockerMachine{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Machine{}, handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("DockerMachine"))), diff --git a/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go b/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go index da1b2c0f948d..23bc1cd2ecf3 100644 --- a/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go +++ b/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go @@ -212,7 +212,7 @@ func (r *InMemoryClusterReconciler) SetupWithManager(ctx context.Context, mgr ct err := ctrl.NewControllerManagedBy(mgr). For(&infrav1.InMemoryCluster{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFunc(ctx, infrav1.GroupVersion.WithKind("InMemoryCluster"), mgr.GetClient(), &infrav1.InMemoryCluster{})), diff --git a/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go b/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go index a196f2eca86c..7ed0d3341379 100644 --- a/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go +++ b/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go @@ -1147,7 +1147,7 @@ func (r *InMemoryMachineReconciler) SetupWithManager(ctx context.Context, mgr ct err = ctrl.NewControllerManagedBy(mgr). For(&infrav1.InMemoryMachine{}). WithOptions(options). - WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(predicateLog, r.WatchFilterValue)). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). Watches( &clusterv1.Machine{}, handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("InMemoryMachine"))), diff --git a/util/predicates/generic_predicates.go b/util/predicates/generic_predicates.go index ccce5de6d1da..b90b79fd6c03 100644 --- a/util/predicates/generic_predicates.go +++ b/util/predicates/generic_predicates.go @@ -20,7 +20,10 @@ import ( "strings" "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -130,19 +133,19 @@ func Any(logger logr.Logger, predicates ...predicate.Funcs) predicate.Funcs { // ResourceHasFilterLabel returns a predicate that returns true only if the provided resource contains // a label with the WatchLabel key and the configured label value exactly. -func ResourceHasFilterLabel(logger logr.Logger, labelValue string) predicate.Funcs { +func ResourceHasFilterLabel(scheme *runtime.Scheme, logger logr.Logger, labelValue string) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - return processIfLabelMatch(logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "update"), e.ObjectNew, labelValue) + return processIfLabelMatch(scheme, logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "update"), e.ObjectNew, labelValue) }, CreateFunc: func(e event.CreateEvent) bool { - return processIfLabelMatch(logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "create"), e.Object, labelValue) + return processIfLabelMatch(scheme, logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "create"), e.Object, labelValue) }, DeleteFunc: func(e event.DeleteEvent) bool { - return processIfLabelMatch(logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "delete"), e.Object, labelValue) + return processIfLabelMatch(scheme, logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "delete"), e.Object, labelValue) }, GenericFunc: func(e event.GenericEvent) bool { - return processIfLabelMatch(logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "generic"), e.Object, labelValue) + return processIfLabelMatch(scheme, logger.WithValues("predicate", "ResourceHasFilterLabel", "eventType", "generic"), e.Object, labelValue) }, } } @@ -157,57 +160,59 @@ func ResourceHasFilterLabel(logger logr.Logger, labelValue string) predicate.Fun // controller, err := ctrl.NewControllerManagedBy(mgr). // For(&v1.MyType{}). // WithOptions(options). -// WithEventFilter(util.ResourceNotPaused(r.Log)). +// WithEventFilter(util.ResourceNotPaused(mgr.GetScheme(), r.Log)). // Build(r) // return err // } -func ResourceNotPaused(logger logr.Logger) predicate.Funcs { +func ResourceNotPaused(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - return processIfNotPaused(logger.WithValues("predicate", "ResourceNotPaused", "eventType", "update"), e.ObjectNew) + return processIfNotPaused(scheme, logger.WithValues("predicate", "ResourceNotPaused", "eventType", "update"), e.ObjectNew) }, CreateFunc: func(e event.CreateEvent) bool { - return processIfNotPaused(logger.WithValues("predicate", "ResourceNotPaused", "eventType", "create"), e.Object) + return processIfNotPaused(scheme, logger.WithValues("predicate", "ResourceNotPaused", "eventType", "create"), e.Object) }, DeleteFunc: func(e event.DeleteEvent) bool { - return processIfNotPaused(logger.WithValues("predicate", "ResourceNotPaused", "eventType", "delete"), e.Object) + return processIfNotPaused(scheme, logger.WithValues("predicate", "ResourceNotPaused", "eventType", "delete"), e.Object) }, GenericFunc: func(e event.GenericEvent) bool { - return processIfNotPaused(logger.WithValues("predicate", "ResourceNotPaused", "eventType", "generic"), e.Object) + return processIfNotPaused(scheme, logger.WithValues("predicate", "ResourceNotPaused", "eventType", "generic"), e.Object) }, } } // ResourceNotPausedAndHasFilterLabel returns a predicate that returns true only if the // ResourceNotPaused and ResourceHasFilterLabel predicates return true. -func ResourceNotPausedAndHasFilterLabel(logger logr.Logger, labelValue string) predicate.Funcs { - return All(logger, ResourceNotPaused(logger), ResourceHasFilterLabel(logger, labelValue)) +func ResourceNotPausedAndHasFilterLabel(scheme *runtime.Scheme, logger logr.Logger, labelValue string) predicate.Funcs { + return All(logger, ResourceNotPaused(scheme, logger), ResourceHasFilterLabel(scheme, logger, labelValue)) } -func processIfNotPaused(logger logr.Logger, obj client.Object) bool { - kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) - log := logger.WithValues("namespace", obj.GetNamespace(), kind, obj.GetName()) +func processIfNotPaused(scheme *runtime.Scheme, logger logr.Logger, obj client.Object) bool { + if gvk, err := apiutil.GVKForObject(obj, scheme); err == nil { + logger = logger.WithValues(gvk.Kind, klog.KObj(obj)) + } if annotations.HasPaused(obj) { - log.V(4).Info("Resource is paused, will not attempt to map resource") + logger.V(4).Info("Resource is paused, will not attempt to map resource") return false } - log.V(6).Info("Resource is not paused, will attempt to map resource") + logger.V(6).Info("Resource is not paused, will attempt to map resource") return true } -func processIfLabelMatch(logger logr.Logger, obj client.Object, labelValue string) bool { +func processIfLabelMatch(scheme *runtime.Scheme, logger logr.Logger, obj client.Object, labelValue string) bool { // Return early if no labelValue was set. if labelValue == "" { return true } - kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) - log := logger.WithValues("namespace", obj.GetNamespace(), kind, obj.GetName()) + if gvk, err := apiutil.GVKForObject(obj, scheme); err == nil { + logger = logger.WithValues(gvk.Kind, klog.KObj(obj)) + } if labels.HasWatchLabel(obj, labelValue) { - log.V(6).Info("Resource matches label, will attempt to map resource") + logger.V(6).Info("Resource matches label, will attempt to map resource") return true } - log.V(4).Info("Resource does not match label, will not attempt to map resource") + logger.V(4).Info("Resource does not match label, will not attempt to map resource") return false } From 2afb5cdd1c1b244bca448cf11a391053aa7ce06f Mon Sep 17 00:00:00 2001 From: Karthik K N Date: Wed, 18 Sep 2024 06:05:07 +0000 Subject: [PATCH 2/3] Add scheme as struct member for tracker --- controllers/external/tracker.go | 5 +++-- controllers/external/tracker_test.go | 11 ++++------- exp/internal/controllers/machinepool_controller.go | 1 + .../controllers/machinepool_controller_phases.go | 4 ++-- internal/controllers/cluster/cluster_controller.go | 1 + .../controllers/cluster/cluster_controller_phases.go | 2 +- internal/controllers/machine/machine_controller.go | 1 + .../controllers/machine/machine_controller_phases.go | 2 +- .../topology/cluster/cluster_controller.go | 5 +++-- .../controllers/dockermachinepool_controller.go | 1 + 10 files changed, 18 insertions(+), 15 deletions(-) diff --git a/controllers/external/tracker.go b/controllers/external/tracker.go index bdbc52bcdaa4..2bfa357c5525 100644 --- a/controllers/external/tracker.go +++ b/controllers/external/tracker.go @@ -39,10 +39,11 @@ type ObjectTracker struct { Controller controller.Controller Cache cache.Cache + Scheme *runtime.Scheme } // Watch uses the controller to issue a Watch only if the object hasn't been seen before. -func (o *ObjectTracker) Watch(scheme *runtime.Scheme, log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error { +func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error { // Consider this a no-op if the controller isn't present. if o.Controller == nil { return nil @@ -59,7 +60,7 @@ func (o *ObjectTracker) Watch(scheme *runtime.Scheme, log logr.Logger, obj clien o.Cache, obj.DeepCopyObject().(client.Object), handler, - append(p, predicates.ResourceNotPaused(scheme, log))..., + append(p, predicates.ResourceNotPaused(o.Scheme, log))..., )) if err != nil { o.m.Delete(key) diff --git a/controllers/external/tracker_test.go b/controllers/external/tracker_test.go index d79737088804..553edb220513 100644 --- a/controllers/external/tracker_test.go +++ b/controllers/external/tracker_test.go @@ -23,7 +23,6 @@ import ( . "github.com/onsi/gomega" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/source" @@ -67,12 +66,11 @@ func TestRetryWatch(t *testing.T) { ctrl := newWatchCountController(true) tracker := ObjectTracker{Controller: ctrl} - scheme := runtime.NewScheme() - err := tracker.Watch(scheme, logger, &clusterv1.Cluster{}, nil) + err := tracker.Watch(logger, &clusterv1.Cluster{}, nil) g.Expect(err).To(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(1)) // Calling Watch on same Object kind that failed earlier should be retryable. - err = tracker.Watch(scheme, logger, &clusterv1.Cluster{}, nil) + err = tracker.Watch(logger, &clusterv1.Cluster{}, nil) g.Expect(err).To(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(2)) } @@ -88,12 +86,11 @@ func TestWatchMultipleTimes(t *testing.T) { APIVersion: clusterv1.GroupVersion.Version, }, } - scheme := runtime.NewScheme() - err := tracker.Watch(scheme, logger, obj, nil) + err := tracker.Watch(logger, obj, nil) g.Expect(err).ToNot(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(1)) // Calling Watch on same Object kind should not register watch again. - err = tracker.Watch(scheme, logger, obj, nil) + err = tracker.Watch(logger, obj, nil) g.Expect(err).ToNot(HaveOccurred()) g.Expect(ctrl.count).Should(Equal(1)) } diff --git a/exp/internal/controllers/machinepool_controller.go b/exp/internal/controllers/machinepool_controller.go index 0ca3ea43d9ab..e8cddb7a51bc 100644 --- a/exp/internal/controllers/machinepool_controller.go +++ b/exp/internal/controllers/machinepool_controller.go @@ -130,6 +130,7 @@ func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M r.externalTracker = external.ObjectTracker{ Controller: c, Cache: mgr.GetCache(), + Scheme: mgr.GetScheme(), } r.ssaCache = ssa.NewCache() diff --git a/exp/internal/controllers/machinepool_controller_phases.go b/exp/internal/controllers/machinepool_controller_phases.go index fab0ea37c126..e46b58f8818c 100644 --- a/exp/internal/controllers/machinepool_controller_phases.go +++ b/exp/internal/controllers/machinepool_controller_phases.go @@ -123,7 +123,7 @@ func (r *MachinePoolReconciler) reconcileExternal(ctx context.Context, cluster * } // Ensure we add a watch to the external object, if there isn't one already. - if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &expv1.MachinePool{})); err != nil { + if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &expv1.MachinePool{})); err != nil { return external.ReconcileOutput{}, err } @@ -379,7 +379,7 @@ func (r *MachinePoolReconciler) reconcileMachines(ctx context.Context, s *scope, sampleInfraMachine.SetKind(infraMachineKind) // Add watcher for infraMachine, if there isn't one already. - if err := r.externalTracker.Watch(r.Client.Scheme(), log, sampleInfraMachine, handler.EnqueueRequestsFromMapFunc(r.infraMachineToMachinePoolMapper)); err != nil { + if err := r.externalTracker.Watch(log, sampleInfraMachine, handler.EnqueueRequestsFromMapFunc(r.infraMachineToMachinePoolMapper)); err != nil { return err } diff --git a/internal/controllers/cluster/cluster_controller.go b/internal/controllers/cluster/cluster_controller.go index c3938dff5d93..854382965c54 100644 --- a/internal/controllers/cluster/cluster_controller.go +++ b/internal/controllers/cluster/cluster_controller.go @@ -99,6 +99,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt r.externalTracker = external.ObjectTracker{ Controller: c, Cache: mgr.GetCache(), + Scheme: mgr.GetScheme(), } return nil } diff --git a/internal/controllers/cluster/cluster_controller_phases.go b/internal/controllers/cluster/cluster_controller_phases.go index 2860c91d2d80..4afa3976f5e9 100644 --- a/internal/controllers/cluster/cluster_controller_phases.go +++ b/internal/controllers/cluster/cluster_controller_phases.go @@ -93,7 +93,7 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.C } // Ensure we add a watcher to the external object. - if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{})); err != nil { + if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{})); err != nil { return external.ReconcileOutput{}, err } diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index a3a63104b38f..777f4f9053c9 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -151,6 +151,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt r.externalTracker = external.ObjectTracker{ Controller: c, Cache: mgr.GetCache(), + Scheme: mgr.GetScheme(), } r.ssaCache = ssa.NewCache() r.drainCache = drain.NewCache() diff --git a/internal/controllers/machine/machine_controller_phases.go b/internal/controllers/machine/machine_controller_phases.go index 1bb857dd1597..e922accef42a 100644 --- a/internal/controllers/machine/machine_controller_phases.go +++ b/internal/controllers/machine/machine_controller_phases.go @@ -135,7 +135,7 @@ func (r *Reconciler) ensureExternalOwnershipAndWatch(ctx context.Context, cluste } // Ensure we add a watch to the external object, if there isn't one already. - if err := r.externalTracker.Watch(r.Client.Scheme(), log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Machine{})); err != nil { + if err := r.externalTracker.Watch(log, obj, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Machine{})); err != nil { return external.ReconcileOutput{}, err } diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index 0d1db575d477..4cfae43d8604 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -121,6 +121,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt r.externalTracker = external.ObjectTracker{ Controller: c, Cache: mgr.GetCache(), + Scheme: mgr.GetScheme(), } r.desiredStateGenerator = desiredstate.NewGenerator(r.Client, r.Tracker, r.RuntimeClient) r.recorder = mgr.GetEventRecorderFor("topology/cluster-controller") @@ -295,7 +296,7 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope.Scope) (ctrl.Result // setupDynamicWatches create watches for InfrastructureCluster and ControlPlane CRs when they exist. func (r *Reconciler) setupDynamicWatches(ctx context.Context, s *scope.Scope) error { if s.Current.InfrastructureCluster != nil { - if err := r.externalTracker.Watch(r.Client.Scheme(), ctrl.LoggerFrom(ctx), s.Current.InfrastructureCluster, + if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.InfrastructureCluster, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}), // Only trigger Cluster reconciliation if the InfrastructureCluster is topology owned. predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil { @@ -303,7 +304,7 @@ func (r *Reconciler) setupDynamicWatches(ctx context.Context, s *scope.Scope) er } } if s.Current.ControlPlane.Object != nil { - if err := r.externalTracker.Watch(r.Client.Scheme(), ctrl.LoggerFrom(ctx), s.Current.ControlPlane.Object, + if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.ControlPlane.Object, handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}), // Only trigger Cluster reconciliation if the ControlPlane is topology owned. predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil { diff --git a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go index 16a4c25c375e..f4e3914987cb 100644 --- a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go +++ b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go @@ -201,6 +201,7 @@ func (r *DockerMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr r.externalTracker = external.ObjectTracker{ Controller: c, Cache: mgr.GetCache(), + Scheme: mgr.GetScheme(), } r.ssaCache = ssa.NewCache() From f8e6c0b08a7186c58ae9fb7cfc804fa040add68e Mon Sep 17 00:00:00 2001 From: Karthik K N Date: Wed, 18 Sep 2024 07:02:01 +0000 Subject: [PATCH 3/3] Make scheme and cache mandatory when controller is set in object tracker --- controllers/external/tracker.go | 4 ++++ controllers/external/tracker_test.go | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/controllers/external/tracker.go b/controllers/external/tracker.go index 2bfa357c5525..ab7a45a68066 100644 --- a/controllers/external/tracker.go +++ b/controllers/external/tracker.go @@ -49,6 +49,10 @@ func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handle return nil } + if o.Cache == nil || o.Scheme == nil { + return errors.New("both scheme and cache must be set for object tracker") + } + gvk := obj.GetObjectKind().GroupVersionKind() key := gvk.GroupKind().String() if _, loaded := o.m.LoadOrStore(key, struct{}{}); loaded { diff --git a/controllers/external/tracker_test.go b/controllers/external/tracker_test.go index 553edb220513..6c89db695de6 100644 --- a/controllers/external/tracker_test.go +++ b/controllers/external/tracker_test.go @@ -23,6 +23,8 @@ import ( . "github.com/onsi/gomega" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/cache/informertest" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/source" @@ -64,7 +66,7 @@ func (c *watchCountController) Watch(_ source.Source) error { func TestRetryWatch(t *testing.T) { g := NewWithT(t) ctrl := newWatchCountController(true) - tracker := ObjectTracker{Controller: ctrl} + tracker := ObjectTracker{Controller: ctrl, Scheme: runtime.NewScheme(), Cache: &informertest.FakeInformers{}} err := tracker.Watch(logger, &clusterv1.Cluster{}, nil) g.Expect(err).To(HaveOccurred()) @@ -78,7 +80,7 @@ func TestRetryWatch(t *testing.T) { func TestWatchMultipleTimes(t *testing.T) { g := NewWithT(t) ctrl := &watchCountController{} - tracker := ObjectTracker{Controller: ctrl} + tracker := ObjectTracker{Controller: ctrl, Scheme: runtime.NewScheme(), Cache: &informertest.FakeInformers{}} obj := &clusterv1.Cluster{ TypeMeta: metav1.TypeMeta{