Remove SetFields from Manager and Controller

Signed-off-by: Vince Prignano <vincepri@redhat.com>
kubernetes-sigs · Jan 12, 2023 · 2bfe2e1 · 2bfe2e1
1 parent 4b67697
commit 2bfe2e1
Show file tree

Hide file tree

Showing 12 changed files with 46 additions and 334 deletions.
diff --git a/pkg/builder/controller.go b/pkg/builder/controller.go
@@ -255,7 +255,7 @@ func (blder *Builder) doWatch() error {
 		allPredicates = append(allPredicates, w.predicates...)
 
 		// If the source of this watch is of type *source.Kind, project it.
-		if srckind, ok := w.src.(source.SyncingSource); ok {
+		if srckind, ok := w.src.(source.TypedSource); ok {
 			if w.objectProjection == projectAsMetadata {
 				if err := source.KindAsPartialMetadata(srckind, blder.mgr.GetScheme()); err != nil {
 					return err

diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go
@@ -37,11 +37,6 @@ import (
 
 // Cluster provides various methods to interact with a cluster.
 type Cluster interface {
-	// SetFields will set any dependencies on an object for which the object has implemented the inject
-	// interface - e.g. inject.Client.
-	// Deprecated: use the equivalent Options field to set a field. This method will be removed in v0.10.
-	SetFields(interface{}) error
-
 	// GetConfig returns an initialized Config
 	GetConfig() *rest.Config
 

diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go
@@ -29,11 +29,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
-	"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
 	intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
 var _ = Describe("cluster.Cluster", func() {
@@ -111,47 +108,6 @@ var _ = Describe("cluster.Cluster", func() {
 		})
 	})
 
-	Describe("SetFields", func() {
-		It("should inject field values", func() {
-			c, err := New(cfg, func(o *Options) {
-				o.NewCache = func(_ *rest.Config, _ cache.Options) (cache.Cache, error) {
-					return &informertest.FakeInformers{}, nil
-				}
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			By("Injecting the dependencies")
-			err = c.SetFields(&injectable{
-				scheme: func(scheme *runtime.Scheme) error {
-					defer GinkgoRecover()
-					Expect(scheme).To(Equal(c.GetScheme()))
-					return nil
-				},
-				client: func(client client.Client) error {
-					defer GinkgoRecover()
-					Expect(client).To(Equal(c.GetClient()))
-					return nil
-				},
-				log: func(logger logr.Logger) error {
-					defer GinkgoRecover()
-					Expect(logger).To(Equal(logf.RuntimeLog.WithName("cluster")))
-					return nil
-				},
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			By("Returning an error if dependency injection fails")
-
-			expected := fmt.Errorf("expected error")
-			err = c.SetFields(&injectable{
-				scheme: func(scheme *runtime.Scheme) error {
-					return expected
-				},
-			})
-			Expect(err).To(Equal(expected))
-		})
-	})
-
 	It("should not leak goroutines when stopped", func() {
 		currentGRs := goleak.IgnoreCurrent()
 
@@ -211,37 +167,3 @@ var _ = Describe("cluster.Cluster", func() {
 		Expect(c.GetAPIReader()).NotTo(BeNil())
 	})
 })
-
-var _ inject.Scheme = &injectable{}
-var _ inject.Logger = &injectable{}
-
-type injectable struct {
-	scheme func(scheme *runtime.Scheme) error
-	client func(client.Client) error
-	log    func(logger logr.Logger) error
-}
-
-func (i *injectable) InjectClient(c client.Client) error {
-	if i.client == nil {
-		return nil
-	}
-	return i.client(c)
-}
-
-func (i *injectable) InjectScheme(scheme *runtime.Scheme) error {
-	if i.scheme == nil {
-		return nil
-	}
-	return i.scheme(scheme)
-}
-
-func (i *injectable) InjectLogger(log logr.Logger) error {
-	if i.log == nil {
-		return nil
-	}
-	return i.log(log)
-}
-
-func (i *injectable) Start(<-chan struct{}) error {
-	return nil
-}
diff --git a/pkg/cluster/internal.go b/pkg/cluster/internal.go
@@ -28,7 +28,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
 type cluster struct {
@@ -64,13 +63,6 @@ type cluster struct {
 	logger logr.Logger
 }
 
-func (c *cluster) SetFields(i interface{}) error {
-	if _, err := inject.SchemeInto(c.scheme, i); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (c *cluster) GetConfig() *rest.Config {
 	return c.config
 }

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -139,11 +139,6 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
 		options.RateLimiter = workqueue.DefaultControllerRateLimiter()
 	}
 
-	// Inject dependencies into Reconciler
-	if err := mgr.SetFields(options.Reconciler); err != nil {
-		return nil, err
-	}
-
 	if options.RecoverPanic == nil {
 		options.RecoverPanic = mgr.GetControllerOptions().RecoverPanic
 	}

diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
@@ -18,14 +18,12 @@ package controller_test
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"go.uber.org/goleak"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/pointer"
 
 	"sigs.k8s.io/controller-runtime/pkg/config/v1alpha1"
@@ -61,16 +59,6 @@ var _ = Describe("controller.Controller", func() {
 			Expect(err.Error()).To(ContainSubstring("must specify Reconciler"))
 		})
 
-		It("NewController should return an error if injecting Reconciler fails", func() {
-			m, err := manager.New(cfg, manager.Options{})
-			Expect(err).NotTo(HaveOccurred())
-
-			c, err := controller.New("foo", m, controller.Options{Reconciler: &failRec{}})
-			Expect(c).To(BeNil())
-			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("expected error"))
-		})
-
 		It("should not return an error if two controllers are registered with different names", func() {
 			m, err := manager.New(cfg, manager.Options{})
 			Expect(err).NotTo(HaveOccurred())
@@ -223,15 +211,3 @@ var _ = Describe("controller.Controller", func() {
 		})
 	})
 })
-
-var _ reconcile.Reconciler = &failRec{}
-
-type failRec struct{}
-
-func (*failRec) Reconcile(context.Context, reconcile.Request) (reconcile.Result, error) {
-	return reconcile.Result{}, nil
-}
-
-func (*failRec) InjectScheme(*runtime.Scheme) error {
-	return fmt.Errorf("expected error")
-}
diff --git a/pkg/handler/enqueue_owner.go b/pkg/handler/enqueue_owner.go
@@ -46,9 +46,9 @@ type OwnerOption func(e *enqueueRequestForOwner)
 // - a source.Kind Source with Type of Pod.
 //
 // - a handler.enqueueRequestForOwner EventHandler with an OwnerType of ReplicaSet and OnlyControllerOwner set to true.
-func EnqueueRequestForOwner(scheme *runtime.Scheme, mapper meta.RESTMapper, owner client.Object, opts ...OwnerOption) EventHandler {
+func EnqueueRequestForOwner(scheme *runtime.Scheme, mapper meta.RESTMapper, ownerType client.Object, opts ...OwnerOption) EventHandler {
 	e := &enqueueRequestForOwner{
-		ownerType: owner,
+		ownerType: ownerType,
 		mapper:    mapper,
 	}
 	if err := e.parseOwnerTypeGroupKind(scheme); err != nil {

diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go
@@ -192,18 +192,18 @@ func (cm *controllerManager) Add(r Runnable) error {
 
 func (cm *controllerManager) add(r Runnable) error {
 	// Set dependencies on the object
-	if err := cm.SetFields(r); err != nil {
+	if err := cm.setFields(r); err != nil {
 		return err
 	}
 	return cm.runnables.Add(r)
 }
 
 // Deprecated: use the equivalent Options field to set a field. This method will be removed in v0.10.
-func (cm *controllerManager) SetFields(i interface{}) error {
-	if err := cm.cluster.SetFields(i); err != nil {
+func (cm *controllerManager) setFields(i interface{}) error {
+	if _, err := inject.SchemeInto(cm.cluster.GetScheme(), i); err != nil {
 		return err
 	}
-	if _, err := inject.InjectorInto(cm.SetFields, i); err != nil {
+	if _, err := inject.InjectorInto(cm.setFields, i); err != nil {
 		return err
 	}
 	if _, err := inject.LoggerInto(cm.logger, i); err != nil {

diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go
@@ -51,11 +51,9 @@ import (
 	intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
 	"sigs.k8s.io/controller-runtime/pkg/leaderelection"
 	fakeleaderelection "sigs.k8s.io/controller-runtime/pkg/leaderelection/fake"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/recorder"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
@@ -1516,58 +1514,6 @@ var _ = Describe("manger.Manager", func() {
 
 		})
 	})
-	Describe("SetFields", func() {
-		It("should inject field values", func() {
-			m, err := New(cfg, Options{
-				NewCache: func(_ *rest.Config, _ cache.Options) (cache.Cache, error) {
-					return &informertest.FakeInformers{}, nil
-				},
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			By("Injecting the dependencies")
-			err = m.SetFields(&injectable{
-				scheme: func(scheme *runtime.Scheme) error {
-					defer GinkgoRecover()
-					Expect(scheme).To(Equal(m.GetScheme()))
-					return nil
-				},
-				client: func(client client.Client) error {
-					defer GinkgoRecover()
-					Expect(client).To(Equal(m.GetClient()))
-					return nil
-				},
-				f: func(f inject.Func) error {
-					defer GinkgoRecover()
-					Expect(f).NotTo(BeNil())
-					return nil
-				},
-				log: func(logger logr.Logger) error {
-					defer GinkgoRecover()
-					Expect(logger).To(Equal(log.Log))
-					return nil
-				},
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			By("Returning an error if dependency injection fails")
-
-			expected := fmt.Errorf("expected error")
-			err = m.SetFields(&injectable{
-				scheme: func(scheme *runtime.Scheme) error {
-					return expected
-				},
-			})
-			Expect(err).To(Equal(expected))
-
-			err = m.SetFields(&injectable{
-				f: func(c inject.Func) error {
-					return expected
-				},
-			})
-			Expect(err).To(Equal(expected))
-		})
-	})
 
 	It("should not leak goroutines when stopped", func() {
 		currentGRs := goleak.IgnoreCurrent()
@@ -1693,49 +1639,6 @@ func (*failRec) InjectScheme(*runtime.Scheme) error {
 	return fmt.Errorf("expected error")
 }
 
-var _ inject.Injector = &injectable{}
-var _ inject.Scheme = &injectable{}
-var _ inject.Logger = &injectable{}
-
-type injectable struct {
-	scheme func(scheme *runtime.Scheme) error
-	client func(client.Client) error
-	f      func(inject.Func) error
-	log    func(logger logr.Logger) error
-}
-
-func (i *injectable) InjectClient(c client.Client) error {
-	if i.client == nil {
-		return nil
-	}
-	return i.client(c)
-}
-
-func (i *injectable) InjectScheme(scheme *runtime.Scheme) error {
-	if i.scheme == nil {
-		return nil
-	}
-	return i.scheme(scheme)
-}
-
-func (i *injectable) InjectFunc(f inject.Func) error {
-	if i.f == nil {
-		return nil
-	}
-	return i.f(f)
-}
-
-func (i *injectable) InjectLogger(log logr.Logger) error {
-	if i.log == nil {
-		return nil
-	}
-	return i.log(log)
-}
-
-func (i *injectable) Start(<-chan struct{}) error {
-	return nil
-}
-
 type runnableError struct {
 }