From 818e6535ad4fc9ecd9b566842ce9352b79b7be14 Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Wed, 13 Nov 2024 14:48:38 -0500 Subject: [PATCH 1/8] Remove hard-coded gateway namespace We need the gateway's namespace to be configured dynamically in the test using the "-n" flag in suite.go. To do this, we move that to a separate file and apply it separately using that flag. All the other resources are still preserved in their original namespaces. --- test/kubernetes/e2e/features/tracing/suite.go | 11 ++++- .../features/tracing/testdata/gateway.yaml | 42 ++++++++++++++++++ .../features/tracing/testdata/tracing.yaml | 44 ------------------- test/kubernetes/e2e/features/tracing/types.go | 1 + 4 files changed, 52 insertions(+), 46 deletions(-) create mode 100644 test/kubernetes/e2e/features/tracing/testdata/gateway.yaml diff --git a/test/kubernetes/e2e/features/tracing/suite.go b/test/kubernetes/e2e/features/tracing/suite.go index 5a9a6235345..a7bf41a4c96 100644 --- a/test/kubernetes/e2e/features/tracing/suite.go +++ b/test/kubernetes/e2e/features/tracing/suite.go @@ -100,8 +100,11 @@ func (s *testingSuite) BeforeTest(string, string) { // HttpListener Error: ProcessingError. Reason: *v1.Upstream { // default.opentelemetry-collector } not found" s.Assert().Eventually(func() bool { - err := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, tracingConfigManifest) - return err == nil + // TODO clean this up + err1 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, tracingConfigManifest) + err2 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gatewayConfigManifest, + "-n", s.testInstallation.Metadata.InstallNamespace) + return err1 == nil && err2 == nil }, time.Second*30, time.Second*5, "can apply gloo tracing config") } @@ -112,6 +115,10 @@ func (s *testingSuite) AfterTest(string, string) { err = s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, tracingConfigManifest) s.Assertions.NoError(err, "can delete gloo tracing config") + + err = s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gatewayConfigManifest, + "-n", s.testInstallation.Metadata.InstallNamespace) + s.Assertions.NoError(err, "can delete gloo tracing config") } func (s *testingSuite) TestSpanNameTransformationsWithoutRouteDecorator() { diff --git a/test/kubernetes/e2e/features/tracing/testdata/gateway.yaml b/test/kubernetes/e2e/features/tracing/testdata/gateway.yaml new file mode 100644 index 00000000000..7a04283601e --- /dev/null +++ b/test/kubernetes/e2e/features/tracing/testdata/gateway.yaml @@ -0,0 +1,42 @@ +# Avoid using the default gateway because we don't want to destroy it when this +# test is over - that will break other tests that depend on the default gateway +# existing. +apiVersion: gateway.solo.io/v1 +kind: Gateway +metadata: + labels: + app: gloo + app.kubernetes.io/name: gateway-proxy-tracing + name: gateway-proxy-tracing +spec: + bindAddress: '::' + bindPort: 18080 + proxyNames: + - gateway-proxy + httpGateway: + virtualServiceSelector: + gateway-type: tracing + options: + httpConnectionManagerSettings: + tracing: + openTelemetryConfig: + collectorUpstreamRef: + name: opentelemetry-collector + namespace: default +--- +apiVersion: v1 +kind: Service +metadata: + name: gateway-proxy-tracing + labels: + app.kubernetes.io/name: gateway-proxy-tracing-service +spec: + ports: + - name: gateway-proxy-tracing + port: 18080 + protocol: TCP + targetPort: 18080 + selector: + gateway-proxy: live + gateway-proxy-id: gateway-proxy + diff --git a/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml b/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml index a2dc5f904c5..644400683b8 100644 --- a/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml +++ b/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml @@ -71,47 +71,3 @@ spec: autoHostRewrite: true tracing: routeDescriptor: THISISAROUTEDESCRIPTOR ---- -# Avoid using the default gateway because we don't want to destroy it when this -# test is over - that will break other tests that depend on the default gateway -# existing. -apiVersion: gateway.solo.io/v1 -kind: Gateway -metadata: - labels: - app: gloo - app.kubernetes.io/name: gateway-proxy-tracing - name: gateway-proxy-tracing - namespace: gloo-gateway-edge-test -spec: - bindAddress: '::' - bindPort: 18080 - proxyNames: - - gateway-proxy - httpGateway: - virtualServiceSelector: - gateway-type: tracing - options: - httpConnectionManagerSettings: - tracing: - openTelemetryConfig: - collectorUpstreamRef: - name: opentelemetry-collector - namespace: default ---- -apiVersion: v1 -kind: Service -metadata: - name: gateway-proxy-tracing - namespace: gloo-gateway-edge-test - labels: - app.kubernetes.io/name: gateway-proxy-tracing-service -spec: - ports: - - name: gateway-proxy-tracing - port: 18080 - protocol: TCP - targetPort: 18080 - selector: - gateway-proxy: live - gateway-proxy-id: gateway-proxy diff --git a/test/kubernetes/e2e/features/tracing/types.go b/test/kubernetes/e2e/features/tracing/types.go index 4c4883e0201..2ececcdea9f 100644 --- a/test/kubernetes/e2e/features/tracing/types.go +++ b/test/kubernetes/e2e/features/tracing/types.go @@ -19,6 +19,7 @@ const ( var ( setupOtelcolManifest = filepath.Join(util.MustGetThisDir(), "testdata", "setup-otelcol.yaml") tracingConfigManifest = filepath.Join(util.MustGetThisDir(), "testdata", "tracing.yaml") + gatewayConfigManifest = filepath.Join(util.MustGetThisDir(), "testdata", "gateway.yaml") otelcolPod = &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "otel-collector", Namespace: "default"}, From 700ce22544dc33655f168e81e7a48fef0bdf86dc Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Wed, 13 Nov 2024 15:06:49 -0500 Subject: [PATCH 2/8] Code formatting --- test/kubernetes/e2e/features/tracing/suite.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/kubernetes/e2e/features/tracing/suite.go b/test/kubernetes/e2e/features/tracing/suite.go index a7bf41a4c96..1e8dedd8bc3 100644 --- a/test/kubernetes/e2e/features/tracing/suite.go +++ b/test/kubernetes/e2e/features/tracing/suite.go @@ -117,7 +117,7 @@ func (s *testingSuite) AfterTest(string, string) { s.Assertions.NoError(err, "can delete gloo tracing config") err = s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gatewayConfigManifest, - "-n", s.testInstallation.Metadata.InstallNamespace) + "-n", s.testInstallation.Metadata.InstallNamespace) s.Assertions.NoError(err, "can delete gloo tracing config") } @@ -132,6 +132,7 @@ func (s *testingSuite) TestSpanNameTransformationsWithoutRouteDecorator() { curl.WithHostHeader(testHostname), curl.WithPort(gatewayProxyPort), curl.WithPath(pathWithoutRouteDescriptor), + curl.Silent(), }, &matchers.HttpResponse{ StatusCode: http.StatusOK, @@ -157,6 +158,7 @@ func (s *testingSuite) TestSpanNameTransformationsWithRouteDecorator() { curl.WithHostHeader("example.com"), curl.WithPort(gatewayProxyPort), curl.WithPath(pathWithRouteDescriptor), + curl.Silent(), }, &matchers.HttpResponse{ StatusCode: http.StatusOK, From fabac9195cfb534c0a143c75a096b0d4b978e739 Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Wed, 13 Nov 2024 17:06:29 -0500 Subject: [PATCH 3/8] Add changelog --- .../remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml diff --git a/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml b/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml new file mode 100644 index 00000000000..1b250f5636f --- /dev/null +++ b/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NON_USER_FACING + issueLink: https://github.com/solo-io/solo-projects/issues/7223 + resolvesIssue: false + description: >- + Remove hard-coded namespaces from the tracing tests. This issue is causing tests to pass in OSS gloo, but fail in solo-projects. From d013dbcb9f7450e2adc04429bd918ed506f7bbd5 Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Thu, 14 Nov 2024 11:53:07 -0500 Subject: [PATCH 4/8] Attempt to clean some stuff up --- test/kubernetes/e2e/features/tracing/suite.go | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/test/kubernetes/e2e/features/tracing/suite.go b/test/kubernetes/e2e/features/tracing/suite.go index 1e8dedd8bc3..c8a101edce8 100644 --- a/test/kubernetes/e2e/features/tracing/suite.go +++ b/test/kubernetes/e2e/features/tracing/suite.go @@ -7,9 +7,13 @@ import ( "github.com/solo-io/gloo/pkg/utils/kubeutils" "github.com/solo-io/gloo/pkg/utils/requestutils/curl" + gloo_defaults "github.com/solo-io/gloo/projects/gloo/pkg/defaults" "github.com/solo-io/gloo/test/gomega/matchers" "github.com/solo-io/gloo/test/kubernetes/e2e" testdefaults "github.com/solo-io/gloo/test/kubernetes/e2e/defaults" + "github.com/solo-io/solo-kit/pkg/api/v1/clients" + "github.com/solo-io/solo-kit/pkg/api/v1/resources" + "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -94,18 +98,35 @@ func (s *testingSuite) BeforeTest(string, string) { otelcolSelector, ) + err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, tracingConfigManifest) + s.NoError(err, "can apply gloo tracing resources") + s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( + func() (resources.InputResource, error) { + return s.testInstallation.ResourceClients.UpstreamClient().Read( + "namespace", "name", clients.ReadOpts{Ctx: s.ctx}) + }, + core.Status_Accepted, + gloo_defaults.GlooReporter, + ) + + time.Sleep(10 * time.Second) + + err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gatewayConfigManifest, + "-n", s.testInstallation.Metadata.InstallNamespace) + s.NoError(err, "can create gateway and service") + // Attempt to apply tracingConfigManifest multiple times. The first time // fails regularly with this message: "failed to validate Proxy [namespace: // gloo-gateway-edge-test, name: gateway-proxy] with gloo validation: // HttpListener Error: ProcessingError. Reason: *v1.Upstream { // default.opentelemetry-collector } not found" - s.Assert().Eventually(func() bool { - // TODO clean this up - err1 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, tracingConfigManifest) - err2 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gatewayConfigManifest, - "-n", s.testInstallation.Metadata.InstallNamespace) - return err1 == nil && err2 == nil - }, time.Second*30, time.Second*5, "can apply gloo tracing config") + // s.Assert().Eventually(func() bool { + // // TODO clean this up + // err1 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, tracingConfigManifest) + // err2 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gatewayConfigManifest, + // "-n", s.testInstallation.Metadata.InstallNamespace) + // return err1 == nil && err2 == nil + // }, time.Second*30, time.Second*5, "can apply gloo tracing config") } func (s *testingSuite) AfterTest(string, string) { From 2361644cb540e6dc19a33b76d518c9b29c770a65 Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Fri, 15 Nov 2024 14:00:57 -0500 Subject: [PATCH 5/8] Clean up debug code --- test/kubernetes/e2e/features/tracing/suite.go | 35 ++++++++++--------- .../features/tracing/testdata/tracing.yaml | 3 +- test/kubernetes/e2e/features/tracing/types.go | 8 +++++ 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/test/kubernetes/e2e/features/tracing/suite.go b/test/kubernetes/e2e/features/tracing/suite.go index c8a101edce8..2b7318d4e07 100644 --- a/test/kubernetes/e2e/features/tracing/suite.go +++ b/test/kubernetes/e2e/features/tracing/suite.go @@ -100,33 +100,36 @@ func (s *testingSuite) BeforeTest(string, string) { err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, tracingConfigManifest) s.NoError(err, "can apply gloo tracing resources") + // accept the upstream s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( func() (resources.InputResource, error) { return s.testInstallation.ResourceClients.UpstreamClient().Read( - "namespace", "name", clients.ReadOpts{Ctx: s.ctx}) + otelcolUpstream.Namespace, otelcolUpstream.Name, clients.ReadOpts{Ctx: s.ctx}) + }, + core.Status_Accepted, + gloo_defaults.GlooReporter, + ) + // accept the virtual service + s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( + func() (resources.InputResource, error) { + return s.testInstallation.ResourceClients.VirtualServiceClient().Read( + tracingVs.Namespace, tracingVs.Name, clients.ReadOpts{Ctx: s.ctx}) }, core.Status_Accepted, gloo_defaults.GlooReporter, ) - - time.Sleep(10 * time.Second) err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gatewayConfigManifest, "-n", s.testInstallation.Metadata.InstallNamespace) s.NoError(err, "can create gateway and service") - - // Attempt to apply tracingConfigManifest multiple times. The first time - // fails regularly with this message: "failed to validate Proxy [namespace: - // gloo-gateway-edge-test, name: gateway-proxy] with gloo validation: - // HttpListener Error: ProcessingError. Reason: *v1.Upstream { - // default.opentelemetry-collector } not found" - // s.Assert().Eventually(func() bool { - // // TODO clean this up - // err1 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, tracingConfigManifest) - // err2 := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gatewayConfigManifest, - // "-n", s.testInstallation.Metadata.InstallNamespace) - // return err1 == nil && err2 == nil - // }, time.Second*30, time.Second*5, "can apply gloo tracing config") + s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( + func() (resources.InputResource, error) { + return s.testInstallation.ResourceClients.GatewayClient().Read( + s.testInstallation.Metadata.InstallNamespace, "gateway-proxy-tracing", clients.ReadOpts{Ctx: s.ctx}) + }, + core.Status_Accepted, + gloo_defaults.GlooReporter, + ) } func (s *testingSuite) AfterTest(string, string) { diff --git a/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml b/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml index 644400683b8..93718e56a6d 100644 --- a/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml +++ b/test/kubernetes/e2e/features/tracing/testdata/tracing.yaml @@ -31,7 +31,8 @@ spec: apiVersion: gateway.solo.io/v1 kind: VirtualService metadata: - name: default + name: virtual-service + namespace: default labels: gateway-type: tracing spec: diff --git a/test/kubernetes/e2e/features/tracing/types.go b/test/kubernetes/e2e/features/tracing/types.go index 2ececcdea9f..a008ff12f2c 100644 --- a/test/kubernetes/e2e/features/tracing/types.go +++ b/test/kubernetes/e2e/features/tracing/types.go @@ -27,4 +27,12 @@ var ( otelcolSelector = metav1.ListOptions{ LabelSelector: "app.kubernetes.io/name=otel-collector", } + otelcolUpstream = &metav1.ObjectMeta{ + Name: "opentelemetry-collector", + Namespace: "default", + } + tracingVs = &metav1.ObjectMeta{ + Name: "virtual-service", + Namespace: "default", + } ) From a6a35a35a27282a839ee59ac891833dbabb4f60d Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Fri, 15 Nov 2024 14:36:47 -0500 Subject: [PATCH 6/8] Code formatting --- test/kubernetes/e2e/features/tracing/suite.go | 2 ++ test/kubernetes/e2e/features/tracing/types.go | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/kubernetes/e2e/features/tracing/suite.go b/test/kubernetes/e2e/features/tracing/suite.go index 2b7318d4e07..c1d2a3034e2 100644 --- a/test/kubernetes/e2e/features/tracing/suite.go +++ b/test/kubernetes/e2e/features/tracing/suite.go @@ -161,6 +161,7 @@ func (s *testingSuite) TestSpanNameTransformationsWithoutRouteDecorator() { &matchers.HttpResponse{ StatusCode: http.StatusOK, }, + 5*time.Second, 30*time.Second, ) s.EventuallyWithT(func(c *assert.CollectT) { @@ -187,6 +188,7 @@ func (s *testingSuite) TestSpanNameTransformationsWithRouteDecorator() { &matchers.HttpResponse{ StatusCode: http.StatusOK, }, + 5*time.Second, 30*time.Second, ) s.EventuallyWithT(func(c *assert.CollectT) { diff --git a/test/kubernetes/e2e/features/tracing/types.go b/test/kubernetes/e2e/features/tracing/types.go index a008ff12f2c..1ea872ed746 100644 --- a/test/kubernetes/e2e/features/tracing/types.go +++ b/test/kubernetes/e2e/features/tracing/types.go @@ -28,11 +28,11 @@ var ( LabelSelector: "app.kubernetes.io/name=otel-collector", } otelcolUpstream = &metav1.ObjectMeta{ - Name: "opentelemetry-collector", + Name: "opentelemetry-collector", Namespace: "default", } tracingVs = &metav1.ObjectMeta{ - Name: "virtual-service", + Name: "virtual-service", Namespace: "default", } ) From 275117dba8e442d5fab19ea420a27d83d299ece9 Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Fri, 15 Nov 2024 15:18:53 -0500 Subject: [PATCH 7/8] Fix a failing unit test --- .../remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml | 6 +++++- test/kubernetes/testutils/helper/util_test.go | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml b/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml index 1b250f5636f..23a513d6240 100644 --- a/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml +++ b/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml @@ -1,6 +1,10 @@ changelog: + - type: NON_USER_FACING + description: >- + Fix failing unit test in `test/kubernetes/testutils/helper/util_test.go` - type: NON_USER_FACING issueLink: https://github.com/solo-io/solo-projects/issues/7223 resolvesIssue: false description: >- - Remove hard-coded namespaces from the tracing tests. This issue is causing tests to pass in OSS gloo, but fail in solo-projects. + Remove hard-coded namespaces from the tracing tests. This issue is + causing tests to pass in OSS gloo, but fail in solo-projects. diff --git a/test/kubernetes/testutils/helper/util_test.go b/test/kubernetes/testutils/helper/util_test.go index 91a8a4ac272..ed8c891639b 100644 --- a/test/kubernetes/testutils/helper/util_test.go +++ b/test/kubernetes/testutils/helper/util_test.go @@ -23,8 +23,8 @@ func TestReturnsLatestPatchForMinor(t *testing.T) { ctx := context.Background() // this is fine because this is a public repo client := githubutils.GetClientWithOrWithoutToken(ctx) - minor, err := getLatestReleasedPatchVersion(ctx, client, "gloo", 1, 8) + minor, err := getLatestReleasedPatchVersion(ctx, client, "gloo", 1, 9) require.NoError(t, err) - assert.Equal(t, "v1.8.37", minor.String()) + assert.Equal(t, "v1.9.30", minor.String()) } From 664f692e023f555a37eed6baafe171937bab5d60 Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Fri, 15 Nov 2024 15:20:37 -0500 Subject: [PATCH 8/8] Fix failing unit tests --- .../remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml | 3 ++- test/kube2e/upgrade/upgrade_utils_test.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml b/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml index 23a513d6240..3692f1b47e7 100644 --- a/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml +++ b/changelog/v1.17.17/remove-hardcoded-namespace-in-tracing-test-v1.17.x.yaml @@ -1,7 +1,8 @@ changelog: - type: NON_USER_FACING description: >- - Fix failing unit test in `test/kubernetes/testutils/helper/util_test.go` + Fix failing unit tests in `test/kubernetes/testutils/helper/util_test.go` + and `test/kubernetes/testutils/helper/util_test.go` - type: NON_USER_FACING issueLink: https://github.com/solo-io/solo-projects/issues/7223 resolvesIssue: false diff --git a/test/kube2e/upgrade/upgrade_utils_test.go b/test/kube2e/upgrade/upgrade_utils_test.go index b12948c79bb..6be4156498a 100644 --- a/test/kube2e/upgrade/upgrade_utils_test.go +++ b/test/kube2e/upgrade/upgrade_utils_test.go @@ -26,9 +26,9 @@ var _ = Describe("upgrade utils unit tests", func() { It("should return latest patch", func() { ctx := context.Background() client, _ := githubutils.GetClient(ctx) - minor, err := getLatestReleasedPatchVersion(ctx, client, "gloo", 1, 8) + minor, err := getLatestReleasedPatchVersion(ctx, client, "gloo", 1, 9) Expect(err).NotTo(HaveOccurred()) - Expect(minor.String()).To(Equal("v1.8.37")) + Expect(minor.String()).To(Equal("v1.9.30")) }) })