feat: scope controller name validation to manager instance

Author: jingyih

pkg/controller/controller.go

@@ -166,6 +166,15 @@ func NewTyped[request comparable](name string, mgr manager.Manager, options Type
 		return nil, err
 	}
 
+	// Validate controller name uniqueness within this manager
+	if options.SkipNameValidation == nil || !*options.SkipNameValidation {
+		if validator, ok := mgr.(manager.ControllerNameValidator); ok {
+			if err := validator.ValidateControllerName(name); err != nil {
+				return nil, err
+			}
+		}
+	}
+
 	// Add the controller as a Manager components
 	return c, mgr.Add(c)
 }
@@ -190,12 +199,6 @@ func NewTypedUnmanaged[request comparable](name string, options TypedOptions[req
 		return nil, fmt.Errorf("must specify Name for Controller")
 	}
 
-	if options.SkipNameValidation == nil || !*options.SkipNameValidation {
-		if err := checkName(name); err != nil {
-			return nil, err
-		}
-	}
-
 	if options.LogConstructor == nil {
 		log := options.Logger.WithValues(
 			"controller", name,

pkg/controller/controller_test.go

@@ -136,6 +136,30 @@ var _ = Describe("controller.Controller", func() {
 			Expect(c2).ToNot(BeNil())
 		})
 
+		It("should allow controllers with same name in different manager instances", func() {
+			// Create two separate manager instances
+			m1, err := manager.New(cfg, manager.Options{})
+			Expect(err).NotTo(HaveOccurred())
+
+			m2, err := manager.New(cfg, manager.Options{})
+			Expect(err).NotTo(HaveOccurred())
+
+			// Both managers should be able to have controllers with the same name
+			c1, err := controller.New("c1", m1, controller.Options{Reconciler: rec})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(c1).ToNot(BeNil())
+
+			c2, err := controller.New("c1", m2, controller.Options{Reconciler: rec})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(c2).ToNot(BeNil())
+
+			// Verify that trying to create another controller with the same name in the same manager fails
+			c3, err := controller.New("c1", m1, controller.Options{Reconciler: rec})
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("controller with name c1 already exists in this manager"))
+			Expect(c3).To(BeNil())
+		})
+
 		It("should not leak goroutines when stopped", func() {
 			currentGRs := goleak.IgnoreCurrent()
 

pkg/controller/name.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/leaderelection"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
@@ -110,6 +111,10 @@ type controllerManager struct {
 	// If none is set, it defaults to log.Log global logger.
 	logger logr.Logger
 
+	// usedControllerNames tracks controller names within this manager instance
+	usedControllerNames sets.Set[string]
+	usedNamesMutex      sync.RWMutex
+
 	// leaderElectionStopped is an internal channel used to signal the stopping procedure that the
 	// LeaderElection.Run(...) function has returned and the shutdown can proceed.
 	leaderElectionStopped chan struct{}
@@ -287,6 +292,23 @@ func (cm *controllerManager) GetControllerOptions() config.Controller {
 	return cm.controllerConfig
 }
 
+// ValidateControllerName validates that a controller name is unique within this manager.
+func (cm *controllerManager) ValidateControllerName(name string) error {
+	cm.usedNamesMutex.Lock()
+	defer cm.usedNamesMutex.Unlock()
+
+	if cm.usedControllerNames == nil {
+		cm.usedControllerNames = sets.Set[string]{}
+	}
+
+	if cm.usedControllerNames.Has(name) {
+		return fmt.Errorf("controller with name %s already exists in this manager. Controller names must be unique within the same controller manager to avoid multiple controllers reporting the same metric. This validation can be disabled via the SkipNameValidation option", name)
+	}
+
+	cm.usedControllerNames.Insert(name)
+	return nil
+}
+
 func (cm *controllerManager) addHealthProbeServer() error {
 	mux := http.NewServeMux()
 	srv := httpserver.New(mux)

pkg/manager/internal.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/utils/ptr"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/cluster"
@@ -95,6 +96,18 @@ type Manager interface {
 
 	// GetControllerOptions returns controller global configuration options.
 	GetControllerOptions() config.Controller
+
+	ControllerNameValidator
+}
+
+// ControllerNameValidator defines the interface for validating controller names
+type ControllerNameValidator interface {
+	// ValidateControllerName validates that a controller name is unique within this manager.
+	// Returns an error if the name is already in use.
+	// This validation can be skipped by setting SkipNameValidation to true in either:
+	// - Manager options (config.Controller.SkipNameValidation) - applies to all controllers
+	// - Controller options (controller.Options.SkipNameValidation) - overrides manager setting
+	ValidateControllerName(name string) error
 }
 
 // Options are the arguments for creating a new Manager.
@@ -428,6 +441,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		metricsServer:                 metricsServer,
 		controllerConfig:              options.Controller,
 		logger:                        options.Logger,
+		usedControllerNames:           sets.Set[string]{},
 		elected:                       make(chan struct{}),
 		webhookServer:                 options.WebhookServer,
 		leaderElectionID:              options.LeaderElectionID,