Skip to content

Commit 6cfbed3

Browse files
author
Shawn Hurley
committed
🐛 EnqueueRequestForOwner correctly enqueue cluster-scoped owner
1 parent 0dd6edb commit 6cfbed3

File tree

6 files changed

+113
-7
lines changed

6 files changed

+113
-7
lines changed

‎Gopkg.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/handler/enqueue_owner.go‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package handler
1919
import (
2020
"fmt"
2121

22+
"k8s.io/apimachinery/pkg/api/meta"
2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2324
"k8s.io/apimachinery/pkg/runtime"
2425
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -51,6 +52,9 @@ type EnqueueRequestForOwner struct {
5152

5253
// groupKind is the cached Group and Kind from OwnerType
5354
groupKind schema.GroupKind
55+
56+
// mapper maps GroupVersionKinds to Resources
57+
mapper meta.RESTMapper
5458
}
5559

5660
// Create implements EventHandler
@@ -126,10 +130,21 @@ func (e *EnqueueRequestForOwner) getOwnerReconcileRequest(object metav1.Object)
126130
// object in the event.
127131
if ref.Kind == e.groupKind.Kind && refGV.Group == e.groupKind.Group {
128132
// Match found - add a Request for the object referred to in the OwnerReference
129-
result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{
130-
Namespace: object.GetNamespace(),
131-
Name: ref.Name,
132-
}})
133+
request := reconcile.Request{NamespacedName: types.NamespacedName{
134+
Name: ref.Name,
135+
}}
136+
137+
// if owner is not namespaced then we should set the namespace to the empty
138+
mapping, err := e.mapper.RESTMapping(e.groupKind, refGV.Version)
139+
if err != nil {
140+
log.Error(err, "Could not retrieve rest mapping", "kind", e.groupKind)
141+
return nil
142+
}
143+
if mapping.Scope.Name() != meta.RESTScopeNameRoot {
144+
request.Namespace = object.GetNamespace()
145+
}
146+
147+
result = append(result, request)
133148
}
134149
}
135150

@@ -163,3 +178,11 @@ var _ inject.Scheme = &EnqueueRequestForOwner{}
163178
func (e *EnqueueRequestForOwner) InjectScheme(s *runtime.Scheme) error {
164179
return e.parseOwnerTypeGroupKind(s)
165180
}
181+
182+
var _ inject.Mapper = &EnqueueRequestForOwner{}
183+
184+
// InjectMapper is called by the Controller to provide the rest mapper used by the manager.
185+
func (e *EnqueueRequestForOwner) InjectMapper(m meta.RESTMapper) error {
186+
e.mapper = m
187+
return nil
188+
}

‎pkg/handler/eventhandler_suite_test.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
. "github.com/onsi/ginkgo"
2323
. "github.com/onsi/gomega"
24+
"k8s.io/client-go/rest"
2425
"sigs.k8s.io/controller-runtime/pkg/envtest"
2526
logf "sigs.k8s.io/controller-runtime/pkg/log"
2627
"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -31,6 +32,18 @@ func TestEventhandler(t *testing.T) {
3132
RunSpecsWithDefaultAndCustomReporters(t, "Eventhandler Suite", []Reporter{envtest.NewlineReporter{}})
3233
}
3334

35+
var testenv *envtest.Environment
36+
var cfg *rest.Config
37+
3438
var _ = BeforeSuite(func() {
3539
logf.SetLogger(zap.LoggerTo(GinkgoWriter, true))
40+
41+
testenv = &envtest.Environment{}
42+
var err error
43+
cfg, err = testenv.Start()
44+
Expect(err).NotTo(HaveOccurred())
45+
})
46+
47+
var _ = AfterSuite(func() {
48+
testenv.Stop()
3649
})

‎pkg/handler/eventhandler_test.go‎

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@ import (
2020
. "github.com/onsi/ginkgo"
2121
. "github.com/onsi/gomega"
2222
appsv1 "k8s.io/api/apps/v1"
23+
autoscalingv1 "k8s.io/api/autoscaling/v1"
2324
corev1 "k8s.io/api/core/v1"
25+
"k8s.io/apimachinery/pkg/api/meta"
2426
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2527
"k8s.io/apimachinery/pkg/types"
2628
"k8s.io/client-go/kubernetes/scheme"
2729
"k8s.io/client-go/util/workqueue"
30+
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
2831
"sigs.k8s.io/controller-runtime/pkg/controller/controllertest"
2932
"sigs.k8s.io/controller-runtime/pkg/event"
3033
"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -35,13 +38,19 @@ var _ = Describe("Eventhandler", func() {
3538
var q workqueue.RateLimitingInterface
3639
var instance handler.EnqueueRequestForObject
3740
var pod *corev1.Pod
41+
var mapper meta.RESTMapper
3842
t := true
3943
BeforeEach(func() {
4044
q = controllertest.Queue{Interface: workqueue.New()}
4145
instance = handler.EnqueueRequestForObject{}
4246
pod = &corev1.Pod{
4347
ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "baz"},
4448
}
49+
Expect(cfg).NotTo(BeNil())
50+
51+
var err error
52+
mapper, err = apiutil.NewDiscoveryRESTMapper(cfg)
53+
Expect(err).ShouldNot(HaveOccurred())
4554
})
4655

4756
Describe("EnqueueRequestForObject", func() {
@@ -347,6 +356,7 @@ var _ = Describe("Eventhandler", func() {
347356
OwnerType: &appsv1.ReplicaSet{},
348357
}
349358
instance.InjectScheme(scheme.Scheme)
359+
instance.InjectMapper(mapper)
350360

351361
pod.OwnerReferences = []metav1.OwnerReference{
352362
{
@@ -372,6 +382,7 @@ var _ = Describe("Eventhandler", func() {
372382
OwnerType: &appsv1.ReplicaSet{},
373383
}
374384
instance.InjectScheme(scheme.Scheme)
385+
instance.InjectMapper(mapper)
375386

376387
pod.OwnerReferences = []metav1.OwnerReference{
377388
{
@@ -401,6 +412,7 @@ var _ = Describe("Eventhandler", func() {
401412
OwnerType: &appsv1.ReplicaSet{},
402413
}
403414
instance.InjectScheme(scheme.Scheme)
415+
instance.InjectMapper(mapper)
404416

405417
pod.OwnerReferences = []metav1.OwnerReference{
406418
{
@@ -439,6 +451,7 @@ var _ = Describe("Eventhandler", func() {
439451
OwnerType: &appsv1.ReplicaSet{},
440452
}
441453
instance.InjectScheme(scheme.Scheme)
454+
instance.InjectMapper(mapper)
442455

443456
pod.OwnerReferences = []metav1.OwnerReference{
444457
{
@@ -465,6 +478,7 @@ var _ = Describe("Eventhandler", func() {
465478
IsController: t,
466479
}
467480
instance.InjectScheme(scheme.Scheme)
481+
instance.InjectMapper(mapper)
468482
pod.OwnerReferences = []metav1.OwnerReference{
469483
{ // Wrong group
470484
Name: "foo1-parent",
@@ -488,14 +502,15 @@ var _ = Describe("Eventhandler", func() {
488502
It("should enqueue a Request if there are owners matching Group "+
489503
"and Kind with a different version.", func() {
490504
instance := handler.EnqueueRequestForOwner{
491-
OwnerType: &appsv1.ReplicaSet{},
505+
OwnerType: &autoscalingv1.HorizontalPodAutoscaler{},
492506
}
493507
instance.InjectScheme(scheme.Scheme)
508+
instance.InjectMapper(mapper)
494509
pod.OwnerReferences = []metav1.OwnerReference{
495510
{
496511
Name: "foo-parent",
497-
Kind: "ReplicaSet",
498-
APIVersion: "apps/v2",
512+
Kind: "HorizontalPodAutoscaler",
513+
APIVersion: "autoscaling/v2beta1",
499514
},
500515
}
501516
evt := event.CreateEvent{
@@ -510,11 +525,38 @@ var _ = Describe("Eventhandler", func() {
510525
NamespacedName: types.NamespacedName{Namespace: pod.GetNamespace(), Name: "foo-parent"}}))
511526
})
512527

528+
It("should enqueue a Request for a owner that is cluster scoped", func() {
529+
instance := handler.EnqueueRequestForOwner{
530+
OwnerType: &corev1.Node{},
531+
}
532+
instance.InjectScheme(scheme.Scheme)
533+
instance.InjectMapper(mapper)
534+
pod.OwnerReferences = []metav1.OwnerReference{
535+
{
536+
Name: "node-1",
537+
Kind: "Node",
538+
APIVersion: "v1",
539+
},
540+
}
541+
evt := event.CreateEvent{
542+
Object: pod,
543+
Meta: pod.GetObjectMeta(),
544+
}
545+
instance.Create(evt, q)
546+
Expect(q.Len()).To(Equal(1))
547+
548+
i, _ := q.Get()
549+
Expect(i).To(Equal(reconcile.Request{
550+
NamespacedName: types.NamespacedName{Namespace: "", Name: "node-1"}}))
551+
552+
})
553+
513554
It("should not enqueue a Request if there are no owners.", func() {
514555
instance := handler.EnqueueRequestForOwner{
515556
OwnerType: &appsv1.ReplicaSet{},
516557
}
517558
instance.InjectScheme(scheme.Scheme)
559+
instance.InjectMapper(mapper)
518560
evt := event.CreateEvent{
519561
Object: pod,
520562
Meta: pod.GetObjectMeta(),
@@ -531,6 +573,7 @@ var _ = Describe("Eventhandler", func() {
531573
IsController: t,
532574
}
533575
instance.InjectScheme(scheme.Scheme)
576+
instance.InjectMapper(mapper)
534577
pod.OwnerReferences = []metav1.OwnerReference{
535578
{
536579
Name: "foo1-parent",
@@ -577,6 +620,7 @@ var _ = Describe("Eventhandler", func() {
577620
IsController: t,
578621
}
579622
instance.InjectScheme(scheme.Scheme)
623+
instance.InjectMapper(mapper)
580624
pod.OwnerReferences = []metav1.OwnerReference{
581625
{
582626
Name: "foo1-parent",
@@ -608,6 +652,7 @@ var _ = Describe("Eventhandler", func() {
608652
IsController: t,
609653
}
610654
instance.InjectScheme(scheme.Scheme)
655+
instance.InjectMapper(mapper)
611656
evt := event.CreateEvent{
612657
Object: pod,
613658
Meta: pod.GetObjectMeta(),
@@ -623,6 +668,7 @@ var _ = Describe("Eventhandler", func() {
623668
OwnerType: &appsv1.ReplicaSet{},
624669
}
625670
instance.InjectScheme(scheme.Scheme)
671+
instance.InjectMapper(mapper)
626672
pod.OwnerReferences = []metav1.OwnerReference{
627673
{
628674
Name: "foo1-parent",
@@ -665,6 +711,7 @@ var _ = Describe("Eventhandler", func() {
665711
OwnerType: &appsv1.ReplicaSet{},
666712
}
667713
instance.InjectScheme(scheme.Scheme)
714+
instance.InjectMapper(mapper)
668715
pod.OwnerReferences = []metav1.OwnerReference{
669716
{
670717
Name: "foo1-parent",
@@ -686,6 +733,7 @@ var _ = Describe("Eventhandler", func() {
686733
OwnerType: &metav1.ListOptions{},
687734
}
688735
instance.InjectScheme(scheme.Scheme)
736+
instance.InjectMapper(mapper)
689737
pod.OwnerReferences = []metav1.OwnerReference{
690738
{
691739
Name: "foo1-parent",
@@ -707,6 +755,7 @@ var _ = Describe("Eventhandler", func() {
707755
OwnerType: &controllertest.ErrorType{},
708756
}
709757
instance.InjectScheme(scheme.Scheme)
758+
instance.InjectMapper(mapper)
710759
pod.OwnerReferences = []metav1.OwnerReference{
711760
{
712761
Name: "foo1-parent",
@@ -727,6 +776,7 @@ var _ = Describe("Eventhandler", func() {
727776
It("should do nothing.", func() {
728777
instance := handler.EnqueueRequestForOwner{}
729778
instance.InjectScheme(scheme.Scheme)
779+
instance.InjectMapper(mapper)
730780
pod.OwnerReferences = []metav1.OwnerReference{
731781
{
732782
Name: "foo1-parent",
@@ -749,6 +799,7 @@ var _ = Describe("Eventhandler", func() {
749799
OwnerType: &appsv1.ReplicaSet{},
750800
}
751801
instance.InjectScheme(scheme.Scheme)
802+
instance.InjectMapper(mapper)
752803
pod.OwnerReferences = []metav1.OwnerReference{
753804
{
754805
Name: "foo1-parent",

‎pkg/manager/internal.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@ func (cm *controllerManager) SetFields(i interface{}) error {
139139
if _, err := inject.DecoderInto(cm.admissionDecoder, i); err != nil {
140140
return err
141141
}
142+
if _, err := inject.MapperInto(cm.mapper, i); err != nil {
143+
return err
144+
}
142145
return nil
143146
}
144147

‎pkg/runtime/inject/inject.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package inject
1818

1919
import (
20+
"k8s.io/apimachinery/pkg/api/meta"
2021
"k8s.io/apimachinery/pkg/runtime"
2122
"k8s.io/client-go/rest"
2223
"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -113,6 +114,20 @@ func StopChannelInto(stop <-chan struct{}, i interface{}) (bool, error) {
113114
return false, nil
114115
}
115116

117+
// Mapper is used to inject the rest mapper to components that may need it
118+
type Mapper interface {
119+
InjectMapper(meta.RESTMapper) error
120+
}
121+
122+
// MapperInto will set the rest mapper on i and return the result if it implements Mapper.
123+
// Returns false if i does not implement Mapper.
124+
func MapperInto(mapper meta.RESTMapper, i interface{}) (bool, error) {
125+
if m, ok := i.(Mapper); ok {
126+
return true, m.InjectMapper(mapper)
127+
}
128+
return false, nil
129+
}
130+
116131
// Func injects dependencies into i.
117132
type Func func(i interface{}) error
118133

0 commit comments

Comments
 (0)