Skip to content

Improve running end-to-end tests locally#972

Open
NBardelot wants to merge 7 commits intokubernetes:masterfrom
NBardelot:feature/e2e_behind_proxy
Open

Improve running end-to-end tests locally#972
NBardelot wants to merge 7 commits intokubernetes:masterfrom
NBardelot:feature/e2e_behind_proxy

Conversation

@NBardelot
Copy link
Contributor

@NBardelot NBardelot commented Jan 26, 2026

  • Allow users to run end-to-end tests with a fully-qualified and/or private alpine image
  • Allow users to run end-to-end tests behind a proxy
  • Introduces a new NO_PROXY parameter in the Makefile that is propagated alongside HTTP_PROXY and HTTPS_PROXY
  • Introduces a new LIST_OF_E2E_TESTS parameter in the Makefile that drives the names of the tests passed to ./test_e2e.sh to run a selection of tests, running all tests by default
  • Allow users to override the git-sync repository URL used by end-to-end tests, by exporting GIT_SYNC_REPOSITORY_URL in their environment, in order to use a mirror if need be
  • Sort .gitgnore for convenience for future diffs
  • Add .idea/ to .gitignore

NOTE : I propose this in parallel of the feature request #970 as I had to make some adaptations locally to be able to run the end-to-end tests and I think they could be used by others. It completes the PR #971 that has already been merged.

/kind feature

* Allow users to run end-to-end tests with a fully-qualified
  and/or private alpine image
* Allow users to run end-to-end tests behind a proxy
* Sort .gitgnore for convenience for future diffs
* Add .idea/ to .gitignore
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 26, 2026
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: NBardelot
Once this PR has been reviewed and has the lgtm label, please assign thockin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 26, 2026
@NBardelot NBardelot changed the title Improve running end-to-end locally Improve running end-to-end tests locally Jan 26, 2026
Allow the user running end-to-end tests to override the
repository's URL in order to use a mirror if need be.
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 27, 2026
Introduces a new LIST_OF_E2E_TESTS option in order
to select end-to-end tests that will be executed when
calling `make test`.

It uses the tests script's arguments parsing that already
exists.
This reverts commit 560274698e3b42eaad01648a1afa81bc8e91ea0a.
Introduces a new LIST_OF_E2E_TESTS option in order
to select end-to-end tests that will be executed when
calling `make test`.

It uses the tests script's arguments parsing that already
exists.
Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the PR, but I don't think we want this. The argument to test_e2e.sh is a pattern, not just a list, and besides that, you can just run the script. It's easier than passing a named argument to make.

@thockin thockin closed this Jan 29, 2026
@NBardelot
Copy link
Contributor Author

NBardelot commented Jan 29, 2026

You can just run the script. It's easier than passing a named argument to make.

Cf. my comment in the test_e2e.sh script, the script itself makes a call to make without any possibility to override the makefile's extra parameters (for buildx, or to use fully-qualified images). Thus, you cannot run test_e2e.sh alone in the same conditions you build the rest of the project.

Running a subset of tests directly from make is the easiest workaround, with no refactoring needed. In an ideal world though, the make calls in test_e2e.sh would probably better be refactored out of the script itself, and in the makefile as a dependency goal.

By the way, you might already know of bats-core as a shell test framework, but if you don't it could be of interest to you, given it offloads a lot of the mechanisms you have to maintain in the test_e2e.sh. It could also help split the tests in smaller test suits, easier to manage for a newcomer like me :)

@thockin
Copy link
Member

thockin commented Jan 29, 2026

Somehow I only saw part of this PR when I looked at it before.

I learned of bats well after I did all the work, so didn't bother to convert it.

@thockin thockin reopened this Jan 29, 2026
fi

# Build it
# NOTE: If you want to run the end-to-end tests locally and you need specific Makefile arguments
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not pass the important args?

Copy link
Contributor Author

@NBardelot NBardelot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance I was surpised that the script was running its prerequisites by calling the Makefile. And I ended up calling the given goals ahead of time myself, and finding it simpler. The alternative would be to map 1..1 every Makefile arg as an env variable in the script, which is possible but not very practical.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many would we actually need to pass? On one hand this doesn't affect me personally so I could just not care. OTOH it feels like leaving a trap for future contributors...

/bin/sh -c " \
./build/test.sh ./... \
"
@if [ -n "$(HTTP_PROXY)" ]; then \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this do anything? Each of these blocks runs in its own shell, so export has no meaning:

thockin@thockin-glaptop4:/tmp/m$ cat Makefile 
foo:
	@export FOO=bar
	@echo $$FOO

bar:
	@export FOO=bar; \
	echo $$FOO
thockin@thockin-glaptop4:/tmp/m$ make foo

thockin@thockin-glaptop4:/tmp/m$ make bar
bar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. Let me add a commit to fix this.

I think someone who uses a proxy will have all three variables already configured in a dotenv file or profile, expecting the NO_PROXY standard variable to be passed alongside the two others. It looks strange when this one is not. So the goal here was to be consistent (and not having to debug why something will be routed through the proxy when the user's config contains an exclusion, for example: the private/local images registries, or the github project's local mirror).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with setting the variables for subprocesses. I don't understand what this specific change is doing.

If I apply this delta:

diff --git Makefile Makefile
index 4b1a9c0..3348685 100644
--- Makefile
+++ Makefile
@@ -278,6 +278,9 @@ test: $(BUILD_DIRS)
 	fi
 	VERBOSE=1 ./test_e2e.sh $(LIST_OF_E2E_TESTS)
 
+foo:
+	echo $$HTTP_PROXY $$HTTPS_PROXY $$NO_PROXY
+
 TEST_TOOLS := $(shell find _test_tools/* -type d -printf "%f ")
 test-tools: $(foreach tool, $(TEST_TOOLS), .container-test_tool.$(tool))
 

Here's what I see:

$ HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789 make foo
123 456 789
$ make foo HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789
123 456 789
$ sh -c "export HTTP_PROXY=123; export HTTPS_PROXY=456; export NO_PROXY=789; make foo"
123 456 789

In all 3 cases the variable are already passed down. Moreover, if I copy this pattern you have:

diff --git Makefile Makefile
index 4b1a9c0..1212d2f 100644
--- Makefile
+++ Makefile
@@ -278,6 +278,18 @@ test: $(BUILD_DIRS)
 	fi
 	VERBOSE=1 ./test_e2e.sh $(LIST_OF_E2E_TESTS)
 
+foo:
+	@if [ -n "$(HTTP_PROXY)" ]; then \
+		export HTTP_PROXY="111 $(HTTP_PROXY)"; \
+	fi
+	@if [ -n "$(HTTPS_PROXY)" ]; then \
+		export HTTPS_PROXY="222 $(HTTPS_PROXY)"; \
+	fi
+	@if [ -n "$(NO_PROXY)" ]; then \
+		export NO_PROXY="333 $(NO_PROXY)"; \
+	fi
+	echo $$HTTP_PROXY $$HTTPS_PROXY $$NO_PROXY
+
 TEST_TOOLS := $(shell find _test_tools/* -type d -printf "%f ")
 test-tools: $(foreach tool, $(TEST_TOOLS), .container-test_tool.$(tool))
 

I get:

$ HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789 make foo
123 456 789
$ HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789 make foo
123 456 789
$ sh -c "export HTTP_PROXY=123; export HTTPS_PROXY=456; export NO_PROXY=789; make foo"
123 456 789

The change you made to this rule doesn't DO anything. What problem spurred you to make this specific piece of the change?

test_e2e.sh Outdated
GIT_SYNC \
--one-time \
--repo="https://github.com/kubernetes/git-sync" \
--repo="${GIT_SYNC_REPOSITORY_URL:-https://github.com/kubernetes/git-sync}" \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I get something like:

local default_repo="https://github.com/kubernetes/git-sync"
local repo="${REMOTE_TEST_REPO_URL:-$default_repo}"

Makefile Outdated
# Allow alpine to be pulled from a private registry when building the end-to-end tests images
ALPINE_REGISTRY_PREFIX ?=

# By default all end-to-end tests are executed, but this allows for a manual selection
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarify that the values here are regexes against the testcase names

echo $(VERSION)

# Run the 'test' goal in a single shell
test: .SHELLFLAGS = -c
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is already -c

/bin/sh -c " \
./build/test.sh ./... \
"
@if [ -n "$(HTTP_PROXY)" ]; then \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with setting the variables for subprocesses. I don't understand what this specific change is doing.

If I apply this delta:

diff --git Makefile Makefile
index 4b1a9c0..3348685 100644
--- Makefile
+++ Makefile
@@ -278,6 +278,9 @@ test: $(BUILD_DIRS)
 	fi
 	VERBOSE=1 ./test_e2e.sh $(LIST_OF_E2E_TESTS)
 
+foo:
+	echo $$HTTP_PROXY $$HTTPS_PROXY $$NO_PROXY
+
 TEST_TOOLS := $(shell find _test_tools/* -type d -printf "%f ")
 test-tools: $(foreach tool, $(TEST_TOOLS), .container-test_tool.$(tool))
 

Here's what I see:

$ HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789 make foo
123 456 789
$ make foo HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789
123 456 789
$ sh -c "export HTTP_PROXY=123; export HTTPS_PROXY=456; export NO_PROXY=789; make foo"
123 456 789

In all 3 cases the variable are already passed down. Moreover, if I copy this pattern you have:

diff --git Makefile Makefile
index 4b1a9c0..1212d2f 100644
--- Makefile
+++ Makefile
@@ -278,6 +278,18 @@ test: $(BUILD_DIRS)
 	fi
 	VERBOSE=1 ./test_e2e.sh $(LIST_OF_E2E_TESTS)
 
+foo:
+	@if [ -n "$(HTTP_PROXY)" ]; then \
+		export HTTP_PROXY="111 $(HTTP_PROXY)"; \
+	fi
+	@if [ -n "$(HTTPS_PROXY)" ]; then \
+		export HTTPS_PROXY="222 $(HTTPS_PROXY)"; \
+	fi
+	@if [ -n "$(NO_PROXY)" ]; then \
+		export NO_PROXY="333 $(NO_PROXY)"; \
+	fi
+	echo $$HTTP_PROXY $$HTTPS_PROXY $$NO_PROXY
+
 TEST_TOOLS := $(shell find _test_tools/* -type d -printf "%f ")
 test-tools: $(foreach tool, $(TEST_TOOLS), .container-test_tool.$(tool))
 

I get:

$ HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789 make foo
123 456 789
$ HTTP_PROXY=123 HTTPS_PROXY=456 NO_PROXY=789 make foo
123 456 789
$ sh -c "export HTTP_PROXY=123; export HTTPS_PROXY=456; export NO_PROXY=789; make foo"
123 456 789

The change you made to this rule doesn't DO anything. What problem spurred you to make this specific piece of the change?

fi

# Build it
# NOTE: If you want to run the end-to-end tests locally and you need specific Makefile arguments
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many would we actually need to pass? On one hand this doesn't affect me personally so I could just not care. OTOH it feels like leaving a trap for future contributors...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants