ci(e2e): start envtest cluster for e2e tests #565
Conversation
feloy
requested review from
SoniaSandler and
gastoner
and removed request for
a team
feloy
force-pushed
the
ci/run-envtest-cluster-2
branch
from
d51fbac to
a06699e
Compare
📝 WalkthroughWalkthroughAdds an envtest-managed Kubernetes cluster lifecycle to the E2E GitHub Actions workflow: installs envtest tooling, sets KUBEBUILDER_ASSETS, starts an envtest cluster (captures KUBECONFIG and ENVTEST_START_PID), verifies accessibility, runs E2E smoke tests with propagated env, then stops the cluster. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runner as Actions Runner
participant Setup as setup-envtest
participant EnvStart as envtest-start
participant Test as E2E Test Step
participant Cluster as Envtest K8s Cluster
Runner->>Setup: install & run setup-envtest (determine KUBEBUILDER_ASSETS)
Setup-->>Runner: export KUBEBUILDER_ASSETS
Runner->>EnvStart: install & start envtest (uses KUBEBUILDER_ASSETS)
EnvStart-->>Runner: return KUBECONFIG + ENVTEST_START_PID
EnvStart->>Cluster: boot cluster
Runner->>Cluster: verify accessibility (kubectl get all / grep)
Runner->>Test: run E2E smoke tests with KUBECONFIG & KUBEBUILDER_ASSETS
Test->>Cluster: execute test interactions
Test-->>Runner: test results
Runner->>EnvStart: stop cluster (kill ENVTEST_START_PID)
EnvStart->>Cluster: initiate shutdown
Cluster-->>Runner: stopped
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
feloy
force-pushed
the
ci/run-envtest-cluster-2
branch
from
a06699e to
7c0ab16
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/e2e-tests.yaml (2)
170-176: Consider more robust cluster verification.The current verification uses
grepwhich might fail if output format changes. Consider usingkubectlwith specific resource queries for more reliable verification.- name: check cluster is accessible, should show a service named "kubernetes" env: KUBEBUILDER_ASSETS: ${{ steps.setup-envtest.outputs.KUBEBUILDER_ASSETS }} KUBECONFIG: ${{ steps.run-envtest.outputs.KUBECONFIG }} shell: bash run: | - $KUBEBUILDER_ASSETS/kubectl get all | grep "service/kubernetes" + $KUBEBUILDER_ASSETS/kubectl get service kubernetes --namespace default
187-195: Add error handling for cleanup step.The cleanup step uses
killwhich should work across both Linux and Windows (via bash), but lacks error handling if the process is already stopped or the PID is invalid.- name: stop envtest cluster if: always() env: ENVTEST_START_PID: ${{ steps.run-envtest.outputs.ENVTEST_START_PID }} shell: bash run: | - kill $ENVTEST_START_PID + kill $ENVTEST_START_PID || echo "Process already stopped or PID invalid" # give some time for the cluster to be stopped gracefully sleep 3
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests smoke / ubuntu-24.04
- GitHub Check: e2e tests smoke / windows-2025
- GitHub Check: Build / windows-2022
- GitHub Check: Build extension OCI image
🔇 Additional comments (3)
.github/workflows/e2e-tests.yaml (3)
158-168: LGTM!The envtest cluster startup logic correctly captures the PID for cleanup and stores the KUBECONFIG path for later steps.
145-149: Pin setup-envtest to a release branch; remove unverifiable envtest-start installation.The installation of
github.com/feloy/envtest-startwith commit hash563d135d56015062c35bd20f8c14ab5fcbee38aecannot be verified as this package does not appear to exist in public repositories. Remove this line. Thesetup-envtest@release-0.22installation is valid and documented.
141-143: Update Go version to 1.25.Go version
1.25is valid and represents the latest stable release. Usego-version: '1.25'instead of a nonexistent version.
Signed-off-by: Philippe Martin <phmartin@redhat.com>
feloy
force-pushed
the
ci/run-envtest-cluster-2
branch
from
7c0ab16 to
e18d9bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/e2e-tests.yaml (2)
170-176: Simplify the cluster accessibility check.The current command pipes
kubectl get alltogrep, which retrieves all resources and then filters. A more direct approach would improve efficiency and clarity.Apply this diff to use a more direct command:
- $KUBEBUILDER_ASSETS/kubectl get all | grep "service/kubernetes" + $KUBEBUILDER_ASSETS/kubectl get service kubernetesThis directly checks for the kubernetes service and will fail with a clear error if it doesn't exist.
187-195: Add error handling and verification to cluster cleanup.The cleanup step could be more robust:
- No check if ENVTEST_START_PID is valid before attempting to kill
- No verification that the process actually terminated
- The 3-second sleep is arbitrary
Apply this diff to improve the cleanup:
- name: stop envtest cluster if: always() env: ENVTEST_START_PID: ${{ steps.run-envtest.outputs.ENVTEST_START_PID }} shell: bash run: | - kill $ENVTEST_START_PID - # give some time for the cluster to be stopped gracefully - sleep 3 + if [ -n "$ENVTEST_START_PID" ]; then + kill $ENVTEST_START_PID 2>/dev/null || echo "Process already terminated" + # Wait for process to terminate (max 10 seconds) + for i in {1..10}; do + if ! kill -0 $ENVTEST_START_PID 2>/dev/null; then + echo "Cluster stopped successfully" + break + fi + sleep 1 + done + else + echo "No ENVTEST_START_PID found, skipping cleanup" + fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build / windows-2022
- GitHub Check: Build / ubuntu-24.04
- GitHub Check: Build / macos-14
- GitHub Check: Build extension OCI image
- GitHub Check: Build Extension Image
🔇 Additional comments (3)
.github/workflows/e2e-tests.yaml (3)
151-156: LGTM!The setup-envtest configuration correctly captures KUBEBUILDER_ASSETS and propagates it to subsequent steps.
178-185: LGTM!The KUBEBUILDER_ASSETS and KUBECONFIG environment variables are correctly propagated to the test step.
145-149: Verify that the envtest-start commit hash exists and is compatible with setup-envtest release-0.22.The
go installfor envtest-start pins to a specific commit that should be validated. While setup-envtest release-0.22 is documented, the envtest-start tool and the pinned commit lack public documentation. Confirm the commit is valid and these tools work together correctly in your CI environment.
gastoner
left a comment
gastoner
left a comment
There was a problem hiding this comment.
Maybe just questin is there a reason why we don't run the tests on Mac? (separate issue)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/e2e-tests.yaml (3)
170-176: Add a brief wait or check for kubeconfig file existence.The envtest cluster is started in the background in the previous step, but there's no verification that the kubeconfig file has been written before this accessibility check runs. While envtest is lightweight, there could be a brief moment where the file doesn't exist yet, causing this step to fail.
🔎 Consider adding a kubeconfig existence check:
- name: check cluster is accessible, should show a service named "kubernetes" env: KUBEBUILDER_ASSETS: ${{ steps.setup-envtest.outputs.KUBEBUILDER_ASSETS }} KUBECONFIG: ${{ steps.run-envtest.outputs.KUBECONFIG }} shell: bash run: | + # Brief wait for kubeconfig to be written + for i in {1..10}; do + if [ -f "$KUBECONFIG" ]; then + break + fi + echo "Waiting for kubeconfig to be written (attempt $i/10)..." + sleep 0.5 + done $KUBEBUILDER_ASSETS/kubectl get all | grep "service/kubernetes"
187-197: Add error handling for process termination.The cleanup step could fail if the process has already terminated or if the PID is invalid. Since this runs with
if: always(), adding error handling ensures the workflow doesn't show as failed during cleanup.🔎 Add error handling to make cleanup more resilient:
- name: stop envtest cluster if: always() env: ENVTEST_START_PID: ${{ steps.run-envtest.outputs.ENVTEST_START_PID }} shell: bash run: | - kill $ENVTEST_START_PID + if kill $ENVTEST_START_PID 2>/dev/null; then + echo "Sent termination signal to process $ENVTEST_START_PID" + else + echo "Process $ENVTEST_START_PID not found or already terminated" + exit 0 + fi # give some time for the cluster to be stopped gracefully echo 'waiting process to be stopped...' - wait $ENVTEST_START_PID + wait $ENVTEST_START_PID 2>/dev/null || true echo 'process stopped'
145-149: Pin setup-envtest to a stable release tag instead of a branch reference.The release-0.22 branch exists in kubernetes-sigs/controller-runtime, but branches can shift. Consider pinning to a specific release tag (e.g.,
v0.22.0or similar) for better reproducibility and to prevent unexpected changes if the branch is rebased or deleted.For the envtest-start binary, verify that the commit hash
563d135d56015062c35bd20f8c14ab5fcbee38aeis accessible and that the repository is actively maintained.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build / macos-14
- GitHub Check: Build / ubuntu-24.04
- GitHub Check: Build / windows-2022
- GitHub Check: Build extension OCI image
- GitHub Check: Build Extension Image
🔇 Additional comments (3)
.github/workflows/e2e-tests.yaml (3)
151-156: LGTM!The step correctly uses
setup-envtestto get the KUBEBUILDER_ASSETS path and exports it for downstream steps.
158-168: LGTM!The step correctly starts the envtest cluster in the background and captures the process ID and kubeconfig path. Using
runner.tempfor the kubeconfig ensures cross-platform compatibility.
178-185: LGTM!The E2E test step correctly receives the KUBEBUILDER_ASSETS and KUBECONFIG environment variables. The comment indentation is also correct.
feloy
force-pushed
the
ci/run-envtest-cluster-2
branch
from
7655275 to
3f1d90c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/e2e-tests.yaml (2)
170-176: Consider adding explicit error message.The grep command will fail silently if the pattern isn't found. Adding an explicit check or error message would improve debuggability.
🔎 Suggested improvement:
shell: bash run: | - $KUBEBUILDER_ASSETS/kubectl get all | grep "service/kubernetes" + if ! $KUBEBUILDER_ASSETS/kubectl get all | grep "service/kubernetes"; then + echo "Error: kubernetes service not found in cluster" + exit 1 + fi
193-193: Consider graceful shutdown signal.Using
killwithout a signal sendsSIGTERMby default on Linux, but explicitly specifying it improves clarity. For more graceful shutdown, consider usingkill -TERMexplicitly.🔎 Suggested change:
- kill $ENVTEST_START_PID + kill -TERM $ENVTEST_START_PID
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Extension Image
- GitHub Check: Build / macos-14
- GitHub Check: Build / windows-2022
- GitHub Check: Build / ubuntu-24.04
- GitHub Check: Build extension OCI image
🔇 Additional comments (5)
.github/workflows/e2e-tests.yaml (5)
145-149: Good practice: pinned to specific versions.Installing
envtest-startwith a commit hash andsetup-envtestwith a release tag ensures reproducible builds and prevents unexpected breakages from upstream changes.
151-156: LGTM!The step correctly installs envtest assets and captures the path for downstream steps.
158-168: Cluster startup approach is reasonable.Starting the envtest cluster in the background and capturing the PID for later cleanup is appropriate. As discussed in previous comments, retry logic can be added later if flakiness occurs.
178-185: LGTM!Environment variables are correctly propagated to the E2E test step, providing the test runner with access to the envtest cluster.
143-143: Remove this review comment - Go 1.25 is valid.Go 1.25.0 is a major release of Go, with go1.25.5 released 2025-12-02. The version string '1.25' is correctly formatted for workflow use, following Go's standard versioning where major.minor versions are valid release designators.
feloy
force-pushed
the
ci/run-envtest-cluster-2
branch
from
3f1d90c to
e4d210e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/e2e-tests.yaml (2)
141-143: Past issue: Pin action to commit hash for security.All other actions in this workflow are pinned to specific commit hashes, but this action uses a tag reference.
Verify Go version validity.
Go version '1.25' appears to be specified, but this version may not exist yet or may be a typo. As of late 2024, the latest Go stable release was in the 1.23.x series.
What is the latest stable version of Go as of December 2025?
187-197: Past issue: Critical Windows compatibility issues in cleanup step.This cleanup step uses Linux-specific commands (
kill,ps,timeout 10s) that will fail on Windows runners. The workflow matrix includeswindows-2025.
🧹 Nitpick comments (1)
.github/workflows/e2e-tests.yaml (1)
145-149: Pin setup-envtest to a commit hash for consistency.setup-envtest supports Windows with OS-specific storage paths, and Windows binaries are available for envtest. The security concern remains valid: while
envtest-startis pinned to commit563d135d56015062c35bd20f8c14ab5fcbee38ae,setup-envtestuses tag@release-0.22. For consistency with your security posture, consider pinningsetup-envtestto a specific commit hash.The workflow matrix includes
windows-2025, and the official Kubernetes envtest binaries support Windows. However, verify thatenvtest-start(the separate feloy tool) is compatible with Windows runners if this job runs on windows-2025.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build / ubuntu-24.04
- GitHub Check: Build Extension Image
- GitHub Check: Build / windows-2022
- GitHub Check: Build / macos-14
- GitHub Check: Build extension OCI image
🔇 Additional comments (4)
.github/workflows/e2e-tests.yaml (4)
151-156: LGTM: Correct capture of KUBEBUILDER_ASSETS.The step correctly uses
setup-envtestto obtain the path to Kubernetes test binaries and exports it for use in subsequent steps.
170-176: LGTM: Good verification step.This accessibility check confirms the envtest cluster started successfully before running tests, which helps catch startup issues early.
Note: This step may be affected by the kubectl binary path issue flagged in the previous segment.
184-185: LGTM: Correct environment variable propagation.The cluster configuration is properly propagated to the test step, allowing tests to access the envtest cluster.
158-168: The review comment's concern about Windows compatibility is partially valid but incomplete. While Git Bash on Windows requires explicit.exeextensions for arbitrary binaries outside the MSYS2 standard paths, the code in lines 158-168 executesenvtest-start(a Go binary that handles platform-specific details internally), so it should work on Windows. However, if other workflow steps directly invoke kubectl from bash on Windows without the extension, that would fail. Verify whether subsequent steps use kubectl directly from within bash shells and clarify the scope of the concern.
feloy
force-pushed
the
ci/run-envtest-cluster-2
branch
6 times, most recently
from
88f4944 to
b9c4941
Compare
Signed-off-by: Philippe Martin <phmartin@redhat.com>
feloy
force-pushed
the
ci/run-envtest-cluster-2
branch
from
b9c4941 to
05f3283
Compare
2 participants
Start a Kubernetes cluster using envtest for e2e tests. The cluster is not used for the moment in the tests, this will come in follow-up PRs.
Check the action result, to see if the cluster s started correctly:
Fixes #76