From 4bc25da49550de69b82e79a5283288a1213f6c62 Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Mon, 30 Sep 2024 14:42:30 +0530 Subject: [PATCH] Log object kind in predicates --- .../controllers/kubeadmconfig_controller.go | 2 +- .../internal/controllers/controller.go | 2 +- .../controllers/machinepool_controller.go | 2 +- .../controllers/machine/machine_controller.go | 4 +- .../machinedeployment_controller.go | 2 +- .../machinehealthcheck_controller.go | 2 +- .../machineset/machineset_controller.go | 2 +- .../topology/cluster/cluster_controller.go | 15 ++-- .../machinedeployment_controller.go | 6 +- .../machineset/machineset_controller.go | 6 +- .../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/cluster_predicates.go | 69 +++++++++++-------- util/predicates/cluster_predicates_test.go | 3 +- util/predicates/generic_predicates.go | 44 ++++++------ 18 files changed, 92 insertions(+), 77 deletions(-) diff --git a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go index 3219033135bc..af9aa1033839 100644 --- a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go +++ b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go @@ -131,7 +131,7 @@ func (r *KubeadmConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl handler.EnqueueRequestsFromMapFunc(r.ClusterToKubeadmConfigs), builder.WithPredicates( predicates.All(predicateLog, - predicates.ClusterUnpausedAndInfrastructureReady(predicateLog), + predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetScheme(), predicateLog), predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go index b624d202f4a4..236316257898 100644 --- a/controlplane/kubeadm/internal/controllers/controller.go +++ b/controlplane/kubeadm/internal/controllers/controller.go @@ -105,7 +105,7 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg builder.WithPredicates( predicates.All(predicateLog, predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), - predicates.ClusterUnpausedAndInfrastructureReady(predicateLog), + predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetScheme(), predicateLog), ), ), ).Build(r) diff --git a/exp/internal/controllers/machinepool_controller.go b/exp/internal/controllers/machinepool_controller.go index e8cddb7a51bc..7727b99add2c 100644 --- a/exp/internal/controllers/machinepool_controller.go +++ b/exp/internal/controllers/machinepool_controller.go @@ -115,7 +115,7 @@ func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M // TODO: should this wait for Cluster.Status.InfrastructureReady similar to Infra Machine resources? builder.WithPredicates( predicates.All(predicateLog, - predicates.ClusterUnpaused(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index 777f4f9053c9..325ff3652e4d 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -127,8 +127,8 @@ 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.Any(predicateLog, - predicates.ClusterUnpaused(predicateLog), - predicates.ClusterControlPlaneInitialized(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), + predicates.ClusterControlPlaneInitialized(mgr.GetScheme(), predicateLog), ), predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go index d26168a48eda..95e824df118a 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller.go @@ -100,7 +100,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt builder.WithPredicates( // TODO: should this wait for Cluster.Status.InfrastructureReady similar to Infra Machine resources? predicates.All(predicateLog, - predicates.ClusterUnpaused(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), ), ), ).Complete(r) diff --git a/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go b/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go index 05cd9e026c27..8148473fea1e 100644 --- a/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go +++ b/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go @@ -100,7 +100,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt builder.WithPredicates( // TODO: should this wait for Cluster.Status.InfrastructureReady similar to Infra Machine resources? predicates.All(predicateLog, - predicates.ClusterUnpaused(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index 45ec61b303ed..ce7495ffd87f 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -123,7 +123,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt builder.WithPredicates( // TODO: should this wait for Cluster.Status.InfrastructureReady similar to Infra Machine resources? predicates.All(predicateLog, - predicates.ClusterUnpaused(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue), ), ), diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index 4cfae43d8604..aeb86f152405 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -91,7 +91,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt c, err := ctrl.NewControllerManagedBy(mgr). For(&clusterv1.Cluster{}, builder.WithPredicates( // Only reconcile Cluster with topology. - predicates.ClusterHasTopology(predicateLog), + predicates.ClusterHasTopology(mgr.GetScheme(), predicateLog), )). Named("topology/cluster"). Watches( @@ -102,13 +102,13 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt &clusterv1.MachineDeployment{}, handler.EnqueueRequestsFromMapFunc(r.machineDeploymentToCluster), // Only trigger Cluster reconciliation if the MachineDeployment is topology owned. - builder.WithPredicates(predicates.ResourceIsTopologyOwned(predicateLog)), + builder.WithPredicates(predicates.ResourceIsTopologyOwned(mgr.GetScheme(), predicateLog)), ). Watches( &expv1.MachinePool{}, handler.EnqueueRequestsFromMapFunc(r.machinePoolToCluster), // Only trigger Cluster reconciliation if the MachinePool is topology owned. - builder.WithPredicates(predicates.ResourceIsTopologyOwned(predicateLog)), + builder.WithPredicates(predicates.ResourceIsTopologyOwned(mgr.GetScheme(), predicateLog)), ). WithOptions(options). WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), predicateLog, r.WatchFilterValue)). @@ -295,19 +295,20 @@ 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 { + scheme := r.Client.Scheme() if s.Current.InfrastructureCluster != nil { if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.InfrastructureCluster, - handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}), + handler.EnqueueRequestForOwner(scheme, r.Client.RESTMapper(), &clusterv1.Cluster{}), // Only trigger Cluster reconciliation if the InfrastructureCluster is topology owned. - predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil { + predicates.ResourceIsTopologyOwned(scheme, ctrl.LoggerFrom(ctx))); err != nil { return errors.Wrap(err, "error watching Infrastructure CR") } } if s.Current.ControlPlane.Object != nil { if err := r.externalTracker.Watch(ctrl.LoggerFrom(ctx), s.Current.ControlPlane.Object, - handler.EnqueueRequestForOwner(r.Client.Scheme(), r.Client.RESTMapper(), &clusterv1.Cluster{}), + handler.EnqueueRequestForOwner(scheme, r.Client.RESTMapper(), &clusterv1.Cluster{}), // Only trigger Cluster reconciliation if the ControlPlane is topology owned. - predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx))); err != nil { + predicates.ResourceIsTopologyOwned(scheme, ctrl.LoggerFrom(ctx))); err != nil { return errors.Wrap(err, "error watching ControlPlane CR") } } diff --git a/internal/controllers/topology/machinedeployment/machinedeployment_controller.go b/internal/controllers/topology/machinedeployment/machinedeployment_controller.go index 4ce5db3a4a53..8ffeab790b95 100644 --- a/internal/controllers/topology/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/topology/machinedeployment/machinedeployment_controller.go @@ -68,7 +68,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt For(&clusterv1.MachineDeployment{}, builder.WithPredicates( predicates.All(predicateLog, - predicates.ResourceIsTopologyOwned(predicateLog), + predicates.ResourceIsTopologyOwned(mgr.GetScheme(), predicateLog), predicates.ResourceNotPaused(mgr.GetScheme(), predicateLog)), ), ). @@ -80,8 +80,8 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt handler.EnqueueRequestsFromMapFunc(clusterToMachineDeployments), builder.WithPredicates( predicates.All(predicateLog, - predicates.ClusterUnpaused(predicateLog), - predicates.ClusterHasTopology(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), + predicates.ClusterHasTopology(mgr.GetScheme(), predicateLog), ), ), ). diff --git a/internal/controllers/topology/machineset/machineset_controller.go b/internal/controllers/topology/machineset/machineset_controller.go index 35d66c60f9f3..70e5eadc91b9 100644 --- a/internal/controllers/topology/machineset/machineset_controller.go +++ b/internal/controllers/topology/machineset/machineset_controller.go @@ -70,7 +70,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt For(&clusterv1.MachineSet{}, builder.WithPredicates( predicates.All(predicateLog, - predicates.ResourceIsTopologyOwned(predicateLog), + predicates.ResourceIsTopologyOwned(mgr.GetScheme(), predicateLog), predicates.ResourceNotPaused(mgr.GetScheme(), predicateLog)), ), ). @@ -82,8 +82,8 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt handler.EnqueueRequestsFromMapFunc(clusterToMachineSets), builder.WithPredicates( predicates.All(predicateLog, - predicates.ClusterUnpaused(predicateLog), - predicates.ClusterHasTopology(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), + predicates.ClusterHasTopology(mgr.GetScheme(), predicateLog), ), ), ). diff --git a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go index f4e3914987cb..8306ce097ad0 100644 --- a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go +++ b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go @@ -190,7 +190,7 @@ func (r *DockerMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToDockerMachinePools), builder.WithPredicates( - predicates.ClusterUnpausedAndInfrastructureReady(predicateLog), + predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetScheme(), predicateLog), ), ).Build(r) if err != nil { diff --git a/test/infrastructure/docker/internal/controllers/dockercluster_controller.go b/test/infrastructure/docker/internal/controllers/dockercluster_controller.go index b9840fcca392..252466fc6b09 100644 --- a/test/infrastructure/docker/internal/controllers/dockercluster_controller.go +++ b/test/infrastructure/docker/internal/controllers/dockercluster_controller.go @@ -207,7 +207,7 @@ func (r *DockerClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFunc(ctx, infrav1.GroupVersion.WithKind("DockerCluster"), mgr.GetClient(), &infrav1.DockerCluster{})), builder.WithPredicates( - predicates.ClusterUnpaused(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), ), ).Complete(r) if err != nil { diff --git a/test/infrastructure/docker/internal/controllers/dockermachine_controller.go b/test/infrastructure/docker/internal/controllers/dockermachine_controller.go index e5079ae86cdd..bce85314a3ee 100644 --- a/test/infrastructure/docker/internal/controllers/dockermachine_controller.go +++ b/test/infrastructure/docker/internal/controllers/dockermachine_controller.go @@ -501,7 +501,7 @@ func (r *DockerMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToDockerMachines), builder.WithPredicates( - predicates.ClusterUnpausedAndInfrastructureReady(predicateLog), + predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetScheme(), predicateLog), ), ).Complete(r) if err != nil { diff --git a/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go b/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go index 23bc1cd2ecf3..aa2f6033e7a4 100644 --- a/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go +++ b/test/infrastructure/inmemory/internal/controllers/inmemorycluster_controller.go @@ -217,7 +217,7 @@ func (r *InMemoryClusterReconciler) SetupWithManager(ctx context.Context, mgr ct &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFunc(ctx, infrav1.GroupVersion.WithKind("InMemoryCluster"), mgr.GetClient(), &infrav1.InMemoryCluster{})), builder.WithPredicates( - predicates.ClusterUnpaused(predicateLog), + predicates.ClusterUnpaused(mgr.GetScheme(), predicateLog), ), ).Complete(r) if err != nil { diff --git a/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go b/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go index 7ed0d3341379..0cf3afb58ad6 100644 --- a/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go +++ b/test/infrastructure/inmemory/internal/controllers/inmemorymachine_controller.go @@ -1160,7 +1160,7 @@ func (r *InMemoryMachineReconciler) SetupWithManager(ctx context.Context, mgr ct &clusterv1.Cluster{}, handler.EnqueueRequestsFromMapFunc(clusterToInMemoryMachines), builder.WithPredicates( - predicates.ClusterUnpausedAndInfrastructureReady(predicateLog), + predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetScheme(), predicateLog), ), ).Complete(r) if err != nil { diff --git a/util/predicates/cluster_predicates.go b/util/predicates/cluster_predicates.go index 92986f8c88b7..59dbb448d791 100644 --- a/util/predicates/cluster_predicates.go +++ b/util/predicates/cluster_predicates.go @@ -21,8 +21,10 @@ import ( "fmt" "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" @@ -32,17 +34,19 @@ import ( // ClusterCreateInfraReady returns a predicate that returns true for a create event when a cluster has Status.InfrastructureReady set as true // it also returns true if the resource provided is not a Cluster to allow for use with controller-runtime NewControllerManagedBy. -func ClusterCreateInfraReady(logger logr.Logger) predicate.Funcs { +func ClusterCreateInfraReady(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { log := logger.WithValues("predicate", "ClusterCreateInfraReady", "eventType", "create") + if gvk, err := apiutil.GVKForObject(e.Object, scheme); err == nil { + log = log.WithValues(gvk.Kind, klog.KObj(e.Object)) + } c, ok := e.Object.(*clusterv1.Cluster) if !ok { log.V(4).Info("Expected Cluster", "type", fmt.Sprintf("%T", e.Object)) return false } - log = log.WithValues("Cluster", klog.KObj(c)) // Only need to trigger a reconcile if the Cluster.Status.InfrastructureReady is true if c.Status.InfrastructureReady { @@ -61,17 +65,19 @@ func ClusterCreateInfraReady(logger logr.Logger) predicate.Funcs { // ClusterCreateNotPaused returns a predicate that returns true for a create event when a cluster has Spec.Paused set as false // it also returns true if the resource provided is not a Cluster to allow for use with controller-runtime NewControllerManagedBy. -func ClusterCreateNotPaused(logger logr.Logger) predicate.Funcs { +func ClusterCreateNotPaused(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { log := logger.WithValues("predicate", "ClusterCreateNotPaused", "eventType", "create") + if gvk, err := apiutil.GVKForObject(e.Object, scheme); err == nil { + log = log.WithValues(gvk.Kind, klog.KObj(e.Object)) + } c, ok := e.Object.(*clusterv1.Cluster) if !ok { log.V(4).Info("Expected Cluster", "type", fmt.Sprintf("%T", e.Object)) return false } - log = log.WithValues("Cluster", klog.KObj(c)) // Only need to trigger a reconcile if the Cluster.Spec.Paused is false if !c.Spec.Paused { @@ -90,17 +96,19 @@ func ClusterCreateNotPaused(logger logr.Logger) predicate.Funcs { // ClusterUpdateInfraReady returns a predicate that returns true for an update event when a cluster has Status.InfrastructureReady changed from false to true // it also returns true if the resource provided is not a Cluster to allow for use with controller-runtime NewControllerManagedBy. -func ClusterUpdateInfraReady(logger logr.Logger) predicate.Funcs { +func ClusterUpdateInfraReady(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { log := logger.WithValues("predicate", "ClusterUpdateInfraReady", "eventType", "update") + if gvk, err := apiutil.GVKForObject(e.ObjectOld, scheme); err == nil { + log = log.WithValues(gvk.Kind, klog.KObj(e.ObjectOld)) + } oldCluster, ok := e.ObjectOld.(*clusterv1.Cluster) if !ok { log.V(4).Info("Expected Cluster", "type", fmt.Sprintf("%T", e.ObjectOld)) return false } - log = log.WithValues("Cluster", klog.KObj(oldCluster)) newCluster := e.ObjectNew.(*clusterv1.Cluster) @@ -120,17 +128,19 @@ func ClusterUpdateInfraReady(logger logr.Logger) predicate.Funcs { // ClusterUpdateUnpaused returns a predicate that returns true for an update event when a cluster has Spec.Paused changed from true to false // it also returns true if the resource provided is not a Cluster to allow for use with controller-runtime NewControllerManagedBy. -func ClusterUpdateUnpaused(logger logr.Logger) predicate.Funcs { +func ClusterUpdateUnpaused(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { log := logger.WithValues("predicate", "ClusterUpdateUnpaused", "eventType", "update") + if gvk, err := apiutil.GVKForObject(e.ObjectOld, scheme); err == nil { + log = log.WithValues(gvk.Kind, klog.KObj(e.ObjectOld)) + } oldCluster, ok := e.ObjectOld.(*clusterv1.Cluster) if !ok { log.V(4).Info("Expected Cluster", "type", fmt.Sprintf("%T", e.ObjectOld)) return false } - log = log.WithValues("Cluster", klog.KObj(oldCluster)) newCluster := e.ObjectNew.(*clusterv1.Cluster) @@ -159,13 +169,13 @@ func ClusterUpdateUnpaused(logger logr.Logger) predicate.Funcs { // err := controller.Watch( // source.Kind(cache, &clusterv1.Cluster{}), // handler.EnqueueRequestsFromMapFunc(clusterToMachines) -// predicates.ClusterUnpaused(r.Log), +// predicates.ClusterUnpaused(mgr.GetScheme(), r.Log), // ) -func ClusterUnpaused(logger logr.Logger) predicate.Funcs { +func ClusterUnpaused(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { log := logger.WithValues("predicate", "ClusterUnpaused") // Use any to ensure we process either create or update events we care about - return Any(log, ClusterCreateNotPaused(log), ClusterUpdateUnpaused(log)) + return Any(log, ClusterCreateNotPaused(scheme, log), ClusterUpdateUnpaused(scheme, log)) } // ClusterControlPlaneInitialized returns a Predicate that returns true on Update events @@ -175,19 +185,21 @@ func ClusterUnpaused(logger logr.Logger) predicate.Funcs { // err := controller.Watch( // source.Kind(cache, &clusterv1.Cluster{}), // handler.EnqueueRequestsFromMapFunc(clusterToMachines) -// predicates.ClusterControlPlaneInitialized(r.Log), +// predicates.ClusterControlPlaneInitialized(mgr.GetScheme(), r.Log), // ) -func ClusterControlPlaneInitialized(logger logr.Logger) predicate.Funcs { +func ClusterControlPlaneInitialized(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { log := logger.WithValues("predicate", "ClusterControlPlaneInitialized", "eventType", "update") + if gvk, err := apiutil.GVKForObject(e.ObjectOld, scheme); err == nil { + log = log.WithValues(gvk.Kind, klog.KObj(e.ObjectOld)) + } oldCluster, ok := e.ObjectOld.(*clusterv1.Cluster) if !ok { log.V(4).Info("Expected Cluster", "type", fmt.Sprintf("%T", e.ObjectOld)) return false } - log = log.WithValues("Cluster", klog.KObj(oldCluster)) newCluster := e.ObjectNew.(*clusterv1.Cluster) @@ -216,16 +228,16 @@ func ClusterControlPlaneInitialized(logger logr.Logger) predicate.Funcs { // err := controller.Watch( // source.Kind(cache, &clusterv1.Cluster{}), // handler.EnqueueRequestsFromMapFunc(clusterToMachines) -// predicates.ClusterUnpausedAndInfrastructureReady(r.Log), +// predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetScheme(), r.Log), // ) -func ClusterUnpausedAndInfrastructureReady(logger logr.Logger) predicate.Funcs { +func ClusterUnpausedAndInfrastructureReady(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { log := logger.WithValues("predicate", "ClusterUnpausedAndInfrastructureReady") // Only continue processing create events if both not paused and infrastructure is ready - createPredicates := All(log, ClusterCreateNotPaused(log), ClusterCreateInfraReady(log)) + createPredicates := All(log, ClusterCreateNotPaused(scheme, log), ClusterCreateInfraReady(scheme, log)) // Process update events if either Cluster is unpaused or infrastructure becomes ready - updatePredicates := Any(log, ClusterUpdateUnpaused(log), ClusterUpdateInfraReady(log)) + updatePredicates := Any(log, ClusterUpdateUnpaused(scheme, log), ClusterUpdateInfraReady(scheme, log)) // Use any to ensure we process either create or update events we care about return Any(log, createPredicates, updatePredicates) @@ -233,37 +245,38 @@ func ClusterUnpausedAndInfrastructureReady(logger logr.Logger) predicate.Funcs { // ClusterHasTopology returns a Predicate that returns true when cluster.Spec.Topology // is NOT nil and false otherwise. -func ClusterHasTopology(logger logr.Logger) predicate.Funcs { +func ClusterHasTopology(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - return processIfTopologyManaged(logger.WithValues("predicate", "ClusterHasTopology", "eventType", "update"), e.ObjectNew) + return processIfTopologyManaged(scheme, logger.WithValues("predicate", "ClusterHasTopology", "eventType", "update"), e.ObjectNew) }, CreateFunc: func(e event.CreateEvent) bool { - return processIfTopologyManaged(logger.WithValues("predicate", "ClusterHasTopology", "eventType", "create"), e.Object) + return processIfTopologyManaged(scheme, logger.WithValues("predicate", "ClusterHasTopology", "eventType", "create"), e.Object) }, DeleteFunc: func(e event.DeleteEvent) bool { - return processIfTopologyManaged(logger.WithValues("predicate", "ClusterHasTopology", "eventType", "delete"), e.Object) + return processIfTopologyManaged(scheme, logger.WithValues("predicate", "ClusterHasTopology", "eventType", "delete"), e.Object) }, GenericFunc: func(e event.GenericEvent) bool { - return processIfTopologyManaged(logger.WithValues("predicate", "ClusterHasTopology", "eventType", "generic"), e.Object) + return processIfTopologyManaged(scheme, logger.WithValues("predicate", "ClusterHasTopology", "eventType", "generic"), e.Object) }, } } -func processIfTopologyManaged(logger logr.Logger, object client.Object) bool { +func processIfTopologyManaged(scheme *runtime.Scheme, logger logr.Logger, object client.Object) bool { + if gvk, err := apiutil.GVKForObject(object, scheme); err == nil { + logger = logger.WithValues(gvk.Kind, klog.KObj(object)) + } cluster, ok := object.(*clusterv1.Cluster) if !ok { logger.V(4).Info("Expected Cluster", "type", fmt.Sprintf("%T", object)) return false } - log := logger.WithValues("Cluster", klog.KObj(cluster)) - if cluster.Spec.Topology != nil { - log.V(6).Info("Cluster has topology, allowing further processing") + logger.V(6).Info("Cluster has topology, allowing further processing") return true } - log.V(6).Info("Cluster does not have topology, blocking further processing") + logger.V(6).Info("Cluster does not have topology, blocking further processing") return false } diff --git a/util/predicates/cluster_predicates_test.go b/util/predicates/cluster_predicates_test.go index 253c1cbcc0e6..8c7ab7f558d1 100644 --- a/util/predicates/cluster_predicates_test.go +++ b/util/predicates/cluster_predicates_test.go @@ -21,6 +21,7 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log" @@ -31,7 +32,7 @@ import ( func TestClusterControlplaneInitializedPredicate(t *testing.T) { g := NewWithT(t) - predicate := predicates.ClusterControlPlaneInitialized(logr.New(log.NullLogSink{})) + predicate := predicates.ClusterControlPlaneInitialized(runtime.NewScheme(), logr.New(log.NullLogSink{})) markedFalse := clusterv1.Cluster{} conditions.MarkFalse(&markedFalse, clusterv1.ControlPlaneInitializedCondition, clusterv1.MissingNodeRefReason, clusterv1.ConditionSeverityWarning, "") diff --git a/util/predicates/generic_predicates.go b/util/predicates/generic_predicates.go index b90b79fd6c03..9f1be08e5af3 100644 --- a/util/predicates/generic_predicates.go +++ b/util/predicates/generic_predicates.go @@ -17,8 +17,6 @@ limitations under the License. package predicates import ( - "strings" - "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2" @@ -220,62 +218,64 @@ func processIfLabelMatch(scheme *runtime.Scheme, logger logr.Logger, obj client. // the externally managed annotation. // This implements a requirement for InfraCluster providers to be able to ignore externally managed // cluster infrastructure. -func ResourceIsNotExternallyManaged(logger logr.Logger) predicate.Funcs { +func ResourceIsNotExternallyManaged(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - return processIfNotExternallyManaged(logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "update"), e.ObjectNew) + return processIfNotExternallyManaged(scheme, logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "update"), e.ObjectNew) }, CreateFunc: func(e event.CreateEvent) bool { - return processIfNotExternallyManaged(logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "create"), e.Object) + return processIfNotExternallyManaged(scheme, logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "create"), e.Object) }, DeleteFunc: func(e event.DeleteEvent) bool { - return processIfNotExternallyManaged(logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "delete"), e.Object) + return processIfNotExternallyManaged(scheme, logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "delete"), e.Object) }, GenericFunc: func(e event.GenericEvent) bool { - return processIfNotExternallyManaged(logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "generic"), e.Object) + return processIfNotExternallyManaged(scheme, logger.WithValues("predicate", "ResourceIsNotExternallyManaged", "eventType", "generic"), e.Object) }, } } -func processIfNotExternallyManaged(logger logr.Logger, obj client.Object) bool { - kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) - log := logger.WithValues("namespace", obj.GetNamespace(), kind, obj.GetName()) +func processIfNotExternallyManaged(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.IsExternallyManaged(obj) { - log.V(4).Info("Resource is externally managed, will not attempt to map resource") + logger.V(4).Info("Resource is externally managed, will not attempt to map resource") return false } - log.V(6).Info("Resource is managed, will attempt to map resource") + logger.V(6).Info("Resource is managed, will attempt to map resource") return true } // ResourceIsTopologyOwned returns a predicate that returns true only if the resource has // the `topology.cluster.x-k8s.io/owned` label. -func ResourceIsTopologyOwned(logger logr.Logger) predicate.Funcs { +func ResourceIsTopologyOwned(scheme *runtime.Scheme, logger logr.Logger) predicate.Funcs { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - return processIfTopologyOwned(logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "update"), e.ObjectNew) + return processIfTopologyOwned(scheme, logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "update"), e.ObjectNew) }, CreateFunc: func(e event.CreateEvent) bool { - return processIfTopologyOwned(logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "create"), e.Object) + return processIfTopologyOwned(scheme, logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "create"), e.Object) }, DeleteFunc: func(e event.DeleteEvent) bool { - return processIfTopologyOwned(logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "delete"), e.Object) + return processIfTopologyOwned(scheme, logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "delete"), e.Object) }, GenericFunc: func(e event.GenericEvent) bool { - return processIfTopologyOwned(logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "generic"), e.Object) + return processIfTopologyOwned(scheme, logger.WithValues("predicate", "ResourceIsTopologyOwned", "eventType", "generic"), e.Object) }, } } -func processIfTopologyOwned(logger logr.Logger, obj client.Object) bool { - kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) - log := logger.WithValues("namespace", obj.GetNamespace(), kind, obj.GetName()) +func processIfTopologyOwned(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 labels.IsTopologyOwned(obj) { - log.V(6).Info("Resource is topology owned, will attempt to map resource") + logger.V(6).Info("Resource is topology owned, will attempt to map resource") return true } // We intentionally log this line only on level 6, because it will be very frequently // logged for MachineDeployments and MachineSets not owned by a topology. - log.V(6).Info("Resource is not topology owned, will not attempt to map resource") + logger.V(6).Info("Resource is not topology owned, will not attempt to map resource") return false }