Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,23 @@ E2E_TIMEOUT ?= 20m
GODOG_ARGS ?=
.PHONY: e2e
e2e: #EXHELP Run the e2e tests.
go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT) $(if $(GODOG_ARGS),-args $(GODOG_ARGS))
ifeq ($(strip $(GODOG_ARGS)),)
set +e; \
timeout -s SIGTERM $(E2E_TIMEOUT) bash -c 'go test -count=1 -v ./test/e2e/features_test.go -args --godog.tags="~@Serial" --godog.concurrency=100; \
parallelExitCode=$$?; \
go test -count=1 -v ./test/e2e/features_test.go -args --godog.tags="@Serial" --godog.concurrency=1; \
serialExitCode=$$?'; \
timeoutExitCode=$$?; \
if [[ $$timeoutExitCode -ne 0 ]]; then \
echo "e2e tests failed: tests timed out"; \
exit 1; \
elif [[ $$parallelExitCode -ne 0 ]] || [[ $$serialExitCode -ne 0 ]]; then \
echo "e2e tests failed: parallel=$$parallelExitCode serial=$$serialExitCode"; \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: variable scoping silently swallows test failures

parallelExitCode and serialExitCode are set inside the bash -c '...' subshell, but read outside it. Variables set in a subshell are not visible to the calling shell.

In bash, [[ "" -ne 0 ]] treats the empty string as 0, so the elif is always false. Additionally, the last statement inside bash -c is a variable assignment (serialExitCode=$?), which always exits 0, so timeout also returns 0 regardless of test results.

Net effect: if either test phase fails, make e2e still exits 0. CI passes today because the tests pass — this only manifests on failure, which is exactly when you need it.

Suggested fix — move the exit-code logic inside the bash -c subshell:

ifeq ($(strip $(GODOG_ARGS)),)
	set +e; \
	timeout -s SIGTERM $(E2E_TIMEOUT) bash -c ' \
		go test -count=1 -v ./test/e2e/features_test.go -args --godog.tags="~@Serial" --godog.concurrency=100; \
		parallelExit=$$?; \
		go test -count=1 -v ./test/e2e/features_test.go -args --godog.tags="@Serial" --godog.concurrency=1; \
		serialExit=$$?; \
		if [[ $$parallelExit -ne 0 ]] || [[ $$serialExit -ne 0 ]]; then \
			echo "e2e tests failed: parallel=$$parallelExit serial=$$serialExit"; \
			exit 1; \
		fi'; \
	exitCode=$$?; \
	if [[ $$exitCode -ne 0 ]]; then \
		exit 1; \
	fi
else

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing out the critical issue - obviously that will need to be fixed.

However, I don't want to use this suggestion as is, because the timeout subcommand cannot be interrupted with SIGINT written this way, it completely ignores it. Running with the --foreground flag brings back the shell prompt but the tests continue to execute in the background.

Adding the required components to allow SIGINT (trap, wait, etc) starts making this subscript really complex and hard to read, so I'll write a shell script and have the make target execute that.

exit 1; \
fi
else
go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT) -args $(GODOG_ARGS)
endif

export CLUSTER_REGISTRY_HOST := docker-registry.operator-controller-e2e.svc:5000
.PHONY: extension-developer-e2e
Expand Down
35 changes: 35 additions & 0 deletions test/e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ Use these variables in YAML templates:

### 5. Feature Tags

Tags can be used for different purposes in the test suite:

#### Feature Gate Tags

Use tags to conditionally run scenarios based on feature gates:

```gherkin
Expand All @@ -216,6 +220,28 @@ Scenario: Install operator having webhooks

Scenarios are skipped if the feature gate is not enabled on the deployed controller.

#### Serial Execution Tag

By default, scenarios run concurrently (up to 100 parallel scenarios). However, some tests must run serially, typically because they:
- Modify shared cluster resources (e.g., cluster-wide TLS configuration)
- Have resource constraints that prevent parallel execution
- Require exclusive access to a resource

To mark a test for serial execution, add the `@Serial` tag:

```gherkin
@Serial
Feature: TLS profile enforcement on metrics endpoints

Scenario: Test TLS configuration
Given the "catalogd" deployment is configured with custom TLS settings
...
```

The `Makefile` automatically separates scenarios when run without additional `GODOG_ARGS`:
- Scenarios **without** `@Serial` run concurrently in the first test phase
- Scenarios **with** `@Serial` run sequentially in a separate serial test phase

## Running Tests

### Run All Tests
Expand All @@ -230,6 +256,15 @@ or
make test-experimental-e2e
```

Custom godog arguments can be modified by setting the following:
```bash
GODOG_ARGS=--godog.tags=@WebhookProviderCertManager make test-experimental-e2e
```

Note that when this is done the `make` target will no longer automatically split the test run into parallel and serial runs, and test execution time may increase. If you wish to add concurrency back into the arguments, it is recommended to also disable the `@Serial` tests:
```bash
GODOG_ARGS="--godog.tags=~@Serial --godog.concurrency=100" make test-experimental-e2e
```
Comment thread
dtfranz marked this conversation as resolved.

### Run Specific Feature

Expand Down
1 change: 1 addition & 0 deletions test/e2e/features/ha.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@Serial
Feature: HA failover for catalogd

When catalogd is deployed with multiple replicas, the remaining pods must
Expand Down
1 change: 1 addition & 0 deletions test/e2e/features/proxy.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@Serial
Feature: HTTPS proxy support for outbound catalog requests

OLM's operator-controller fetches catalog data from catalogd over HTTPS.
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/features/revision.feature
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,7 @@ Feature: Install ClusterObjectSet
And resource "deployment/test-deployment" is eventually not found

@ProgressDeadline
@Serial
Scenario: COS recovers from ProgressDeadlineExceeded to Succeeded when probes pass
Given min value for ClusterObjectSet .spec.progressDeadlineMinutes is set to 1
And ServiceAccount "olm-sa" with needed permissions is available in test namespace
Expand Down Expand Up @@ -721,6 +722,7 @@ Feature: Install ClusterObjectSet
containers:
- name: delayed-ready
image: busybox:1.36
imagePullPolicy: IfNotPresent
command: ["sleep", "1000"]
readinessProbe:
exec:
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/features/tls.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@Serial
Feature: TLS profile enforcement on metrics endpoints
Comment thread
dtfranz marked this conversation as resolved.

Background:
Expand All @@ -6,11 +7,15 @@ Feature: TLS profile enforcement on metrics endpoints
# Each scenario patches the deployment with the TLS settings under test and
# restores the original configuration during cleanup, so scenarios are independent.

# This feature file is run serially to avoid potential issues modifying the catalogd
# deployment during functional testing.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: trailing whitespace on lines 9 and 11.


# All three scenarios test catalogd only: the enforcement logic lives in the shared
# tlsprofiles package, so one component is sufficient. TLS 1.2 is used for cipher
# and curve enforcement because Go's crypto/tls does not allow the server to restrict
# TLS 1.3 cipher suites — CipherSuites config only applies to TLS 1.2. The e2e cert
# uses ECDSA, so ECDHE_ECDSA cipher families are required.

@TLSProfile
Scenario: catalogd metrics endpoint enforces configured minimum TLS version
Given the "catalogd" deployment is configured with custom TLS minimum version "TLSv1.3"
Expand Down