From 0edc5c2afb51816e2834bec25dc4838ef5fe67c5 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Thu, 27 Jul 2023 17:11:44 -0400 Subject: [PATCH] :sparkles: add logging to Enqueue_owner event handler This PR adds logging to Enqueue owner event handler, that is helpful in logging the GVK of the owner that caused reconcilation. Signed-off-by: Varsha Prasad Narsing --- pkg/handler/enqueue_owner.go | 50 ++++++++++++++++++++++++++ pkg/handler/eventhandler_suite_test.go | 4 ++- pkg/handler/eventhandler_test.go | 36 +++++++++++++++---- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/pkg/handler/enqueue_owner.go b/pkg/handler/enqueue_owner.go index 02e7d756f8..d0ba418ab7 100644 --- a/pkg/handler/enqueue_owner.go +++ b/pkg/handler/enqueue_owner.go @@ -84,6 +84,7 @@ type enqueueRequestForOwner struct { // Create implements EventHandler. func (e *enqueueRequestForOwner) Create(ctx context.Context, evt event.CreateEvent, q workqueue.RateLimitingInterface) { + e.logEvent("Create", evt.Object, nil) reqs := map[reconcile.Request]empty{} e.getOwnerReconcileRequest(evt.Object, reqs) for req := range reqs { @@ -93,6 +94,7 @@ func (e *enqueueRequestForOwner) Create(ctx context.Context, evt event.CreateEve // Update implements EventHandler. func (e *enqueueRequestForOwner) Update(ctx context.Context, evt event.UpdateEvent, q workqueue.RateLimitingInterface) { + e.logEvent("Update", evt.ObjectOld, evt.ObjectNew) reqs := map[reconcile.Request]empty{} e.getOwnerReconcileRequest(evt.ObjectOld, reqs) e.getOwnerReconcileRequest(evt.ObjectNew, reqs) @@ -103,6 +105,7 @@ func (e *enqueueRequestForOwner) Update(ctx context.Context, evt event.UpdateEve // Delete implements EventHandler. func (e *enqueueRequestForOwner) Delete(ctx context.Context, evt event.DeleteEvent, q workqueue.RateLimitingInterface) { + e.logEvent("Delete", evt.Object, nil) reqs := map[reconcile.Request]empty{} e.getOwnerReconcileRequest(evt.Object, reqs) for req := range reqs { @@ -112,6 +115,7 @@ func (e *enqueueRequestForOwner) Delete(ctx context.Context, evt event.DeleteEve // Generic implements EventHandler. func (e *enqueueRequestForOwner) Generic(ctx context.Context, evt event.GenericEvent, q workqueue.RateLimitingInterface) { + e.logEvent("Generic", evt.Object, nil) reqs := map[reconcile.Request]empty{} e.getOwnerReconcileRequest(evt.Object, reqs) for req := range reqs { @@ -197,3 +201,49 @@ func (e *enqueueRequestForOwner) getOwnersReferences(object metav1.Object) []met // No Controller OwnerReference found return nil } + +// logEvent logs an event with the version, kind and name of the object and its owner. +func (e *enqueueRequestForOwner) logEvent(eventType string, object, newObject client.Object) { + var ownerReference *metav1.OwnerReference + if e.ownerType != nil && object != nil { + ownerReference = extractTypedOwnerReference(e.ownerType.GetObjectKind().GroupVersionKind(), object.GetOwnerReferences()) + if ownerReference == nil && newObject != nil { + ownerReference = extractTypedOwnerReference(e.ownerType.GetObjectKind().GroupVersionKind(), newObject.GetOwnerReferences()) + } + } + + // If no ownerReference was found then it's probably not an event we care about + if ownerReference != nil { + kvs := []interface{}{ + "Event type", eventType, + "GroupVersionKind", object.GetObjectKind().GroupVersionKind().String(), + "Name", object.GetName(), + } + if objectNs := object.GetNamespace(); objectNs != "" { + kvs = append(kvs, "Namespace", objectNs) + } + kvs = append(kvs, + "Owner APIVersion", ownerReference.APIVersion, + "Owner Kind", ownerReference.Kind, + "Owner Name", ownerReference.Name, + ) + + log.Info("OwnerReference handler event", kvs...) + } +} + +func extractTypedOwnerReference(ownerGVK schema.GroupVersionKind, ownerReferences []metav1.OwnerReference) *metav1.OwnerReference { + for _, ownerRef := range ownerReferences { + refGV, err := schema.ParseGroupVersion(ownerRef.APIVersion) + if err != nil { + log.Error(err, "Could not parse OwnerReference APIVersion", + "api version", ownerRef.APIVersion) + } + + if ownerGVK.Group == refGV.Group && + ownerGVK.Kind == ownerRef.Kind { + return &ownerRef + } + } + return nil +} diff --git a/pkg/handler/eventhandler_suite_test.go b/pkg/handler/eventhandler_suite_test.go index 3f6b17f337..863077083d 100644 --- a/pkg/handler/eventhandler_suite_test.go +++ b/pkg/handler/eventhandler_suite_test.go @@ -17,6 +17,7 @@ limitations under the License. package handler_test import ( + "bytes" "testing" . "github.com/onsi/ginkgo/v2" @@ -34,9 +35,10 @@ func TestEventhandler(t *testing.T) { var testenv *envtest.Environment var cfg *rest.Config +var logBuffer bytes.Buffer var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New(zap.WriteTo(&logBuffer), zap.UseDevMode(true))) testenv = &envtest.Environment{} var err error diff --git a/pkg/handler/eventhandler_test.go b/pkg/handler/eventhandler_test.go index 6ae87b0988..551e18f2b5 100644 --- a/pkg/handler/eventhandler_test.go +++ b/pkg/handler/eventhandler_test.go @@ -26,6 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -50,6 +51,7 @@ var _ = Describe("Eventhandler", func() { pod = &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "baz"}, } + pod.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}) Expect(cfg).NotTo(BeNil()) httpClient, err := rest.HTTPClientFor(cfg) @@ -307,8 +309,15 @@ var _ = Describe("Eventhandler", func() { }) Describe("EnqueueRequestForOwner", func() { + var repl *appsv1.ReplicaSet + + BeforeEach(func() { + repl = &appsv1.ReplicaSet{} + repl.SetGroupVersionKind(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "ReplicaSet"}) + }) + It("should enqueue a Request with the Owner of the object in the CreateEvent.", func() { - instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, &appsv1.ReplicaSet{}) + instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, repl) pod.OwnerReferences = []metav1.OwnerReference{ { @@ -320,17 +329,20 @@ var _ = Describe("Eventhandler", func() { evt := event.CreateEvent{ Object: pod, } + logBuffer.Reset() instance.Create(ctx, evt, q) Expect(q.Len()).To(Equal(1)) i, _ := q.Get() Expect(i).To(Equal(reconcile.Request{ NamespacedName: types.NamespacedName{Namespace: pod.GetNamespace(), Name: "foo-parent"}})) + Expect(logBuffer.String()).To(MatchRegexp( + `controller-runtime.eventhandler.enqueueRequestForOwner.*OwnerReference.*handler.*event.*Create.*Owner.APIVersion.*Owner.Kind.*Owner.Name.*`, + )) }) It("should enqueue a Request with the Owner of the object in the DeleteEvent.", func() { - instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, &appsv1.ReplicaSet{}) - + instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, repl) pod.OwnerReferences = []metav1.OwnerReference{ { Name: "foo-parent", @@ -341,12 +353,16 @@ var _ = Describe("Eventhandler", func() { evt := event.DeleteEvent{ Object: pod, } + logBuffer.Reset() instance.Delete(ctx, evt, q) Expect(q.Len()).To(Equal(1)) i, _ := q.Get() Expect(i).To(Equal(reconcile.Request{ NamespacedName: types.NamespacedName{Namespace: pod.GetNamespace(), Name: "foo-parent"}})) + Expect(logBuffer.String()).To(MatchRegexp( + `controller-runtime.eventhandler.enqueueRequestForOwner.*OwnerReference.*handler.*event.*Delete.*Owner.APIVersion.*Owner.Kind.*Owner.Name.*`, + )) }) It("should enqueue a Request with the Owners of both objects in the UpdateEvent.", func() { @@ -354,7 +370,7 @@ var _ = Describe("Eventhandler", func() { newPod.Name = pod.Name + "2" newPod.Namespace = pod.Namespace + "2" - instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, &appsv1.ReplicaSet{}) + instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, repl) pod.OwnerReferences = []metav1.OwnerReference{ { @@ -370,6 +386,7 @@ var _ = Describe("Eventhandler", func() { APIVersion: "apps/v1", }, } + logBuffer.Reset() evt := event.UpdateEvent{ ObjectOld: pod, ObjectNew: newPod, @@ -385,6 +402,9 @@ var _ = Describe("Eventhandler", func() { reconcile.Request{ NamespacedName: types.NamespacedName{Namespace: newPod.GetNamespace(), Name: "foo2-parent"}}, )) + Expect(logBuffer.String()).To(MatchRegexp( + `controller-runtime.eventhandler.enqueueRequestForOwner.*OwnerReference.*handler.*event.*Update.*Owner.APIVersion.*Owner.Kind.*Owner.Name.*`, + )) }) It("should enqueue a Request with the one duplicate Owner of both objects in the UpdateEvent.", func() { @@ -420,7 +440,7 @@ var _ = Describe("Eventhandler", func() { }) It("should enqueue a Request with the Owner of the object in the GenericEvent.", func() { - instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, &appsv1.ReplicaSet{}) + instance := handler.EnqueueRequestForOwner(scheme.Scheme, mapper, repl) pod.OwnerReferences = []metav1.OwnerReference{ { Name: "foo-parent", @@ -431,12 +451,16 @@ var _ = Describe("Eventhandler", func() { evt := event.GenericEvent{ Object: pod, } + logBuffer.Reset() instance.Generic(ctx, evt, q) Expect(q.Len()).To(Equal(1)) i, _ := q.Get() Expect(i).To(Equal(reconcile.Request{ NamespacedName: types.NamespacedName{Namespace: pod.GetNamespace(), Name: "foo-parent"}})) + Expect(logBuffer.String()).To(MatchRegexp( + `controller-runtime.eventhandler.enqueueRequestForOwner.*OwnerReference.*handler.*event.*Generic.*Owner.APIVersion.*Owner.Kind.*Owner.Name.*`, + )) }) It("should not enqueue a Request if there are no owners matching Group and Kind.", func() { @@ -467,7 +491,7 @@ var _ = Describe("Eventhandler", func() { { Name: "foo-parent", Kind: "HorizontalPodAutoscaler", - APIVersion: "autoscaling/v2beta1", + APIVersion: "autoscaling/v2", }, } evt := event.CreateEvent{