NO-JIRA: add ENVTEST_JOBS knob for parallel envtest execution#8243
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@jparrill: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request adds Sequence Diagram(s)sequenceDiagram
participant User
participant Make as Make (main)
participant Fetch as Asset Fetch
participant Parallel as Parallel Make<br/>(v1..vN)
participant Test as Test Runner<br/>(_run-single-envtest-%)
User->>Make: make test-envtest-kube<br/>ENVTEST_JOBS=3
Make->>Fetch: Pre-fetch envtest assets<br/>for all versions
Fetch-->>Make: Assets ready
Make->>Parallel: make -j3 _run-single-envtest-v1 _run-single-envtest-v2 _run-single-envtest-v3
par Parallel Execution
Parallel->>Test: run v1 test
Parallel->>Test: run v2 test
Parallel->>Test: run v3 test
and
Test->>Test: go test -tags envtest<br/>(isolated env v1)
Test->>Test: go test -tags envtest<br/>(isolated env v2)
Test->>Test: go test -tags envtest<br/>(isolated env v3)
end
Test-->>Parallel: Results
Parallel-->>Make: All tests done
Make-->>User: Complete
sequenceDiagram
participant User
participant Make as Make (main)
participant Test as Test Runner<br/>(_run-single-envtest-%)
User->>Make: make test-envtest-kube<br/>ENVTEST_JOBS=0
rect rgba(200, 100, 100, 0.5)
Note over Make,Test: Sequential execution (per-version)
end
loop For each version (v1, v2, v3...)
Make->>Test: _run-single-envtest-v1
Test->>Test: Fetch assets + go test
Test-->>Make: Complete
Make->>Test: _run-single-envtest-v2
Test->>Test: Fetch assets + go test
Test-->>Make: Complete
end
Make-->>User: All tests done
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
area/testing
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
approved
|
This is how looks like the execution in local: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
368-369: Add upfront validation forENVTEST_JOBSto fail early with clear error messages.Currently, invalid values (e.g.,
ENVTEST_JOBS=abcor negative numbers) pass through tomake -j...on lines 401 and 432, causing vague parallelism errors. Adding a check near the target entry catches this immediately.Suggested fix
test-envtest-ocp: generate $(SETUP_ENVTEST) ## Run envtest tests for all supported OCP versions (ENVTEST_JOBS=0|N|MAX) + `@if` ! echo "$(ENVTEST_JOBS)" | grep -Eq '^(0|MAX|[1-9][0-9]*)$$'; then \ + echo "Invalid ENVTEST_JOBS='$(ENVTEST_JOBS)'. Use 0, MAX, or a positive integer."; \ + exit 2; \ + fi ifeq ($(ENVTEST_JOBS),0) @@ test-envtest-kube: generate $(SETUP_ENVTEST) ## Run envtest tests for all supported Kubernetes versions (ENVTEST_JOBS=0|N|MAX) + `@if` ! echo "$(ENVTEST_JOBS)" | grep -Eq '^(0|MAX|[1-9][0-9]*)$$'; then \ + echo "Invalid ENVTEST_JOBS='$(ENVTEST_JOBS)'. Use 0, MAX, or a positive integer."; \ + exit 2; \ + fi ifeq ($(ENVTEST_JOBS),0)Also applies to lines 410–411.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 368 - 369, Add upfront validation for the ENVTEST_JOBS Make variable so invalid values (non-integer, negative) fail fast with a clear message; detect the special "MAX" token or a non-negative integer, and if the value is invalid call $(error ...) from the Makefile before any make -j$(ENVTEST_JOBS) invocations (referencing the ENVTEST_JOBS variable and the targets that run make -j), e.g., insert a validation block near the target entry that checks ENVTEST_JOBS with make functions (filter, or shell test via expr/grep) and emits a descriptive error like "ENVTEST_JOBS must be 'MAX' or a non-negative integer" when the check fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 389-392: The envtest prefetch loops (iterating over
ENVTEST_OCP_K8S_VERSIONS and the other mirror loop) currently redirect
$(SETUP_ENVTEST) output to /dev/null but do not abort on failure; update both
loops so the $(SETUP_ENVTEST) use ... $$k8s_ver > /dev/null command is followed
by a failure guard (e.g. || exit 1 or || { echo "Failed to fetch envtest assets
for $$k8s_ver"; exit 1; }) so that failures in SETUP_ENVTEST cause the Makefile
to stop instead of continuing with stale/missing assets; apply the same change
to the second loop that uses ENVTEST_OCP_ASSETS_DIR/ENVTEST_OCP_INDEX to ensure
both prefetch loops fail fast.
---
Nitpick comments:
In `@Makefile`:
- Around line 368-369: Add upfront validation for the ENVTEST_JOBS Make variable
so invalid values (non-integer, negative) fail fast with a clear message; detect
the special "MAX" token or a non-negative integer, and if the value is invalid
call $(error ...) from the Makefile before any make -j$(ENVTEST_JOBS)
invocations (referencing the ENVTEST_JOBS variable and the targets that run make
-j), e.g., insert a validation block near the target entry that checks
ENVTEST_JOBS with make functions (filter, or shell test via expr/grep) and emits
a descriptive error like "ENVTEST_JOBS must be 'MAX' or a non-negative integer"
when the check fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: af7ffe58-75df-4809-ab0f-e9ea8202a559
📒 Files selected for processing (2)
Makefiletest/envtest/README.md
jparrill
force-pushed
the
parallel-env-test
branch
from
90a83ff to
80bb3fe
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Makefile (1)
394-397:⚠️ Potential issue | 🟠 MajorFail fast on envtest asset prefetch failures.
Both prefetch loops ignore
setup-envtestexit status. A failed fetch can silently continue and only fail later with harder-to-debug test errors.Suggested fix
`@for` k8s_ver in $(ENVTEST_OCP_K8S_VERSIONS); do \ echo " Fetching K8s $$k8s_ver..."; \ - $(SETUP_ENVTEST) use --use-env --bin-dir $(ENVTEST_OCP_ASSETS_DIR) -p path --index $(ENVTEST_OCP_INDEX) $$k8s_ver > /dev/null; \ + $(SETUP_ENVTEST) use --use-env --bin-dir $(ENVTEST_OCP_ASSETS_DIR) -p path --index $(ENVTEST_OCP_INDEX) $$k8s_ver > /dev/null || { echo "Failed to fetch envtest assets for $$k8s_ver"; exit 1; }; \ done @@ `@for` k8s_ver in $(ENVTEST_KUBE_VERSIONS); do \ echo " Fetching K8s $$k8s_ver..."; \ - $(SETUP_ENVTEST) use --use-env --bin-dir $(ENVTEST_KUBE_ASSETS_DIR) -p path $$k8s_ver > /dev/null; \ + $(SETUP_ENVTEST) use --use-env --bin-dir $(ENVTEST_KUBE_ASSETS_DIR) -p path $$k8s_ver > /dev/null || { echo "Failed to fetch envtest assets for $$k8s_ver"; exit 1; }; \ doneAlso applies to: 425-428
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 394 - 397, The envtest prefetch loops ignore the exit status of the setup-envtest command, allowing failures to be swallowed; update the loops that call $(SETUP_ENVTEST) (the blocks iterating over ENVTEST_OCP_K8S_VERSIONS and the similar loop at lines 425-428) to detect failures and fail fast—e.g., run $(SETUP_ENVTEST) ... $$k8s_ver and if it returns non-zero then print an error referencing ENVTEST_OCP_ASSETS_DIR and ENVTEST_OCP_INDEX and exit with a non-zero status (use shell conditional or "|| exit 1") so any fetch failure aborts immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Makefile`:
- Around line 394-397: The envtest prefetch loops ignore the exit status of the
setup-envtest command, allowing failures to be swallowed; update the loops that
call $(SETUP_ENVTEST) (the blocks iterating over ENVTEST_OCP_K8S_VERSIONS and
the similar loop at lines 425-428) to detect failures and fail fast—e.g., run
$(SETUP_ENVTEST) ... $$k8s_ver and if it returns non-zero then print an error
referencing ENVTEST_OCP_ASSETS_DIR and ENVTEST_OCP_INDEX and exit with a
non-zero status (use shell conditional or "|| exit 1") so any fetch failure
aborts immediately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f7bbc945-07bb-4b32-8c36-0900cc53b059
📒 Files selected for processing (2)
Makefiletest/envtest/README.md
✅ Files skipped from review due to trivial changes (1)
- test/envtest/README.md
Allow running envtest API validation suites in parallel across Kubernetes versions via ENVTEST_JOBS (0=sequential, N=parallel jobs, MAX=all versions at once). Each version gets its own isolated envtest environment so there are no conflicts between runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
jparrill
force-pushed
the
parallel-env-test
branch
from
80bb3fe to
d8ab273
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8243 +/- ##
=======================================
Coverage 34.65% 34.65%
=======================================
Files 767 767
Lines 93263 93263
=======================================
Hits 32318 32318
Misses 58266 58266
Partials 2679 2679 🚀 New features to boost your workflow:
|
|
/lgtm |
|
Scheduling tests matching the |
Test Resultse2e-aws
e2e-aks
|
|
/verified by envtests |
openshift-ci-robot
added
the
verified
|
@bryan-cox: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@jparrill: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
5 participants
Summary
ENVTEST_JOBSvariable to control parallel execution of envtest API validation suitesENVTEST_JOBS=0(default): sequential execution, backward compatibleENVTEST_JOBS=N: run up to N versions in parallelENVTEST_JOBS=MAX: run all versions in parallel (auto-detects count from version list)Usage
Test plan
make test-envtest-ocp(no ENVTEST_JOBS) runs sequentially as beforemake test-envtest-ocp ENVTEST_JOBS=3runs with 3 parallel jobsmake test-envtest-ocp ENVTEST_JOBS=MAXruns all 6 OCP versions in parallelmake test-envtest-kube ENVTEST_JOBS=MAXruns all 5 kube versions in parallel🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation