refactor: E2E tests to support parallel tests #2501

Revolyssup · 2025-07-31T16:02:15Z

What this PR does / why we need it:

Currently running tests in parallel doesn't work due to conflict of Shared resources. For instance, the tests in crds/v2 share GatewayProxy and IngressClass configuration causing issues while running tests in parallel.
Goal of this PR is to eliminate the use of shared resources and use unique resources for tests.

Eventually from here and now, we need unique resource <name/namespace> for GatewayProxy, IngressClass, Gateway, GatewayClass, Ingress(similarly HTTPRoute, etc) so that parallel tests don't override each other's configuration.
In this PR, I have replaced all static names with s.Namespace() as it already provides a unique string.

Things to note before review:

In the test helper function, if replica is 0 then test will not tunnel and wait for pod.
The test that require to share resource like "IngressClass" for eg: testing default ingressclass cannot be run in parallel therefore have been marked as Serial.
To make tests more resilient, direct request calls have been replaced with Eventully() block to retry until success avoiding Sleep.
The timeout has been increased for retries.

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

…into revolyssup/ci

test/e2e/crds/v1alpha1/gatewayproxy.go

Revolyssup · 2025-08-07T16:53:34Z

Another way of doing this is to create a random name generator like in this PR
: https://github.com/Revolyssup/apisix-ingress-controller/pull/4/files#diff-d32745cfa87775abccdb567bf340d4532ae85ca97ed745d25bd56337e2c08658

If this method looks better, then reviewers can suggest.

AlinsRan · 2025-08-08T01:08:33Z

https://github.com/apache/apisix-ingress-controller/actions/runs/16802790400/job/47587870938?pr=2501#step:10:16666

Each job ran all the use cases.
Referring to the previous e2e-test, we need to add label-filter

Copilot

Pull Request Overview

This PR refactors E2E tests to support parallel execution by eliminating shared resources that cause conflicts when tests run simultaneously. The changes replace static names with unique identifiers (primarily using s.Namespace()) for resources like GatewayProxy, IngressClass, Gateway, and other Kubernetes objects to ensure test isolation.

Key Changes

Replace static resource names with unique names based on test namespace
Update timeout values to be more resilient for parallel execution
Mark certain tests requiring shared resources (like default IngressClass) as Serial
Replace direct request calls with Eventually() blocks for better retry handling

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/e2e/scaffold/assertion.go	Increase timeout and interval values for better parallel test stability
test/e2e/scaffold/apisix_deployer.go	Add conditional logic to skip tunnels and pod waiting when replicas is 0
test/e2e/ingress/ingress.go	Convert static resource names to namespace-based unique names, mark Serial tests
test/e2e/gatewayapi/*.go	Update all Gateway API tests with unique resource naming and increased timeouts
test/e2e/crds/v2/*.go	Refactor CRD v2 tests with unique naming and improved timeout handling
test/e2e/crds/v1alpha1/*.go	Update CRD v1alpha1 tests for parallel execution compatibility
test/e2e/framework/assertion.go	Increase polling intervals and timeouts for more reliable assertions
Makefile	Increase E2E_NODES from 2 to 4 and add label filtering support

Copilot · 2025-08-08T09:29:42Z

test/e2e/ingress/ingress.go

+				Check:    scaffold.WithExpectedStatus(http.StatusOK),
+				Interval: time.Second * 10,
+				Timeout:  3 * time.Minute,
+			})


The sleep duration of 10 minutes is excessively long and will significantly slow down test execution. Consider reducing this to a more reasonable duration like 30 seconds or use Eventually() blocks instead.

Suggested change

})

Eventually(func(g Gomega) {

s.RequestAssert(&scaffold.RequestAssert{

Method: "GET",

Path: "/get",

Host: "httpbin.external",

Check: scaffold.WithExpectedStatus(http.StatusOK),

Interval: time.Second * 10,

Timeout: 3 * time.Minute,

})

}, 3*time.Minute, 10*time.Second).Should(Succeed())

Copilot · 2025-08-08T09:29:42Z

test/e2e/ingress/ingress.go

-				Expect().
-				Status(200)
-
+			time.Sleep(10 * time.Minute)


Another 10-minute sleep that will make tests unnecessarily slow. This should be reduced to a reasonable duration or replaced with proper wait conditions.

Suggested change

time.Sleep(10 * time.Minute)

// Removed unnecessary 10-minute sleep; rely on RequestAssert for readiness

wow I really forgot to remove this😂

Copilot · 2025-08-08T09:29:42Z

test/e2e/gatewayapi/httproute.go

+				Timeout:  30 * time.Second,
 			})

+			time.Sleep((8 * time.Second))


Unnecessary parentheses around the sleep duration expression. This should be simplified to time.Sleep(8 * time.Second).

Suggested change

time.Sleep((8 * time.Second))

time.Sleep(8 * time.Second)

Copilot · 2025-08-08T09:29:43Z

test/e2e/scaffold/apisix_deployer.go

-		LabelSelector: "app.kubernetes.io/name=apisix",
-	})
-	Expect(err).ToNot(HaveOccurred(), "waiting for gateway pod ready")
+	if opts.Replicas == nil || (opts.Replicas != nil && *opts.Replicas > 0) {


The condition can be simplified. Since opts.Replicas != nil is redundant when already checked with ||, this can be written as if opts.Replicas == nil || *opts.Replicas > 0 {.

Suggested change

if opts.Replicas == nil || (opts.Replicas != nil && *opts.Replicas > 0) {

if opts.Replicas == nil || *opts.Replicas > 0 {

Copilot · 2025-08-08T09:29:43Z

test/e2e/crds/v2/route.go

 			}

 			By("wait for route to be ready")
+


There's an empty line followed by a comment without any actual waiting logic. This appears to be incomplete - either add the waiting logic or remove the misleading comment.

Suggested change

ronething · 2025-08-08T09:34:03Z

.github/workflows/apisix-e2e-test.yml

+      - name: Install ginkgo
+        run: |
+            GINKGO_VERSION=$(cd test/e2e && go list -m  -mod=readonly  -f {{.Version}}  github.com/onsi/ginkgo/v2)
+            go install github.com/onsi/ginkgo/v2/ginkgo@$GINKGO_VERSION


use make install-ginkgo?

ronething · 2025-08-08T09:35:31Z

test/e2e/crds/v1alpha1/backendtrafficpolicy.go

-	s := scaffold.NewDefaultScaffold()
+	var (
+		s = scaffold.NewScaffold(&scaffold.Options{
+			ControllerName: fmt.Sprintf("apisix.apache.org/apisix-ingress-controller-%d", time.Now().Unix()),


This can be used as the default behavior.

It already has default behavior:
https://github.com/apache/apisix-ingress-controller/blob/master/test/e2e/scaffold/apisix_deployer.go#L69

revered to defaultscaffold

I have reverted back to this behavior. I see a lot of tests fail when defaultScaffold is used. I am looking into the reason but for now I have reverted to this.

AlinsRan · 2025-08-08T09:42:09Z

test/e2e/crds/v1alpha1/backendtrafficpolicy.go

+			time.Sleep(time.Second)
+
+			By("create Gateway")
+			err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultGateway, gatewayName, gatewayClassName, gatewayProxyName), s.Namespace())


CreateResourceFromString uses the namespace of the current test, so there's no need to explicitly specify it for namespace-scoped resources.

Only cluster level resources will use CreateResourceFromStringWithnamespace, or a fixed namespace.

ronething · 2025-08-08T09:42:33Z

test/e2e/crds/v2/config.go

+func getGatewayProxyYaml(namespace, endpoint, adminKey string) string {
+	return fmt.Sprintf(gatewayProxyYaml, namespace, namespace, endpoint, adminKey)
+}


The function can be encapsulated in the Scaffold, so we don't need to manually pass many parameters.

test/e2e/crds/v2/route.go

This reverts commit 89646cc.

This reverts commit 470c6eb.

…into revolyssup/ci

ronething

lgtm

test/e2e/crds/v2/route.go

chore: refactor E2E tests to support parallel tests

4842859

Revolyssup marked this pull request as draft July 31, 2025 16:02

Revolyssup added 5 commits July 31, 2025 22:06

fix ci

c2340e5

install ginkgo

0fe8b52

Merge branch 'master' of github.com:apache/apisix-ingress-controller …

bbadbbd

…into revolyssup/ci

fix

f3b6bf8

fix

835a81d

github-advanced-security bot found potential problems Aug 1, 2025

View reviewed changes

test/e2e/crds/v1alpha1/gatewayproxy.go Fixed Show fixed Hide fixed

Revolyssup added 15 commits August 1, 2025 19:19

fix

d164ae8

fix ingress test

e7ec20a

fix tls

26ada20

fix tls

6afab7e

fix

9dcb4ef

fix

0741306

fix ing

7b28f3b

fix ing

1906033

fix ing

79600b4

pause ingress tests

2f17c77

fix consumer

91feeaf

pause defaultingress tests

badb102

fix

be9929a

fix lint

4f3edb0

fix lint

c3f8dbb

Revolyssup changed the title ~~[DRAFT]chore: refactor E2E tests to support parallel tests~~ chore: refactor E2E tests to support parallel tests Aug 4, 2025

Revolyssup marked this pull request as ready for review August 4, 2025 19:54

Revolyssup added 5 commits August 5, 2025 12:11

fix

7e0c971

fix

0ab4fdc

fix

19cd878

fix lint

8f33813

fix status

7b35de6

Revolyssup added 2 commits August 8, 2025 10:07

use 4 nodes

fcef3fd

add label filter

dcad5b1

ronething requested a review from Copilot August 8, 2025 09:27

Copilot AI reviewed Aug 8, 2025

View reviewed changes

ronething reviewed Aug 8, 2025

View reviewed changes

remove sleep

9bb7169

ronething reviewed Aug 8, 2025

View reviewed changes

remove pause from tests

d327d42

AlinsRan reviewed Aug 8, 2025

View reviewed changes

ronething reviewed Aug 8, 2025

View reviewed changes

test/e2e/crds/v2/route.go Outdated Show resolved Hide resolved

Revolyssup added 13 commits August 8, 2025 17:44

remove redundant createresourcefromstringwithnamespace

8ff0cb4

apply copilot suggestion

fc3f4c0

apply suggestions

470c6eb

apply sug

89646cc

Revert "apply sug"

6375c58

This reverts commit 89646cc.

Revert "apply suggestions"

6e579f1

This reverts commit 470c6eb.

use make install-ginkgo

9c1babf

increase timeout

f73fffb

increase timeout

9b715d0

Merge branch 'master' of github.com:apache/apisix-ingress-controller …

cd7034b

…into revolyssup/ci

use default scaffold

097efb1

apply suggestions

c4eb985

revert default scaffold

c856c8f

ronething approved these changes Aug 12, 2025

View reviewed changes

AlinsRan reviewed Aug 13, 2025

View reviewed changes

test/e2e/crds/v2/route.go Show resolved Hide resolved

AlinsRan approved these changes Aug 13, 2025

View reviewed changes

Revolyssup merged commit ce0c5f4 into apache:master Aug 13, 2025
27 of 28 checks passed

-			})
+			Eventually(func(g Gomega) {
+				s.RequestAssert(&scaffold.RequestAssert{
+					Method:   "GET",
+					Path:     "/get",
+					Host:     "httpbin.external",
+					Check:    scaffold.WithExpectedStatus(http.StatusOK),
+					Interval: time.Second * 10,
+					Timeout:  3 * time.Minute,
+				})
+			}, 3*time.Minute, 10*time.Second).Should(Succeed())

	time.Sleep(10 * time.Minute)
	// Removed unnecessary 10-minute sleep; rely on RequestAssert for readiness

	if opts.Replicas == nil \|\| (opts.Replicas != nil && *opts.Replicas > 0) {
	if opts.Replicas == nil \|\| *opts.Replicas > 0 {

refactor: E2E tests to support parallel tests #2501

refactor: E2E tests to support parallel tests #2501

Uh oh!

Conversation

Revolyssup commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Things to note before review:

Pre-submission checklist:

Uh oh!

Uh oh!

Revolyssup commented Aug 7, 2025

Uh oh!

AlinsRan commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlinsRan Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ronething Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ronething left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Revolyssup commented Jul 31, 2025 •

edited

Loading

AlinsRan commented Aug 8, 2025 •

edited

Loading

AlinsRan Aug 8, 2025 •

edited

Loading

ronething Aug 8, 2025 •

edited

Loading