test/e2e: NamespacedTest helper should be able to create/delete multiple namespaces

[sunjayBhatia]

Some tests need to manage multiple namespaces to create resources in.

Currently the NamespacedTest helper and associated NamespacedTestBody signatures are as follows:

type NamespacedTestBody func(string)

func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody) {

We're allowed to specify a namespace and any additional namespaces need to be managed in the test.

We could expand this to support an optional list of namespaces to ensure test boilerplate of managing namespaces is minimized/accidental pollution does not happen.

Ideas:

• change signature to func (f *Framework) NamespacedTest(body NamespacedTestBody, namespaces ...string) {
  • Assumption is that first ns is the "main" one to use
• change signature to func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody, additionalNamespaces ...string) {
  • Gives ability to keep syntax as-is, only tests that need additional nses will use it

See #3803

[sunjayBhatia]

Here's a diff of what the second option above starts to look like:

diff --git a/test/e2e/framework.go b/test/e2e/framework.go
index 8eb42e0b..dc81705e 100644
--- a/test/e2e/framework.go
+++ b/test/e2e/framework.go
@@ -200,9 +200,9 @@ func (f *Framework) T() ginkgo.GinkgoTInterface {
        return f.t
 }

-type NamespacedTestBody func(string)
+type NamespacedTestBody func(namespaces string, additionalNamespaces ...string)

-func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody) {
+func (f *Framework) NamespacedTest(body NamespacedTestBody, namespace string, additionalNamespaces ...string) {
        ginkgo.Context("with namespace: "+namespace, func() {
                ginkgo.BeforeEach(func() {
                        f.CreateNamespace(namespace)
@@ -211,7 +211,7 @@ func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody) {
                        f.DeleteNamespace(namespace, false)
                })

-               body(namespace)
+               body(namespace, additionalNamespaces...)
        })
 }

diff --git a/test/e2e/ingress/001_tls_wildcard_host_test.go b/test/e2e/ingress/001_tls_wildcard_host_test.go
index a70dc8be..139b89d9 100644
--- a/test/e2e/ingress/001_tls_wildcard_host_test.go
+++ b/test/e2e/ingress/001_tls_wildcard_host_test.go
@@ -26,7 +26,7 @@ import (
        metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )

-func testTLSWildcardHost(namespace string) {
+func testTLSWildcardHost(namespace string, _ ...string) {
        Specify("wildcard hostname matching works with TLS", func() {
                t := f.T()
                hostSuffix := "wildcardhost.ingress.projectcontour.io"
diff --git a/test/e2e/ingress/002_ensure_v1beta1_test.go b/test/e2e/ingress/002_ensure_v1beta1_test.go
index 96ba2ce7..2a33ddde 100644
--- a/test/e2e/ingress/002_ensure_v1beta1_test.go
+++ b/test/e2e/ingress/002_ensure_v1beta1_test.go
@@ -26,7 +26,7 @@ import (
        "k8s.io/apimachinery/pkg/util/intstr"
 )

-func testEnsureV1Beta1(namespace string) {
+func testEnsureV1Beta1(namespace string, _ ...string) {
        Specify("Ingress v1beta1 resources continue to work", func() {
                t := f.T()

[skriss]

👍 yeah I was thinking about that second option too, happy to go with that if it seems cleanest

[youngnick]

Option 2 looks good to me.

[github-actions]

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

• Mark this Issue as fresh by commenting
• Close this Issue
• Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

[satyazzz123]

Some tests need to manage multiple namespaces to create resources in.

Currently the NamespacedTest helper and associated NamespacedTestBody signatures are as follows:

type NamespacedTestBody func(string)

func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody) {

We're allowed to specify a namespace and any additional namespaces need to be managed in the test.

We could expand this to support an optional list of namespaces to ensure test boilerplate of managing namespaces is minimized/accidental pollution does not happen.

Ideas:

• change signature to func (f *Framework) NamespacedTest(body NamespacedTestBody, namespaces ...string) {

  • Assumption is that first ns is the "main" one to use

• change signature to func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody, additionalNamespaces ...string) {

  • Gives ability to keep syntax as-is, only tests that need additional nses will use it

@sunjayBhatia I am new to this project. In this issue do I need to make changes in function signautre from this
func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody) to this
func (f *Framework) NamespacedTest(namespace string, body NamespacedTestBody, additionalNamespaces ...string) ? Please can you guide me on how to approach this issue ,Thank you.

[sunjayBhatia]

yep @satyazzz123 that looks correct 👍🏽

another part of this that would be super helpful would be to start using this helper for any tests that currently create any namespaces themselves

[satyazzz123]

another part of this that would be super helpful would be to start using this helper for any tests that currently create any namespaces themselves

@sunjayBhatia when I am testing the already present e2e tests in the /contour/test/e2e directory I am constantly getting tests failed even if I havent altered any code . I am running this command go test --tags=e2e ./httpproxy for running the e2e tests in httpproxy directory. Is it the right command ? Or am I missing something?

plus I have refactored one of the function using creaateNamespace to create namespaces themselves

this is before the changes:

	Context("using root namespaces", func() {
		Context("configured via config CRD", func() {
			rootNamespaces := []string{
				"root-ns-crd-1",
				"root-ns-crd-2",
			}
			nonRootNamespace := "nonroot-ns-crd"

			BeforeEach(func() {
				if !e2e.UsingContourConfigCRD() {
					// Test only applies to contour config CRD.
					Skip("")
				}
				for _, ns := range rootNamespaces {
					f.CreateNamespace(ns)
				}
				contourConfiguration.Spec.HTTPProxy.RootNamespaces = rootNamespaces
			})

			AfterEach(func() {
				for _, ns := range rootNamespaces {
					f.DeleteNamespace(ns, false)
				}
			})

			f.NamespacedTest(nonRootNamespace, testRootNamespaces(rootNamespaces))
		})

this is after the changes

f.NamespacedTest("using root namespaces", func(namespace string) {
    if e2e.UsingContourConfigCRD() {
        Context("configured via config CRD", func() {
            rootNamespaces := []string{
                "root-ns-crd-1",
                "root-ns-crd-2",
            }
            nonRootNamespace := "nonroot-ns-crd"

            BeforeEach(func() {
                for _, ns := range rootNamespaces {
                    f.CreateNamespace(ns)
                }
                contourConfiguration.Spec.HTTPProxy.RootNamespaces = rootNamespaces
            })

            AfterEach(func() {
                for _, ns := range rootNamespaces {
                    f.DeleteNamespace(ns, false)
                }
            })

            f.NamespacedTest(nonRootNamespace, func(namespace string) {
                testRootNamespaces(rootNamespaces)(namespace)
            })
        })
    } else {
   
        Skip("")
    }
})

does the changess look good or what changes do i need to make ? need little help in this Thank you

[sunjayBhatia]

you can use the e2e or run-e2e make targets to run the end to end tests: https://github.com/projectcontour/contour/blob/main/Makefile#L321-L324

you will need docker installed in the environment you are running the tests on, as we use it to create a kind cluster to deploy contour/envoy and run tests against

[satyazzz123]

you can use the e2e or run-e2e make targets to run the end to end tests: https://github.com/projectcontour/contour/blob/main/Makefile#L321-L324

you will need docker installed in the environment you are running the tests on, as we use it to create a kind cluster to deploy contour/envoy and run tests against

Do the code changes look good? is it correct?

[sunjayBhatia]

Do the code changes look good? is it correct?

Not quite no, we want to basically get rid of the parts of the test setup in the original block that are creating/deleting namespaces and pass the namespace names used to the f.NamespacedTest helper you modified earlier so that creates/deletes them

[satyazzz123]

@sunjayBhatia please verify the changes. Thank you