Skip to content

Commit

Permalink
Remove SetFields from Manager and Controller
Browse files Browse the repository at this point in the history
Signed-off-by: Vince Prignano <vincepri@redhat.com>
  • Loading branch information
vincepri committed Jan 18, 2023
1 parent b324b0b commit ea1fcf3
Show file tree
Hide file tree
Showing 16 changed files with 152 additions and 461 deletions.
34 changes: 20 additions & 14 deletions pkg/builder/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ import (
"strings"

"github.com/go-logr/logr"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"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/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
internalsource "sigs.k8s.io/controller-runtime/pkg/internal/source"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -196,16 +198,18 @@ func (blder *Builder) Build(r reconcile.Reconciler) (controller.Controller, erro
return blder.ctrl, nil
}

func (blder *Builder) project(obj client.Object, proj objectProjection) (source.Source, error) {
src := source.Kind(blder.mgr.GetCache(), obj)
func (blder *Builder) project(obj client.Object, proj objectProjection) (client.Object, error) {
switch proj {
case projectAsNormal:
return src, nil
return obj, nil
case projectAsMetadata:
if err := source.KindAsPartialMetadata(src, blder.mgr.GetScheme()); err != nil {
return nil, err
metaObj := &metav1.PartialObjectMetadata{}
gvk, err := getGvk(obj, blder.mgr.GetScheme())
if err != nil {
return nil, fmt.Errorf("unable to determine GVK of %T for a metadata-only watch: %w", obj, err)
}
return src, nil
metaObj.SetGroupVersionKind(gvk)
return metaObj, nil
default:
panic(fmt.Sprintf("unexpected projection type %v on type %T, should not be possible since this is an internal field", proj, obj))
}
Expand All @@ -214,10 +218,11 @@ func (blder *Builder) project(obj client.Object, proj objectProjection) (source.
func (blder *Builder) doWatch() error {
// Reconcile type
if blder.forInput.object != nil {
src, err := blder.project(blder.forInput.object, blder.forInput.objectProjection)
obj, err := blder.project(blder.forInput.object, blder.forInput.objectProjection)
if err != nil {
return err
}
src := source.Kind(blder.mgr.GetCache(), obj)
hdler := &handler.EnqueueRequestForObject{}
allPredicates := append(blder.globalPredicates, blder.forInput.predicates...)
if err := blder.ctrl.Watch(src, hdler, allPredicates...); err != nil {
Expand All @@ -230,10 +235,11 @@ func (blder *Builder) doWatch() error {
return errors.New("Owns() can only be used together with For()")
}
for _, own := range blder.ownsInput {
src, err := blder.project(own.object, own.objectProjection)
obj, err := blder.project(own.object, own.objectProjection)
if err != nil {
return err
}
src := source.Kind(blder.mgr.GetCache(), obj)
hdler := handler.EnqueueRequestForOwner(
blder.mgr.GetScheme(), blder.mgr.GetRESTMapper(),
blder.forInput.object,
Expand All @@ -254,13 +260,13 @@ func (blder *Builder) doWatch() error {
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
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 w.objectProjection == projectAsMetadata {
if err := source.KindAsPartialMetadata(srckind, blder.mgr.GetScheme()); err != nil {
return err
}
// If the source of this watch is of type Kind, project it.
if srckind, ok := w.src.(*internalsource.Kind); ok {
typeForSrc, err := blder.project(srckind.Type, w.objectProjection)
if err != nil {
return err
}
srckind.Type = typeForSrc
}

if err := blder.ctrl.Watch(w.src, w.eventhandler, allPredicates...); err != nil {
Expand Down
5 changes: 0 additions & 5 deletions pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
78 changes: 0 additions & 78 deletions pkg/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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
}
8 changes: 0 additions & 8 deletions pkg/cluster/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
24 changes: 0 additions & 24 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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")
}
4 changes: 2 additions & 2 deletions pkg/handler/enqueue_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"k8s.io/client-go/util/workqueue"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/source/internal"
internal "sigs.k8s.io/controller-runtime/pkg/internal/source"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down
Loading

0 comments on commit ea1fcf3

Please sign in to comment.