Skip to content

Commit

Permalink
🐛 EnqueueRequestForOwner correctly enqueue cluster-scoped owner
Browse files Browse the repository at this point in the history
  • Loading branch information
Shawn Hurley committed Jan 3, 2019
1 parent 4c0ea9d commit 667e8d5
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 7 deletions.
1 change: 1 addition & 0 deletions Gopkg.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 27 additions & 4 deletions pkg/handler/enqueue_owner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package handler
import (
"fmt"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -51,6 +52,9 @@ type EnqueueRequestForOwner struct {

// groupKind is the cached Group and Kind from OwnerType
groupKind schema.GroupKind

// mapper maps GroupVersionKinds to Resources
mapper meta.RESTMapper
}

// Create implements EventHandler
Expand Down Expand Up @@ -126,10 +130,21 @@ func (e *EnqueueRequestForOwner) getOwnerReconcileRequest(object metav1.Object)
// object in the event.
if ref.Kind == e.groupKind.Kind && refGV.Group == e.groupKind.Group {
// Match found - add a Request for the object referred to in the OwnerReference
result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{
Namespace: object.GetNamespace(),
Name: ref.Name,
}})
r := reconcile.Request{NamespacedName: types.NamespacedName{
Name: ref.Name,
}}

// if owner is not namespaced then we should set the namespace to the empty
mapping, err := e.mapper.RESTMapping(e.groupKind, refGV.Version)
if err != nil {
log.Error(err, "Could not retrieve rest mapping", "group", e.groupKind.Group, "kindv", e.groupKind.Kind)
return nil
}
if mapping.Scope.Name() != meta.RESTScopeNameRoot {
r.Namespace = object.GetNamespace()
}

result = append(result, r)
}
}

Expand Down Expand Up @@ -163,3 +178,11 @@ var _ inject.Scheme = &EnqueueRequestForOwner{}
func (e *EnqueueRequestForOwner) InjectScheme(s *runtime.Scheme) error {
return e.parseOwnerTypeGroupKind(s)
}

var _ inject.Mapper = &EnqueueRequestForOwner{}

// InjectMapper is called by the Controller to provide the rest mapper used by the manager.
func (e *EnqueueRequestForOwner) InjectMapper(m meta.RESTMapper) error {
e.mapper = m
return nil
}
13 changes: 13 additions & 0 deletions pkg/handler/eventhandler_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/envtest"
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
)
Expand All @@ -30,6 +31,18 @@ func TestEventhandler(t *testing.T) {
RunSpecsWithDefaultAndCustomReporters(t, "Eventhandler Suite", []Reporter{envtest.NewlineReporter{}})
}

var testenv *envtest.Environment
var cfg *rest.Config

var _ = BeforeSuite(func() {
logf.SetLogger(logf.ZapLoggerTo(GinkgoWriter, true))

testenv = &envtest.Environment{}
var err error
cfg, err = testenv.Start()
Expect(err).NotTo(HaveOccurred())
})

var _ = AfterSuite(func() {
testenv.Stop()
})
57 changes: 54 additions & 3 deletions pkg/handler/eventhandler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
autoscalingv1 "k8s.io/api/autoscaling/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/util/workqueue"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/controller/controllertest"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand All @@ -35,13 +38,19 @@ var _ = Describe("Eventhandler", func() {
var q workqueue.RateLimitingInterface
var instance handler.EnqueueRequestForObject
var pod *corev1.Pod
var mapper meta.RESTMapper
t := true
BeforeEach(func() {
q = controllertest.Queue{Interface: workqueue.New()}
instance = handler.EnqueueRequestForObject{}
pod = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "baz"},
}
Expect(cfg).NotTo(BeNil())

var err error
mapper, err = apiutil.NewDiscoveryRESTMapper(cfg)
Expect(err).ShouldNot(HaveOccurred())
})

Describe("EnqueueRequestForObject", func() {
Expand Down Expand Up @@ -347,6 +356,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &appsv1.ReplicaSet{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)

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

pod.OwnerReferences = []metav1.OwnerReference{
{
Expand Down Expand Up @@ -401,6 +412,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &appsv1.ReplicaSet{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)

pod.OwnerReferences = []metav1.OwnerReference{
{
Expand Down Expand Up @@ -439,6 +451,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &appsv1.ReplicaSet{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)

pod.OwnerReferences = []metav1.OwnerReference{
{
Expand All @@ -465,6 +478,7 @@ var _ = Describe("Eventhandler", func() {
IsController: t,
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{ // Wrong group
Name: "foo1-parent",
Expand All @@ -488,14 +502,15 @@ var _ = Describe("Eventhandler", func() {
It("should enqueue a Request if there are owners matching Group "+
"and Kind with a different version.", func() {
instance := handler.EnqueueRequestForOwner{
OwnerType: &appsv1.ReplicaSet{},
OwnerType: &autoscalingv1.HorizontalPodAutoscaler{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo-parent",
Kind: "ReplicaSet",
APIVersion: "apps/v2",
Kind: "HorizontalPodAutoscaler",
APIVersion: "autoscaling/v2beta1",
},
}
evt := event.CreateEvent{
Expand All @@ -510,11 +525,38 @@ var _ = Describe("Eventhandler", func() {
NamespacedName: types.NamespacedName{Namespace: pod.GetNamespace(), Name: "foo-parent"}}))
})

It("should enqueue a Request for a owner that is cluster scoped", func() {
instance := handler.EnqueueRequestForOwner{
OwnerType: &corev1.Node{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "node-1",
Kind: "Node",
APIVersion: "v1",
},
}
evt := event.CreateEvent{
Object: pod,
Meta: pod.GetObjectMeta(),
}
instance.Create(evt, q)
Expect(q.Len()).To(Equal(1))

i, _ := q.Get()
Expect(i).To(Equal(reconcile.Request{
NamespacedName: types.NamespacedName{Namespace: "", Name: "node-1"}}))

})

It("should not enqueue a Request if there are no owners.", func() {
instance := handler.EnqueueRequestForOwner{
OwnerType: &appsv1.ReplicaSet{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
evt := event.CreateEvent{
Object: pod,
Meta: pod.GetObjectMeta(),
Expand All @@ -531,6 +573,7 @@ var _ = Describe("Eventhandler", func() {
IsController: t,
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand Down Expand Up @@ -577,6 +620,7 @@ var _ = Describe("Eventhandler", func() {
IsController: t,
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand Down Expand Up @@ -608,6 +652,7 @@ var _ = Describe("Eventhandler", func() {
IsController: t,
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
evt := event.CreateEvent{
Object: pod,
Meta: pod.GetObjectMeta(),
Expand All @@ -623,6 +668,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &appsv1.ReplicaSet{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand Down Expand Up @@ -665,6 +711,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &appsv1.ReplicaSet{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand All @@ -686,6 +733,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &metav1.ListOptions{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand All @@ -707,6 +755,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &controllertest.ErrorType{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand All @@ -727,6 +776,7 @@ var _ = Describe("Eventhandler", func() {
It("should do nothing.", func() {
instance := handler.EnqueueRequestForOwner{}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand All @@ -749,6 +799,7 @@ var _ = Describe("Eventhandler", func() {
OwnerType: &appsv1.ReplicaSet{},
}
instance.InjectScheme(scheme.Scheme)
instance.InjectMapper(mapper)
pod.OwnerReferences = []metav1.OwnerReference{
{
Name: "foo1-parent",
Expand Down
3 changes: 3 additions & 0 deletions pkg/manager/internal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ func (cm *controllerManager) SetFields(i interface{}) error {
if _, err := inject.DecoderInto(cm.admissionDecoder, i); err != nil {
return err
}
if _, err := inject.MapperInto(cm.mapper, i); err != nil {
return err
}
return nil
}

Expand Down
15 changes: 15 additions & 0 deletions pkg/runtime/inject/inject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package inject

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

// Mapper is used to inject the rest mapper to components that may need it
type Mapper interface {
InjectMapper(meta.RESTMapper) error
}

// MapperInto will set the rest mapper on i and return the result if it implements Mapper.
// Returns false if i does not implement Mapper.
func MapperInto(mapper meta.RESTMapper, i interface{}) (bool, error) {
if m, ok := i.(Mapper); ok {
return true, m.InjectMapper(mapper)
}
return false, nil
}

// Func injects dependencies into i.
type Func func(i interface{}) error

Expand Down

0 comments on commit 667e8d5

Please sign in to comment.