From 4cdda683fb211ea70f27ba682d9d3c5dabe645dd Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Sun, 26 Jul 2020 16:03:53 -0400 Subject: [PATCH 1/4] Add test for InstrumentedEnqueueRequestForObject --- handler/handler_suite_test.go | 3 +- handler/instrumented_enqueue_object_test.go | 149 ++++++++++++++++++++ 2 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 handler/instrumented_enqueue_object_test.go diff --git a/handler/handler_suite_test.go b/handler/handler_suite_test.go index 7a9c9f7..a623983 100644 --- a/handler/handler_suite_test.go +++ b/handler/handler_suite_test.go @@ -19,10 +19,9 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/envtest/printer" ) func TestEventhandler(t *testing.T) { RegisterFailHandler(Fail) - RunSpecsWithDefaultAndCustomReporters(t, "Handler Suite", []Reporter{printer.NewlineReporter{}}) + RunSpecs(t, "Handler Suite") } diff --git a/handler/instrumented_enqueue_object_test.go b/handler/instrumented_enqueue_object_test.go new file mode 100644 index 0000000..fd68e3f --- /dev/null +++ b/handler/instrumented_enqueue_object_test.go @@ -0,0 +1,149 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package handler + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllertest" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/metrics" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "k8s.io/client-go/util/workqueue" +) + +var _ = Describe("InstrumentedEnqueueRequestForObject", func() { + var q workqueue.RateLimitingInterface + var instance InstrumentedEnqueueRequestForObject + var pod *corev1.Pod + + BeforeEach(func() { + q = controllertest.Queue{Interface: workqueue.New()} + instance = InstrumentedEnqueueRequestForObject{} + pod = &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "biz", + Name: "biz", + }, + } + }) + Describe("Create", func() { + It("should enqueue a request & emit a metric on a CreateEvent", func() { + 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: pod.Namespace, + Name: pod.Name, + }, + })) + + gauges, err := metrics.Registry.Gather() + Expect(err).Should(BeNil()) + Expect(len(gauges)).To(Equal(1)) + for _, l := range gauges[0].Metric[0].Label { + if l.Name == ptrString("name") || l.Name == ptrString("namespace") { + Expect(l.Value).To(Equal("biz")) + } + } + }) + }) + + Describe("Delete", func() { + It("should enqueue a request & remove the metric on a DeleteEvent", func() { + evt := event.DeleteEvent{ + Object: pod, + Meta: pod.GetObjectMeta(), + } + + instance.Delete(evt, q) + Expect(q.Len()).To(Equal(1)) + i, _ := q.Get() + Expect(i).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: pod.Namespace, + Name: pod.Name, + }, + })) + + gauges, err := metrics.Registry.Gather() + Expect(err).Should(BeNil()) + Expect(len(gauges)).To(Equal(0)) + }) + }) + + Describe("Update", func() { + It("should enqueue a request in case of UpdateEvent", func() { + newpod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "baz", + Name: "baz", + }, + } + evt := event.UpdateEvent{ + ObjectOld: pod, + MetaOld: pod.GetObjectMeta(), + ObjectNew: newpod, + MetaNew: newpod.GetObjectMeta(), + } + + instance.Update(evt, q) + Expect(q.Len()).To(Equal(2)) + i, _ := q.Get() + Expect(i).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: pod.Namespace, + Name: pod.Name, + }, + })) + + gauges, err := metrics.Registry.Gather() + Expect(err).Should(BeNil()) + Expect(len(gauges)).To(Equal(1)) + for _, l := range gauges[0].Metric[0].Label { + if l.Name == ptrString("name") || l.Name == ptrString("namespace") { + Expect(l.Value).To(Equal("biz")) + } + } + }) + }) + + Describe("getResourceLabels", func() { + It("should fill out map with values from given objects", func() { + labelMap := getResourceLabels(pod.GetObjectMeta(), pod) + Expect(labelMap).ShouldNot(BeEmpty()) + Expect(len(labelMap)).To(Equal(5)) + Expect(labelMap["name"]).To(Equal(pod.GetObjectMeta().GetName())) + Expect(labelMap["namespace"]).To(Equal(pod.GetObjectMeta().GetNamespace())) + Expect(labelMap["group"]).To(Equal(pod.GetObjectKind().GroupVersionKind().Group)) + Expect(labelMap["version"]).To(Equal(pod.GetObjectKind().GroupVersionKind().Version)) + Expect(labelMap["kind"]).To(Equal(pod.GetObjectKind().GroupVersionKind().Kind)) + }) + }) +}) + +func ptrString(s string) *string { + return &s +} From f4f0ef3cf95b2c5adbb6ca6776877642cd5e2698 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 30 Jul 2020 00:15:02 -0400 Subject: [PATCH 2/4] Verify metric values --- go.mod | 1 + handler/instrumented_enqueue_object_test.go | 82 ++++++++++++++++----- 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index 47b9849..dfa60e3 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/onsi/ginkgo v1.12.1 github.com/onsi/gomega v1.10.1 github.com/prometheus/client_golang v1.0.0 + github.com/prometheus/client_model v0.2.0 github.com/stretchr/testify v1.5.1 k8s.io/api v0.18.4 k8s.io/apimachinery v0.18.4 diff --git a/handler/instrumented_enqueue_object_test.go b/handler/instrumented_enqueue_object_test.go index fd68e3f..6ce346f 100644 --- a/handler/instrumented_enqueue_object_test.go +++ b/handler/instrumented_enqueue_object_test.go @@ -15,8 +15,11 @@ package handler import ( + "time" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + dto "github.com/prometheus/client_model/go" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -37,9 +40,14 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { q = controllertest.Queue{Interface: workqueue.New()} instance = InstrumentedEnqueueRequestForObject{} pod = &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + APIVersion: "v1", + }, ObjectMeta: metav1.ObjectMeta{ - Namespace: "biz", - Name: "biz", + Namespace: "biznamespace", + Name: "bizname", + CreationTimestamp: metav1.Time{time.Now()}, }, } }) @@ -50,7 +58,10 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { Meta: pod.GetObjectMeta(), } + // test the create instance.Create(evt, q) + + // verify workqueue Expect(q.Len()).To(Equal(1)) i, _ := q.Get() Expect(i).To(Equal(reconcile.Request{ @@ -60,14 +71,11 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { }, })) + // verify metrics gauges, err := metrics.Registry.Gather() - Expect(err).Should(BeNil()) + Expect(err).NotTo(HaveOccurred()) Expect(len(gauges)).To(Equal(1)) - for _, l := range gauges[0].Metric[0].Label { - if l.Name == ptrString("name") || l.Name == ptrString("namespace") { - Expect(l.Value).To(Equal("biz")) - } - } + assertMetrics(gauges[0], 1, []*corev1.Pod{pod}) }) }) @@ -78,7 +86,10 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { Meta: pod.GetObjectMeta(), } + // test the delete instance.Delete(evt, q) + + // verify workqueue Expect(q.Len()).To(Equal(1)) i, _ := q.Get() Expect(i).To(Equal(reconcile.Request{ @@ -88,8 +99,9 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { }, })) + // verify metrics gauges, err := metrics.Registry.Gather() - Expect(err).Should(BeNil()) + Expect(err).NotTo(HaveOccurred()) Expect(len(gauges)).To(Equal(0)) }) }) @@ -98,8 +110,8 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { It("should enqueue a request in case of UpdateEvent", func() { newpod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "baz", - Name: "baz", + Namespace: "baznamespace", + Name: "bazname", }, } evt := event.UpdateEvent{ @@ -109,7 +121,10 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { MetaNew: newpod.GetObjectMeta(), } + // test the update instance.Update(evt, q) + + // verify workqueue Expect(q.Len()).To(Equal(2)) i, _ := q.Get() Expect(i).To(Equal(reconcile.Request{ @@ -118,15 +133,19 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { Name: pod.Name, }, })) + i, _ = q.Get() + Expect(i).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: newpod.Namespace, + Name: newpod.Name, + }, + })) + // verify metrics gauges, err := metrics.Registry.Gather() - Expect(err).Should(BeNil()) + Expect(err).NotTo(HaveOccurred()) Expect(len(gauges)).To(Equal(1)) - for _, l := range gauges[0].Metric[0].Label { - if l.Name == ptrString("name") || l.Name == ptrString("namespace") { - Expect(l.Value).To(Equal("biz")) - } - } + assertMetrics(gauges[0], 2, []*corev1.Pod{newpod, pod}) }) }) @@ -144,6 +163,31 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { }) }) -func ptrString(s string) *string { - return &s +func assertMetrics(gauge *dto.MetricFamily, count int, pods []*corev1.Pod) { + // need variables to compare the pointers + name := "name" + namespace := "namespace" + g := "group" + v := "version" + k := "kind" + + Expect(len(gauge.Metric)).To(Equal(count)) + for i := 0; i < count; i++ { + Expect(*gauge.Metric[i].Gauge.Value).To(Equal(float64(pods[i].GetObjectMeta().GetCreationTimestamp().UTC().Unix()))) + + for _, l := range gauge.Metric[i].Label { + switch l.Name { + case &name: + Expect(l.Value).To(Equal(pods[i].GetObjectMeta().GetName())) + case &namespace: + Expect(l.Value).To(Equal(pods[i].GetObjectMeta().GetNamespace())) + case &g: + Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Group)) + case &v: + Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Version)) + case &k: + Expect(l.Value).To(Equal(pods[i].GetObjectKind().GroupVersionKind().Kind)) + } + } + } } From 5f3326e1ebfceb02270b47c14e5dc0aec99691f1 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 30 Jul 2020 00:31:30 -0400 Subject: [PATCH 3/4] Add contexts for the delete case --- handler/instrumented_enqueue_object_test.go | 83 +++++++++++++++------ 1 file changed, 60 insertions(+), 23 deletions(-) diff --git a/handler/instrumented_enqueue_object_test.go b/handler/instrumented_enqueue_object_test.go index 6ce346f..1351c95 100644 --- a/handler/instrumented_enqueue_object_test.go +++ b/handler/instrumented_enqueue_object_test.go @@ -80,30 +80,67 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { }) Describe("Delete", func() { - It("should enqueue a request & remove the metric on a DeleteEvent", func() { - evt := event.DeleteEvent{ - Object: pod, - Meta: pod.GetObjectMeta(), - } - - // test the delete - instance.Delete(evt, q) - - // verify workqueue - Expect(q.Len()).To(Equal(1)) - i, _ := q.Get() - Expect(i).To(Equal(reconcile.Request{ - NamespacedName: types.NamespacedName{ - Namespace: pod.Namespace, - Name: pod.Name, - }, - })) - - // verify metrics - gauges, err := metrics.Registry.Gather() - Expect(err).NotTo(HaveOccurred()) - Expect(len(gauges)).To(Equal(0)) + Context("when a gauge already exists", func() { + BeforeEach(func() { + evt := event.CreateEvent{ + Object: pod, + Meta: pod.GetObjectMeta(), + } + instance.Create(evt, q) + Expect(q.Len()).To(Equal(1)) + }) + It("should enqueue a request & remove the metric on a DeleteEvent", func() { + evt := event.DeleteEvent{ + Object: pod, + Meta: pod.GetObjectMeta(), + } + + // test the delete + instance.Delete(evt, q) + + // verify workqueue + Expect(q.Len()).To(Equal(1)) + i, _ := q.Get() + Expect(i).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: pod.Namespace, + Name: pod.Name, + }, + })) + + // verify metrics + gauges, err := metrics.Registry.Gather() + Expect(err).NotTo(HaveOccurred()) + Expect(len(gauges)).To(Equal(0)) + }) }) + Context("when a gauge does not exist", func() { + It("should enqueue a request & there should be no new metric on a DeleteEvent", func() { + evt := event.DeleteEvent{ + Object: pod, + Meta: pod.GetObjectMeta(), + } + + // test the delete + instance.Delete(evt, q) + + // verify workqueue + Expect(q.Len()).To(Equal(1)) + i, _ := q.Get() + Expect(i).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: pod.Namespace, + Name: pod.Name, + }, + })) + + // verify metrics + gauges, err := metrics.Registry.Gather() + Expect(err).NotTo(HaveOccurred()) + Expect(len(gauges)).To(Equal(0)) + }) + }) + }) Describe("Update", func() { From 20e17577f714a4f4bc60309908dd31df22502d0b Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 30 Jul 2020 00:46:13 -0400 Subject: [PATCH 4/4] Fix composite literal uses unkeyed fields error --- handler/instrumented_enqueue_object_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/handler/instrumented_enqueue_object_test.go b/handler/instrumented_enqueue_object_test.go index 1351c95..fda73c2 100644 --- a/handler/instrumented_enqueue_object_test.go +++ b/handler/instrumented_enqueue_object_test.go @@ -15,8 +15,6 @@ package handler import ( - "time" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" dto "github.com/prometheus/client_model/go" @@ -47,7 +45,7 @@ var _ = Describe("InstrumentedEnqueueRequestForObject", func() { ObjectMeta: metav1.ObjectMeta{ Namespace: "biznamespace", Name: "bizname", - CreationTimestamp: metav1.Time{time.Now()}, + CreationTimestamp: metav1.Now(), }, } })