From ca6f78425c81cb28f606a12975aad3d47bece140 Mon Sep 17 00:00:00 2001 From: Bernard Van De Walle Date: Thu, 23 Jul 2020 12:55:18 -0700 Subject: [PATCH] Adding Zipkin collector to the E2E opentracing test as it is required to load at least one tracer to enable opentracing Work on PR comments Add tests for template builder Signed-off-by: Bernard Van De Walle --- .../ingress/controller/template/template.go | 7 +++++ .../controller/template/template_test.go | 15 +++++++++++ rootfs/etc/nginx/template/nginx.tmpl | 10 ------- test/e2e/settings/opentracing.go | 26 ++++++++++++------- 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 3666defbf..d9dc6aec7 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -970,6 +970,13 @@ func buildOpentracing(c interface{}, s interface{}) string { buf.WriteString("\r\n") + if cfg.OpentracingOperationName != "" { + buf.WriteString(fmt.Sprintf("opentracing_operation_name \"%s\";\n", cfg.OpentracingOperationName)) + } + if cfg.OpentracingLocationOperationName != "" { + buf.WriteString(fmt.Sprintf("opentracing_location_operation_name \"%s\";\n", cfg.OpentracingLocationOperationName)) + } + return buf.String() } diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 2f2dd670a..488636653 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -1193,6 +1193,21 @@ func TestBuildOpenTracing(t *testing.T) { t.Errorf("Expected '%v' but returned '%v'", expected, actual) } + cfgOpenTracing := config.Configuration{ + EnableOpentracing: true, + DatadogCollectorHost: "datadog-host.com", + OpentracingOperationName: "my-operation-name", + OpentracingLocationOperationName: "my-location-operation-name", + } + expected = "opentracing_load_tracer /usr/local/lib64/libdd_opentracing.so /etc/nginx/opentracing.json;\r\n" + expected += "opentracing_operation_name \"my-operation-name\";\n" + expected += "opentracing_location_operation_name \"my-location-operation-name\";\n" + actual = buildOpentracing(cfgOpenTracing, []*ingress.Server{}) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + } func TestEnforceRegexModifier(t *testing.T) { diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index bf7bc1029..46c7fdbfa 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -276,16 +276,6 @@ http { limit_req_status {{ $cfg.LimitReqStatusCode }}; limit_conn_status {{ $cfg.LimitConnStatusCode }}; - {{ if $cfg.EnableOpentracing }} - opentracing on; - {{ if not (empty $cfg.OpentracingOperationName) }} - opentracing_operation_name "{{ $cfg.OpentracingOperationName }}"; - {{ end }} - {{ if not (empty $cfg.OpentracingLocationOperationName) }} - opentracing_location_operation_name "{{ $cfg.OpentracingLocationOperationName }}"; - {{ end }} - {{ end }} - {{ buildOpentracing $cfg $servers }} include /etc/nginx/mime.types; diff --git a/test/e2e/settings/opentracing.go b/test/e2e/settings/opentracing.go index 10a3c1a55..7367ccdbb 100644 --- a/test/e2e/settings/opentracing.go +++ b/test/e2e/settings/opentracing.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Kubernetes Authors. +Copyright 2020 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -19,7 +19,7 @@ package settings import ( "strings" - . "github.com/onsi/ginkgo" + "github.com/onsi/ginkgo" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -27,17 +27,18 @@ import ( var _ = framework.IngressNginxDescribe("Configure OpenTracing", func() { f := framework.NewDefaultFramework("enable-opentracing") enableOpentracing := "enable-opentracing" + zipkinCollectorHost := "zipkin-collector-host" opentracingOperationName := "opentracing-operation-name" opentracingLocationOperationName := "opentracing-location-operation-name" - BeforeEach(func() { + ginkgo.BeforeEach(func() { f.NewEchoDeployment() }) - AfterEach(func() { + ginkgo.AfterEach(func() { }) - It("should not exists opentracing directive", func() { + ginkgo.It("should not exists opentracing directive", func() { f.UpdateNginxConfigMapData(enableOpentracing, "false") f.EnsureIngress(framework.NewSingleIngress(enableOpentracing, "/", enableOpentracing, f.Namespace, "http-svc", 80, nil)) @@ -48,8 +49,9 @@ var _ = framework.IngressNginxDescribe("Configure OpenTracing", func() { }) }) - It("should exists opentracing directive when is enabled", func() { + ginkgo.It("should exists opentracing directive when is enabled", func() { f.UpdateNginxConfigMapData(enableOpentracing, "true") + f.UpdateNginxConfigMapData(zipkinCollectorHost, "127.0.0.1") f.EnsureIngress(framework.NewSingleIngress(enableOpentracing, "/", enableOpentracing, f.Namespace, "http-svc", 80, nil)) @@ -59,8 +61,9 @@ var _ = framework.IngressNginxDescribe("Configure OpenTracing", func() { }) }) - It("should not exists opentracing_operation_name directive when is empty", func() { + ginkgo.It("should not exists opentracing_operation_name directive when is empty", func() { f.UpdateNginxConfigMapData(enableOpentracing, "true") + f.UpdateNginxConfigMapData(zipkinCollectorHost, "127.0.0.1") f.UpdateNginxConfigMapData(opentracingOperationName, "") f.EnsureIngress(framework.NewSingleIngress(enableOpentracing, "/", enableOpentracing, f.Namespace, "http-svc", 80, nil)) @@ -71,8 +74,9 @@ var _ = framework.IngressNginxDescribe("Configure OpenTracing", func() { }) }) - It("should exists opentracing_operation_name directive when is configured", func() { + ginkgo.It("should exists opentracing_operation_name directive when is configured", func() { f.UpdateNginxConfigMapData(enableOpentracing, "true") + f.UpdateNginxConfigMapData(zipkinCollectorHost, "127.0.0.1") f.UpdateNginxConfigMapData(opentracingOperationName, "HTTP $request_method $uri") f.EnsureIngress(framework.NewSingleIngress(enableOpentracing, "/", enableOpentracing, f.Namespace, "http-svc", 80, nil)) @@ -83,8 +87,9 @@ var _ = framework.IngressNginxDescribe("Configure OpenTracing", func() { }) }) - It("should not exists opentracing_operation_name directive when is empty", func() { + ginkgo.It("should not exists opentracing_location_operation_name directive when is empty", func() { f.UpdateNginxConfigMapData(enableOpentracing, "true") + f.UpdateNginxConfigMapData(zipkinCollectorHost, "127.0.0.1") f.UpdateNginxConfigMapData(opentracingLocationOperationName, "") f.EnsureIngress(framework.NewSingleIngress(enableOpentracing, "/", enableOpentracing, f.Namespace, "http-svc", 80, nil)) @@ -95,8 +100,9 @@ var _ = framework.IngressNginxDescribe("Configure OpenTracing", func() { }) }) - It("should exists opentracing_operation_name directive when is configured", func() { + ginkgo.It("should exists opentracing_location_operation_name directive when is configured", func() { f.UpdateNginxConfigMapData(enableOpentracing, "true") + f.UpdateNginxConfigMapData(zipkinCollectorHost, "127.0.0.1") f.UpdateNginxConfigMapData(opentracingLocationOperationName, "HTTP $request_method $uri") f.EnsureIngress(framework.NewSingleIngress(enableOpentracing, "/", enableOpentracing, f.Namespace, "http-svc", 80, nil))