Skip to content

Commit

Permalink
Test that For and Owns can only be used with reconcile.Request
Browse files Browse the repository at this point in the history
  • Loading branch information
alvaroaleman committed Jun 28, 2024
1 parent 10a38de commit 8cee9f3
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 20 deletions.
6 changes: 1 addition & 5 deletions pkg/builder/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func (blder *TypedBuilder[request]) doWatch() error {
}

if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) {
return fmt.Errorf("For() can only be used with reconcile.Request, got %T", reflect.TypeFor[request]())
return fmt.Errorf("For() can only be used with reconcile.Request, got %T", *new(request))
}

var hdler handler.TypedEventHandler[client.Object, request]
Expand All @@ -340,10 +340,6 @@ func (blder *TypedBuilder[request]) doWatch() error {
opts = append(opts, handler.OnlyControllerOwner())
}

if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) {
return errors.New("Owns() is not supported for TypedBuilder")
}

var hdler handler.TypedEventHandler[client.Object, request]
reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(handler.EnqueueRequestForOwner(
blder.mgr.GetScheme(), blder.mgr.GetRESTMapper(),
Expand Down
49 changes: 34 additions & 15 deletions pkg/builder/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ import (

var _ untypedWatchesInput = (*WatchesInput[struct{}])(nil)

type typedNoop struct{}

func (typedNoop) Reconcile(context.Context, reconcile.Request) (reconcile.Result, error) {
return reconcile.Result{}, nil
}

type testLogger struct {
logr.Logger
}
Expand All @@ -77,10 +71,15 @@ func (l *testLogger) WithName(name string) logr.LogSink {
return l
}

type empty struct{}

var _ = Describe("application", func() {
noop := reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
return reconcile.Result{}, nil
})
typedNoop := reconcile.TypedFunc[empty](func(context.Context, empty) (reconcile.Result, error) {
return reconcile.Result{}, nil
})

Describe("New", func() {
It("should return success if given valid objects", func() {
Expand Down Expand Up @@ -179,6 +178,34 @@ var _ = Describe("application", func() {
// manifest when we try to default the controller name, which is good to double check.
})

It("should return error if in For is used with a custom request type", func() {
By("creating a controller manager")
m, err := manager.New(cfg, manager.Options{})
Expect(err).NotTo(HaveOccurred())

instance, err := TypedControllerManagedBy[empty](m).
For(&appsv1.ReplicaSet{}).
Build(typedNoop)
Expect(err).To(MatchError(ContainSubstring("For() can only be used with reconcile.Request, got builder.empty")))
Expect(instance).To(BeNil())
})

It("should return error if in Owns is used with a custom request type", func() {
By("creating a controller manager")
m, err := manager.New(cfg, manager.Options{})
Expect(err).NotTo(HaveOccurred())

instance, err := TypedControllerManagedBy[empty](m).
Named("my_controller").
Owns(&appsv1.ReplicaSet{}).
Build(typedNoop)
// If we ever allow Owns() without For() we need to update the code to error
// out on Owns() if the request type is different from reconcile.Request just
// like we do in For().
Expect(err).To(MatchError("Owns() can only be used together with For()"))
Expect(instance).To(BeNil())
})

It("should return an error if it cannot create the controller", func() {

By("creating a controller manager")
Expand Down Expand Up @@ -303,22 +330,14 @@ var _ = Describe("application", func() {
})

It("should not allow multiple reconcilers during creation of controller", func() {
newController := func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) {
if options.Reconciler != (typedNoop{}) {
return nil, fmt.Errorf("Custom reconciler expected %T but found %T", typedNoop{}, options.Reconciler)
}
return controller.New(name, mgr, options)
}

By("creating a controller manager")
m, err := manager.New(cfg, manager.Options{})
Expect(err).NotTo(HaveOccurred())

builder := ControllerManagedBy(m).
For(&appsv1.ReplicaSet{}).
Owns(&appsv1.ReplicaSet{}).
WithOptions(controller.Options{Reconciler: typedNoop{}})
builder.newController = newController
WithOptions(controller.Options{Reconciler: noop})
instance, err := builder.Build(noop)
Expect(err).To(HaveOccurred())
Expect(instance).To(BeNil())
Expand Down

0 comments on commit 8cee9f3

Please sign in to comment.