Address PR review feedback

gardener · Jan 11, 2023 · 4586950 · 4586950
1 parent 336f68d
commit 4586950
Show file tree

Hide file tree

Showing 20 changed files with 131 additions and 108 deletions.
diff --git a/pkg/controller/service/add.go b/pkg/controller/service/add.go
@@ -16,7 +16,6 @@ package service
 
 import (
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -41,7 +40,6 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, predicates ...predicate.P
 		For(&corev1.Service{}, builder.WithPredicates(predicates...)).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: 5,
-			RecoverPanic:            pointer.Bool(true),
 		}).
 		Complete(r)
 }
diff --git a/pkg/controllermanager/controller/project/activity/add.go b/pkg/controllermanager/controller/project/activity/add.go
@@ -51,6 +51,8 @@ func (r *Reconciler) AddToManager(mgr manager.Manager) error {
 		r.Clock = clock.RealClock{}
 	}
 
+	// It's not possible to call builder.Build() without adding atleast one watch, and without this, we can't get the controller logger.
+	// Hence, we have to build up the controller manually.
 	c, err := controller.New(
 		ControllerName,
 		mgr,

diff --git a/pkg/controllermanager/controller/seed/backupbucketscheck/add.go b/pkg/controllermanager/controller/seed/backupbucketscheck/add.go
@@ -22,7 +22,6 @@ import (
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/utils/clock"
 	"k8s.io/utils/pointer"
-	"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/event"
@@ -47,21 +46,27 @@ func (r *Reconciler) AddToManager(mgr manager.Manager) error {
 		r.Clock = clock.RealClock{}
 	}
 
-	return builder.
-		ControllerManagedBy(mgr).
-		Named(ControllerName).
-		WithOptions(controller.Options{
+	// It's not possible to call builder.Build() without adding atleast one watch, and without this, we can't get the controller logger.
+	// Hence, we have to build up the controller manually.
+	c, err := controller.New(
+		ControllerName,
+		mgr,
+		controller.Options{
+			Reconciler:              r,
 			MaxConcurrentReconciles: pointer.IntDeref(r.Config.ConcurrentSyncs, 0),
-
 			// if going into exponential backoff, wait at most the configured sync period
 			RateLimiter: workqueue.NewWithMaxWaitRateLimiter(workqueue.DefaultControllerRateLimiter(), r.Config.SyncPeriod.Duration),
-		}).
-		Watches(
-			&source.Kind{Type: &gardencorev1beta1.BackupBucket{}},
-			mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapBackupBucketToSeed), mapper.UpdateWithNew, mgr.GetLogger()),
-			builder.WithPredicates(r.BackupBucketPredicate()),
-		).
-		Complete(r)
+		},
+	)
+	if err != nil {
+		return err
+	}
+
+	return c.Watch(
+		&source.Kind{Type: &gardencorev1beta1.BackupBucket{}},
+		mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapBackupBucketToSeed), mapper.UpdateWithNew, c.GetLogger()),
+		r.BackupBucketPredicate(),
+	)
 }
 
 // BackupBucketPredicate reacts only on 'CREATE' and 'UPDATE' events. It returns false if .spec.seedName == nil. For

diff --git a/pkg/controllermanager/controller/seed/extensionscheck/add.go b/pkg/controllermanager/controller/seed/extensionscheck/add.go
@@ -22,7 +22,6 @@ import (
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/utils/clock"
 	"k8s.io/utils/pointer"
-	"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/manager"
@@ -46,30 +45,36 @@ func (r *Reconciler) AddToManager(mgr manager.Manager) error {
 		r.Clock = clock.RealClock{}
 	}
 
-	return builder.
-		ControllerManagedBy(mgr).
-		Named(ControllerName).
-		WithOptions(controller.Options{
+	// It's not possible to call builder.Build() without adding atleast one watch, and without this, we can't get the controller logger.
+	// Hence, we have to build up the controller manually.
+	c, err := controller.New(
+		ControllerName,
+		mgr,
+		controller.Options{
+			Reconciler:              r,
 			MaxConcurrentReconciles: pointer.IntDeref(r.Config.ConcurrentSyncs, 0),
-
 			// if going into exponential backoff, wait at most the configured sync period
 			RateLimiter: workqueue.NewWithMaxWaitRateLimiter(workqueue.DefaultControllerRateLimiter(), r.Config.SyncPeriod.Duration),
-		}).
-		Watches(
-			&source.Kind{Type: &gardencorev1beta1.ControllerInstallation{}},
-			mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapControllerInstallationToSeed), mapper.UpdateWithNew, logr.Discard()),
-			builder.WithPredicates(predicateutils.RelevantConditionsChanged(
-				func(obj client.Object) []gardencorev1beta1.Condition {
-					controllerInstallation, ok := obj.(*gardencorev1beta1.ControllerInstallation)
-					if !ok {
-						return nil
-					}
-					return controllerInstallation.Status.Conditions
-				},
-				conditionsToCheck...,
-			)),
-		).
-		Complete(r)
+		},
+	)
+	if err != nil {
+		return err
+	}
+
+	return c.Watch(
+		&source.Kind{Type: &gardencorev1beta1.ControllerInstallation{}},
+		mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapControllerInstallationToSeed), mapper.UpdateWithNew, c.GetLogger()),
+		predicateutils.RelevantConditionsChanged(
+			func(obj client.Object) []gardencorev1beta1.Condition {
+				controllerInstallation, ok := obj.(*gardencorev1beta1.ControllerInstallation)
+				if !ok {
+					return nil
+				}
+				return controllerInstallation.Status.Conditions
+			},
+			conditionsToCheck...,
+		),
+	)
 }
 
 // MapControllerInstallationToSeed is a mapper.MapFunc for mapping a ControllerInstallation to the referenced Seed.

diff --git a/pkg/controllermanager/controller/seed/secrets/add.go b/pkg/controllermanager/controller/seed/secrets/add.go
@@ -52,22 +52,24 @@ func (r *Reconciler) AddToManager(mgr manager.Manager) error {
 		r.GardenNamespace = v1beta1constants.GardenNamespace
 	}
 
-	return builder.
+	c, err := builder.
 		ControllerManagedBy(mgr).
 		Named(ControllerName).
 		For(&gardencorev1beta1.Seed{}, builder.WithPredicates(predicateutils.ForEventTypes(predicateutils.Create))).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: 5,
 		}).
-		Watches(
-			&source.Kind{Type: &corev1.Secret{}},
-			mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapToAllSeeds), mapper.UpdateWithNew, mgr.GetLogger()),
-			builder.WithPredicates(
-				r.GardenSecretPredicate(),
-				r.SecretPredicate(),
-			),
-		).
-		Complete(r)
+		Build(r)
+	if err != nil {
+		return err
+	}
+
+	return c.Watch(
+		&source.Kind{Type: &corev1.Secret{}},
+		mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapToAllSeeds), mapper.UpdateWithNew, c.GetLogger()),
+		r.GardenSecretPredicate(),
+		r.SecretPredicate(),
+	)
 }
 
 var (

diff --git a/pkg/gardenlet/controller/backupbucket/add.go b/pkg/gardenlet/controller/backupbucket/add.go
@@ -59,13 +59,12 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster cluster.Clu
 		r.GardenNamespace = v1beta1constants.GardenNamespace
 	}
 
-	return builder.
+	c, err := builder.
 		ControllerManagedBy(mgr).
 		Named(ControllerName).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: pointer.IntDeref(r.Config.ConcurrentSyncs, 0),
-
-			RateLimiter: r.RateLimiter,
+			RateLimiter:             r.RateLimiter,
 		}).
 		Watches(
 			source.NewKindWithCache(&gardencorev1beta1.BackupBucket{}, gardenCluster.GetCache()),
@@ -75,12 +74,16 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster cluster.Clu
 				r.SeedNamePredicate(),
 			),
 		).
-		Watches(
-			source.NewKindWithCache(&extensionsv1alpha1.BackupBucket{}, seedCluster.GetCache()),
-			mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapExtensionBackupBucketToCoreBackupBucket), mapper.UpdateWithNew, logr.Discard()),
-			builder.WithPredicates(predicateutils.ExtensionStatusChanged()),
-		).
-		Complete(r)
+		Build(r)
+	if err != nil {
+		return err
+	}
+
+	return c.Watch(
+		source.NewKindWithCache(&extensionsv1alpha1.BackupBucket{}, seedCluster.GetCache()),
+		mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapExtensionBackupBucketToCoreBackupBucket), mapper.UpdateWithNew, c.GetLogger()),
+		predicateutils.ExtensionStatusChanged(),
+	)
 }
 
 // SeedNamePredicate returns a predicate which returns true when the object belongs to this seed.

diff --git a/pkg/gardenlet/controller/backupentry/backupentry/add.go b/pkg/gardenlet/controller/backupentry/backupentry/add.go
@@ -67,8 +67,7 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster, seedCluste
 		Named(ControllerName).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: pointer.IntDeref(r.Config.ConcurrentSyncs, 0),
-
-			RateLimiter: r.RateLimiter,
+			RateLimiter:             r.RateLimiter,
 		}).
 		Watches(
 			source.NewKindWithCache(&gardencorev1beta1.BackupEntry{}, gardenCluster.GetCache()),

diff --git a/pkg/gardenlet/controller/bastion/add.go b/pkg/gardenlet/controller/bastion/add.go
@@ -59,8 +59,7 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster, seedCluste
 		Named(ControllerName).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: pointer.IntDeref(r.Config.ConcurrentSyncs, 0),
-
-			RateLimiter: r.RateLimiter,
+			RateLimiter:             r.RateLimiter,
 		}).
 		Watches(
 			source.NewKindWithCache(&operationsv1alpha1.Bastion{}, gardenCluster.GetCache()),

diff --git a/pkg/gardenlet/controller/controllerinstallation/care/add.go b/pkg/gardenlet/controller/controllerinstallation/care/add.go
@@ -58,12 +58,11 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster, seedCluste
 		r.GardenNamespace = v1beta1constants.GardenNamespace
 	}
 
-	return builder.
+	c, err := builder.
 		ControllerManagedBy(mgr).
 		Named(ControllerName).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: pointer.IntDeref(r.Config.ConcurrentSyncs, 0),
-
 			// if going into exponential backoff, wait at most the configured sync period
 			RateLimiter: workqueue.NewWithMaxWaitRateLimiter(workqueue.DefaultControllerRateLimiter(), r.Config.SyncPeriod.Duration),
 		}).
@@ -72,15 +71,17 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster, seedCluste
 			&handler.EnqueueRequestForObject{},
 			builder.WithPredicates(predicateutils.ForEventTypes(predicateutils.Create)),
 		).
-		Watches(
-			source.NewKindWithCache(&resourcesv1alpha1.ManagedResource{}, seedCluster.GetCache()),
-			mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapManagedResourceToControllerInstallation), mapper.UpdateWithNew, logr.Discard()),
-			builder.WithPredicates(
-				r.IsExtensionDeployment(),
-				predicateutils.ManagedResourceConditionsChanged(),
-			),
-		).
-		Complete(r)
+		Build(r)
+	if err != nil {
+		return err
+	}
+
+	return c.Watch(
+		source.NewKindWithCache(&resourcesv1alpha1.ManagedResource{}, seedCluster.GetCache()),
+		mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapManagedResourceToControllerInstallation), mapper.UpdateWithNew, c.GetLogger()),
+		r.IsExtensionDeployment(),
+		predicateutils.ManagedResourceConditionsChanged(),
+	)
 }
 
 // IsExtensionDeployment returns a predicate which evaluates to true in case the object is in the garden namespace and

diff --git a/pkg/gardenlet/controller/controllerinstallation/required/add.go b/pkg/gardenlet/controller/controllerinstallation/required/add.go
@@ -59,6 +59,8 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster, seedCluste
 	r.Lock = &sync.RWMutex{}
 	r.KindToRequiredTypes = make(map[string]sets.String)
 
+	// It's not possible to call builder.Build() without adding atleast one watch, and without this, we can't get the controller logger.
+	// Hence, we have to build up the controller manually.
 	c, err := controller.New(
 		ControllerName,
 		mgr,

diff --git a/pkg/gardenlet/controller/managedseed/add.go b/pkg/gardenlet/controller/managedseed/add.go
@@ -93,7 +93,7 @@ func (r *Reconciler) AddToManager(
 		)
 	}
 
-	return builder.
+	c, err := builder.
 		ControllerManagedBy(mgr).
 		Named(ControllerName).
 		WithOptions(controller.Options{
@@ -106,11 +106,16 @@ func (r *Reconciler) AddToManager(
 				&predicate.GenerationChangedPredicate{},
 			),
 		).
-		Watches(
-			source.NewKindWithCache(&gardencorev1beta1.Seed{}, gardenCluster.GetCache()),
-			mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapSeedToManagedSeed), mapper.UpdateWithNew, logr.Discard()),
-			builder.WithPredicates(r.SeedOfManagedSeedPredicate(r.Config.SeedConfig.SeedTemplate.Name)),
-		).Complete(r)
+		Build(r)
+	if err != nil {
+		return err
+	}
+
+	return c.Watch(
+		source.NewKindWithCache(&gardencorev1beta1.Seed{}, gardenCluster.GetCache()),
+		mapper.EnqueueRequestsFrom(mapper.MapFunc(r.MapSeedToManagedSeed), mapper.UpdateWithNew, c.GetLogger()),
+		r.SeedOfManagedSeedPredicate(r.Config.SeedConfig.SeedTemplate.Name),
+	)
 }
 
 // ManagedSeedPredicate returns the predicate for ManagedSeed events.

diff --git a/pkg/gardenlet/controller/seed/care/add.go b/pkg/gardenlet/controller/seed/care/add.go
@@ -60,7 +60,6 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster, seedCluste
 		Named(ControllerName).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: 1,
-
 			// if going into exponential backoff, wait at most the configured sync period
 			RateLimiter: workqueue.NewWithMaxWaitRateLimiter(workqueue.DefaultControllerRateLimiter(), r.Config.SyncPeriod.Duration),
 		}).

diff --git a/pkg/gardenlet/controller/seed/seed/add.go b/pkg/gardenlet/controller/seed/seed/add.go
@@ -62,6 +62,8 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster cluster.Clu
 		r.ClientCertificateExpirationTimestamp = &metav1.Time{Time: gardenletClientCertificate.Leaf.NotAfter}
 	}
 
+	// It's not possible to call builder.Build() without adding atleast one watch, and without this, we can't get the controller logger.
+	// Hence, we have to build up the controller manually.
 	c, err := controller.New(
 		ControllerName,
 		mgr,

diff --git a/pkg/gardenlet/controller/shoot/care/add.go b/pkg/gardenlet/controller/shoot/care/add.go
@@ -52,7 +52,6 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster cluster.Clu
 		Named(ControllerName).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: pointer.IntDeref(r.Config.Controllers.ShootCare.ConcurrentSyncs, 0),
-
 			// if going into exponential backoff, wait at most the configured sync period
 			RateLimiter: workqueue.NewWithMaxWaitRateLimiter(workqueue.DefaultControllerRateLimiter(), r.Config.Controllers.ShootCare.SyncPeriod.Duration),
 		}).

diff --git a/pkg/gardenlet/controller/shoot/shoot/add.go b/pkg/gardenlet/controller/shoot/shoot/add.go
@@ -50,6 +50,8 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster cluster.Clu
 		r.Clock = clock.RealClock{}
 	}
 
+	// It's not possible to call builder.Build() without adding atleast one watch, and without this, we can't get the controller logger.
+	// Hence, we have to build up the controller manually.
 	c, err := controller.New(
 		ControllerName,
 		mgr,

diff --git a/pkg/gardenlet/controller/shootstate/secret/add.go b/pkg/gardenlet/controller/shootstate/secret/add.go
@@ -45,8 +45,7 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, gardenCluster, seedCluste
 		For(&corev1.Secret{}, builder.WithPredicates(r.SecretPredicate())).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: *r.Config.ConcurrentSyncs,
-
-			RateLimiter: workqueue.DefaultControllerRateLimiter(),
+			RateLimiter:             workqueue.DefaultControllerRateLimiter(),
 		}).
 		Complete(r)
 }

diff --git a/pkg/resourcemanager/controller/health/health/add.go b/pkg/resourcemanager/controller/health/health/add.go
@@ -58,6 +58,8 @@ func (r *Reconciler) AddToManager(mgr manager.Manager, sourceCluster, targetClus
 		r.Clock = clock.RealClock{}
 	}
 
+	// It's not possible to call builder.Build() without adding atleast one watch, and without this, we can't get the controller logger.
+	// Hence, we have to build up the controller manually.
 	c, err := controller.New(
 		ControllerName,
 		mgr,