pkg/reconciler: fix dependent watches

Author: joelanford

pkg/reconciler/internal/hook/hook.go

@@ -24,7 +24,6 @@ import (
 	"helm.sh/helm/v3/pkg/releaseutil"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -37,10 +36,9 @@ import (
 	"github.com/joelanford/helm-operator/pkg/internal/sdk/predicate"
 )
 
-func NewDependentResourceWatcher(c controller.Controller, rm meta.RESTMapper, owner runtime.Object) hook.PostHook {
+func NewDependentResourceWatcher(c controller.Controller, rm meta.RESTMapper) hook.PostHook {
 	return &dependentResourceWatcher{
 		controller: c,
-		owner:      owner,
 		restMapper: rm,
 		m:          sync.Mutex{},
 		watches:    make(map[schema.GroupVersionKind]struct{}),
@@ -49,14 +47,13 @@ func NewDependentResourceWatcher(c controller.Controller, rm meta.RESTMapper, ow
 
 type dependentResourceWatcher struct {
 	controller controller.Controller
-	owner      runtime.Object
 	restMapper meta.RESTMapper
 
 	m       sync.Mutex
 	watches map[schema.GroupVersionKind]struct{}
 }
 
-func (d *dependentResourceWatcher) Exec(_ *unstructured.Unstructured, rel release.Release, log logr.Logger) error {
+func (d *dependentResourceWatcher) Exec(owner *unstructured.Unstructured, rel release.Release, log logr.Logger) error {
 	// using predefined functions for filtering events
 	dependentPredicate := predicate.DependentPredicateFuncs()
 
@@ -75,17 +72,22 @@ func (d *dependentResourceWatcher) Exec(_ *unstructured.Unstructured, rel releas
 			continue
 		}
 
-		useOwnerRef, err := controllerutil.SupportsOwnerReference(d.restMapper, d.owner, &obj)
+		useOwnerRef, err := controllerutil.SupportsOwnerReference(d.restMapper, owner, &obj)
 		if err != nil {
 			return err
 		}
 
 		if useOwnerRef {
-			if err := d.controller.Watch(&source.Kind{Type: &obj}, &handler.EnqueueRequestForOwner{OwnerType: d.owner}, dependentPredicate); err != nil {
+			if err := d.controller.Watch(&source.Kind{Type: &obj}, &handler.EnqueueRequestForOwner{
+				OwnerType:    owner,
+				IsController: true,
+			}, dependentPredicate); err != nil {
 				return err
 			}
 		} else {
-			if err := d.controller.Watch(&source.Kind{Type: &obj}, &sdkhandler.EnqueueRequestForAnnotation{Type: d.owner.GetObjectKind().GroupVersionKind().GroupKind().String()}, dependentPredicate); err != nil {
+			if err := d.controller.Watch(&source.Kind{Type: &obj}, &sdkhandler.EnqueueRequestForAnnotation{
+				Type: owner.GetObjectKind().GroupVersionKind().GroupKind().String(),
+			}, dependentPredicate); err != nil {
 				return err
 			}
 		}

pkg/reconciler/internal/hook/hook_test.go

@@ -66,22 +66,22 @@ var _ = Describe("Hook", func() {
 				rel = &release.Release{
 					Manifest: strings.Join([]string{rsOwnerNamespace}, "---\n"),
 				}
-				drw = internalhook.NewDependentResourceWatcher(c, rm, owner)
+				drw = internalhook.NewDependentResourceWatcher(c, rm)
 			})
 			It("should fail with an invalid release manifest", func() {
 				rel.Manifest = "---\nfoobar"
-				err := drw.Exec(nil, *rel, log)
+				err := drw.Exec(owner, *rel, log)
 				Expect(err).NotTo(BeNil())
 			})
 			It("should fail with unknown owner kind", func() {
-				Expect(drw.Exec(nil, *rel, log)).To(MatchError(&meta.NoKindMatchError{
+				Expect(drw.Exec(owner, *rel, log)).To(MatchError(&meta.NoKindMatchError{
 					GroupKind:        schema.GroupKind{Group: "apps", Kind: "Deployment"},
 					SearchedVersions: []string{"v1"},
 				}))
 			})
 			It("should fail with unknown dependent kind", func() {
 				rm.Add(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, meta.RESTScopeNamespace)
-				Expect(drw.Exec(nil, *rel, log)).To(MatchError(&meta.NoKindMatchError{
+				Expect(drw.Exec(owner, *rel, log)).To(MatchError(&meta.NoKindMatchError{
 					GroupKind:        schema.GroupKind{Group: "apps", Kind: "ReplicaSet"},
 					SearchedVersions: []string{"v1"},
 				}))
@@ -110,8 +110,8 @@ var _ = Describe("Hook", func() {
 				rel = &release.Release{
 					Manifest: strings.Join([]string{clusterRole, clusterRole, rsOwnerNamespace, rsOwnerNamespace}, "---\n"),
 				}
-				drw = internalhook.NewDependentResourceWatcher(c, rm, owner)
-				Expect(drw.Exec(nil, *rel, log)).To(Succeed())
+				drw = internalhook.NewDependentResourceWatcher(c, rm)
+				Expect(drw.Exec(owner, *rel, log)).To(Succeed())
 				Expect(c.WatchCalls).To(HaveLen(2))
 				Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
 				Expect(c.WatchCalls[1].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
@@ -133,8 +133,8 @@ var _ = Describe("Hook", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{rsOwnerNamespace, ssOtherNamespace}, "---\n"),
 					}
-					drw = internalhook.NewDependentResourceWatcher(c, rm, owner)
-					Expect(drw.Exec(nil, *rel, log)).To(Succeed())
+					drw = internalhook.NewDependentResourceWatcher(c, rm)
+					Expect(drw.Exec(owner, *rel, log)).To(Succeed())
 					Expect(c.WatchCalls).To(HaveLen(2))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
 					Expect(c.WatchCalls[1].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
@@ -144,8 +144,8 @@ var _ = Describe("Hook", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{clusterRole, clusterRoleBinding}, "---\n"),
 					}
-					drw = internalhook.NewDependentResourceWatcher(c, rm, owner)
-					Expect(drw.Exec(nil, *rel, log)).To(Succeed())
+					drw = internalhook.NewDependentResourceWatcher(c, rm)
+					Expect(drw.Exec(owner, *rel, log)).To(Succeed())
 					Expect(c.WatchCalls).To(HaveLen(2))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
 					Expect(c.WatchCalls[1].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
@@ -170,8 +170,8 @@ var _ = Describe("Hook", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{rsOwnerNamespace}, "---\n"),
 					}
-					drw = internalhook.NewDependentResourceWatcher(c, rm, owner)
-					Expect(drw.Exec(nil, *rel, log)).To(Succeed())
+					drw = internalhook.NewDependentResourceWatcher(c, rm)
+					Expect(drw.Exec(owner, *rel, log)).To(Succeed())
 					Expect(c.WatchCalls).To(HaveLen(1))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{}))
 				})
@@ -180,8 +180,8 @@ var _ = Describe("Hook", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{clusterRole}, "---\n"),
 					}
-					drw = internalhook.NewDependentResourceWatcher(c, rm, owner)
-					Expect(drw.Exec(nil, *rel, log)).To(Succeed())
+					drw = internalhook.NewDependentResourceWatcher(c, rm)
+					Expect(drw.Exec(owner, *rel, log)).To(Succeed())
 					Expect(c.WatchCalls).To(HaveLen(1))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
 				})
@@ -190,8 +190,8 @@ var _ = Describe("Hook", func() {
 					rel = &release.Release{
 						Manifest: strings.Join([]string{ssOtherNamespace}, "---\n"),
 					}
-					drw = internalhook.NewDependentResourceWatcher(c, rm, owner)
-					Expect(drw.Exec(nil, *rel, log)).To(Succeed())
+					drw = internalhook.NewDependentResourceWatcher(c, rm)
+					Expect(drw.Exec(owner, *rel, log)).To(Succeed())
 					Expect(c.WatchCalls).To(HaveLen(1))
 					Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{}))
 				})

pkg/reconciler/reconciler.go

@@ -770,7 +770,7 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err
 	}
 
 	if !r.skipDependentWatches {
-		r.postHooks = append([]hook.PostHook{internalhook.NewDependentResourceWatcher(c, mgr.GetRESTMapper(), obj)}, r.postHooks...)
+		r.postHooks = append([]hook.PostHook{internalhook.NewDependentResourceWatcher(c, mgr.GetRESTMapper())}, r.postHooks...)
 	}
 	return nil
 }