From 84fa5ad468c8558654c4e4080571ea329d6381c1 Mon Sep 17 00:00:00 2001 From: Jon Carl Date: Fri, 16 Aug 2024 16:02:02 -0600 Subject: [PATCH] add tests Signed-off-by: Jon Carl --- .../ingress/metric/collectors/socket_test.go | 75 ++++++++++++++++--- pkg/flags/flags_test.go | 26 +++++++ test/e2e/metrics/metrics.go | 47 ++++++++++++ 3 files changed, 139 insertions(+), 9 deletions(-) diff --git a/internal/ingress/metric/collectors/socket_test.go b/internal/ingress/metric/collectors/socket_test.go index a8261a9ca..3a1f29f35 100644 --- a/internal/ingress/metric/collectors/socket_test.go +++ b/internal/ingress/metric/collectors/socket_test.go @@ -87,14 +87,15 @@ func TestCollector(t *testing.T) { maxBuckets := uint32(100) cases := []struct { - name string - data []string - metrics []string - useStatusClasses bool - excludeMetrics []string - wantBefore string - removeIngresses []string - wantAfter string + name string + data []string + metrics []string + metricsPerUndefinedHost bool + useStatusClasses bool + excludeMetrics []string + wantBefore string + removeIngresses []string + wantAfter string }{ { name: "invalid metric object should not increase prometheus metrics", @@ -591,13 +592,69 @@ func TestCollector(t *testing.T) { nginx_ingress_controller_response_duration_seconds_count{canary="",controller_class="ingress",controller_namespace="default",controller_pod="pod",host="testshop.com",ingress="web-yml",method="GET",namespace="test-app-production",path="/admin",service="test-app",status="2xx"} 1 `, }, + { + name: "metrics with a host should not be dropped when the host is not in the hosts slice but metricsPerUndefinedHost is true", + data: []string{`[{ + "host":"wildcard.testshop.com", + "status":"200", + "bytesSent":150.0, + "method":"GET", + "path":"/admin", + "requestLength":300.0, + "requestTime":60.0, + "upstreamLatency":1.0, + "upstreamHeaderTime":5.0, + "upstreamName":"test-upstream", + "upstreamIP":"1.1.1.1:8080", + "upstreamResponseTime":200, + "upstreamStatus":"220", + "namespace":"test-app-production", + "ingress":"web-yml", + "service":"test-app", + "canary":"" + }]`}, + excludeMetrics: []string{"response_duration_seconds2", "test.*", "nginx_ingress_.*", "response_duration_secon"}, + metrics: []string{"nginx_ingress_controller_requests"}, + metricsPerUndefinedHost: true, + useStatusClasses: true, + wantBefore: ` + # HELP nginx_ingress_controller_requests The total number of client requests + # TYPE nginx_ingress_controller_requests counter + nginx_ingress_controller_requests{canary="",controller_class="ingress",controller_namespace="default",controller_pod="pod",host="wildcard.testshop.com",ingress="web-yml",method="GET",namespace="test-app-production",path="/admin",service="test-app",status="2xx"} 1 + `, + }, + { + name: "metrics with a host should be dropped when the host is not in the hosts slice", + data: []string{`[{ + "host":"wildcard.testshop.com", + "status":"200", + "bytesSent":150.0, + "method":"GET", + "path":"/admin", + "requestLength":300.0, + "requestTime":60.0, + "upstreamLatency":1.0, + "upstreamHeaderTime":5.0, + "upstreamName":"test-upstream", + "upstreamIP":"1.1.1.1:8080", + "upstreamResponseTime":200, + "upstreamStatus":"220", + "namespace":"test-app-production", + "ingress":"web-yml", + "service":"test-app", + "canary":"" + }]`}, + excludeMetrics: []string{"response_duration_seconds2", "test.*", "nginx_ingress_.*", "response_duration_secon"}, + metrics: []string{"nginx_ingress_controller_requests"}, + useStatusClasses: true, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { registry := prometheus.NewPedanticRegistry() - sc, err := NewSocketCollector("pod", "default", "ingress", true, c.useStatusClasses, buckets, bucketFactor, maxBuckets, c.excludeMetrics) + sc, err := NewSocketCollector("pod", "default", "ingress", true, c.metricsPerUndefinedHost, c.useStatusClasses, buckets, bucketFactor, maxBuckets, c.excludeMetrics) if err != nil { t.Errorf("%v: unexpected error creating new SocketCollector: %v", c.name, err) } diff --git a/pkg/flags/flags_test.go b/pkg/flags/flags_test.go index e51d5fa6c..fdf153021 100644 --- a/pkg/flags/flags_test.go +++ b/pkg/flags/flags_test.go @@ -212,3 +212,29 @@ func TestLeaderElectionTTLParseValueInHours(t *testing.T) { t.Fatalf("Expected --election-ttl and conf.ElectionTTL as 1h, but found: %v", conf.ElectionTTL) } } + +func TestMetricsPerUndefinedHost(t *testing.T) { + ResetForTesting(func() { t.Fatal("Parsing failed") }) + + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + os.Args = []string{"cmd", "--metrics-per-undefined-host=true"} + + _, _, err := ParseFlags() + if err != nil { + t.Fatalf("Expected no error but got: %s", err) + } +} + +func TestMetricsPerUndefinedHostWithMetricsPerHostFalse(t *testing.T) { + ResetForTesting(func() { t.Fatal("Parsing failed") }) + + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + os.Args = []string{"cmd", "--metrics-per-host=false", "--metrics-per-undefined-host=true"} + + _, _, err := ParseFlags() + if err == nil { + t.Fatalf("Expected an error parsing flags but none returned") + } +} diff --git a/test/e2e/metrics/metrics.go b/test/e2e/metrics/metrics.go index 907b53732..bec09bb37 100644 --- a/test/e2e/metrics/metrics.go +++ b/test/e2e/metrics/metrics.go @@ -36,6 +36,7 @@ const waitForMetrics = 2 * time.Second var _ = framework.IngressNginxDescribe("[metrics] exported prometheus metrics", func() { f := framework.NewDefaultFramework("metrics") host := "foo.com" + wildcardHost := "wildcard." + host ginkgo.BeforeEach(func() { f.NewEchoDeployment() @@ -91,4 +92,50 @@ var _ = framework.IngressNginxDescribe("[metrics] exported prometheus metrics", assert.Nil(ginkgo.GinkgoT(), err) assert.NotNil(ginkgo.GinkgoT(), mf) }) + ginkgo.It("request metrics per undefined host are present when flag is set", func() { + err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error { + args := deployment.Spec.Template.Spec.Containers[0].Args + args = append(args, "--metrics-per-undefined-host=true") + deployment.Spec.Template.Spec.Containers[0].Args = args + _, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{}) + return err + }) + assert.Nil(ginkgo.GinkgoT(), err, "updating deployment") + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", wildcardHost). + Expect(). + Status(http.StatusNotFound) + time.Sleep(waitForMetrics) + + ip := f.GetNginxPodIP() + reqMetrics, err := f.GetMetric("nginx_ingress_controller_requests", ip) + assert.Nil(ginkgo.GinkgoT(), err) + assert.NotNil(ginkgo.GinkgoT(), reqMetrics.Metric) + assert.Len(ginkgo.GinkgoT(), reqMetrics.Metric, 1) + + containedLabel := false + for _, label := range reqMetrics.Metric[0].Label { + if *label.Name == "host" && *label.Value == wildcardHost { + containedLabel = true + break + } + } + + assert.Truef(ginkgo.GinkgoT(), containedLabel, "expected reqMetrics to contain label with \"name\"=\"host\" \"value\"=%q, but it did not: %s", wildcardHost, reqMetrics.String()) + }) + ginkgo.It("request metrics per undefined host are not present when flag is not set", func() { + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", wildcardHost). + Expect(). + Status(http.StatusNotFound) + time.Sleep(waitForMetrics) + + ip := f.GetNginxPodIP() + reqMetrics, err := f.GetMetric("nginx_ingress_controller_requests", ip) + assert.EqualError(ginkgo.GinkgoT(), err, "there is no metric with name nginx_ingress_controller_requests") + assert.Nil(ginkgo.GinkgoT(), reqMetrics) + }) })