Development: Pre-pull the e2e exercise images before the tests run - #13495
Conversation
The E2E stack mounts the runner's Docker socket, so exercise builds run
against the host daemon — but nothing provisions the exercise images.
The first Java build of a run therefore discovers that the ~1 GB Maven
image is missing and pulls it while the suite is already competing for
CPU, disk and network.
On a recent PR run every pull of ls1tum/artemis-maven-template died
mid-extraction ("DockerClientException: Could not pull image:
Extracting"), 261 build jobs failed over the first 25 minutes, and each
test asserting on a Java build result failed with "0%, Build failed" —
which reads as a grading regression and says nothing about the cause.
Once the image finally landed, later Java builds passed in ~10s. Because
test order is stable, the same early-scheduled tests failed on every
pull request: both ProgrammingExercisePracticeMode submission tests and
the static-code-analysis feedback test. They pass locally, where the
images are already on the machine.
Pulling the images in the setup action instead moves that one-off cost
in front of the suite, where it competes with nothing and can be
retried, and turns an unobtainable image into one loud setup failure
rather than a handful of unexplained assertion failures. Images already
present are left alone, so on a warm self-hosted runner this is a no-op.
The image list is guarded against drift: each tag must still appear in
application.yml, so bumping a tag there without updating the script
fails with an explicit message instead of silently reintroducing the
on-demand pull.
Note that this does not explain why the host's pulls die mid-extraction
(disk pressure, concurrent jobs and registry flakiness are all
candidates and need host-level access to tell apart). If the host cannot
pull at all, the job now fails fast at setup with the real reason.
GitHub Actions
Inactive
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ci/E2E-tests/prepull-exercise-images.sh:
- Around line 41-43: Update the image validation loop in
prepull-exercise-images.sh to match only active
continuous-integration.build.images properties in CONFIG_FILE, rather than
grepping the entire YAML text. Handle the YAML value forms used by the
configuration, including quoted and unquoted tags, while preserving the existing
stale-image error behavior.
- Around line 55-65: Update the docker pull invocation in the attempt loop to
run under a supported per-command timeout, ensuring stalled pulls terminate with
a nonzero status so the existing backoff and MAX_ATTEMPTS logic proceeds
unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 57b2d40a-9e48-4f0b-a904-ced3dd712506
⛔ Files ignored due to path filters (1)
.github/actions/e2e-setup/action.ymlis excluded by!**/*.yml
📒 Files selected for processing (1)
.ci/E2E-tests/prepull-exercise-images.sh
github-project-automation
Bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End Test Results
Test Strategy: Two-phase execution
❌ Failed Tests (Phase 2)
Overall: ❌ E2E: real (non-flaky) test failure 🔗 Workflow Run · 📊 Test Report Phase 1 · 📊 Test Report Phase 2 |
GitHub Actions
Inactive
Both points from the review, and the first one mattered: the premise of this script is that these pulls stall, so an unbounded `docker pull` in the setup step would hang the job until its timeout — the same undiagnosable failure the script exists to remove, only earlier. The pull now runs under a `timeout` capped at 10 minutes, which is generous for a cold ~1 GB image but turns a stall into a non-zero exit the existing backoff can act on. `--kill-after` escalates to SIGKILL for a pull wedged badly enough to ignore the SIGTERM. macOS names the coreutils binary `gtimeout`, so both are accepted and the bound also holds for local runs. The drift check no longer greps the whole configuration. It reads the image values out of the build agent's own `images:` block, which the first version got wrong in a way worth noting: application.yml holds a second, unrelated `images:` block for Kubernetes app definitions, and matching the raw file text meant an image named there could have satisfied the check. Values are accepted quoted or bare, trailing comments are stripped, and a layout change that yields no images now fails with its own message instead of silently passing.
|
Both review points addressed in 336d87d. Bounded pull — this was the one that mattered. The premise of the script is that these pulls stall, so an unbounded Scoped drift check — no longer greps the whole file. It now reads the image values out of the build agent's own Verified locally, five paths:
Still not ready to merge, for a reason unrelated to the review: Phase 1 selected only |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pull request adds a Bash script for E2E exercise-image preparation. It validates configured images, skips local images, and pulls missing images with timeouts, retries, backoff, and termination handling. ChangesE2E exercise image preparation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant E2E setup script
participant application.yml
participant Docker
E2E setup script->>application.yml: Read and validate exercise images
E2E setup script->>Docker: Check local image availability
E2E setup script->>Docker: Pull missing images with timeout and retries
Docker-->>E2E setup script: Return provisioning status
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.ci/E2E-tests/prepull-exercise-images.sh (3)
35-35: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winResolve
CONFIG_FILErelative to the repository, not the caller's directory.
CONFIG_FILEis a relative path, so the script only works when the caller's working directory is the repository root. If a developer runs it from another directory, awk fails withcan't open file, andset -eaborts before any of the curated::errormessages can print. The header states the script is also used locally, so this path is reachable.♻️ Proposed fix to anchor the path
-CONFIG_FILE="src/main/resources/config/application.yml" +REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/../.." && pwd)" +CONFIG_FILE="${REPO_ROOT}/src/main/resources/config/application.yml" +if [ ! -r "$CONFIG_FILE" ]; then + echo "::error title=Could not read E2E exercise images::${CONFIG_FILE} is missing or unreadable." + exit 1 +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ci/E2E-tests/prepull-exercise-images.sh at line 35, Update CONFIG_FILE in prepull-exercise-images.sh to resolve from the repository or script location rather than the caller’s current working directory, while preserving the existing application.yml target so the script works when invoked locally from any directory.
104-135: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the total time spent on one image, not only each attempt.
timeoutterminates thedocker pullclient process. The Docker daemon continues the pull in the background, so the next attempt attaches to the same in-progress pull. If the daemon or the registry is genuinely stalled, each attempt consumes the fullPULL_TIMEOUT_SECONDS. The worst case for a single image is 3 × 600s plus 30s of backoff, which is about 30 minutes before the step reports the failure. Four images give a worst case near two hours, and the surrounding job timeout would cut the step before the curated::errormessage on Line 138 prints.Add a deadline for the whole per-image loop, or reduce
PULL_TIMEOUT_SECONDSso thatMAX_ATTEMPTS× the per-attempt bound stays inside the step budget.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ci/E2E-tests/prepull-exercise-images.sh around lines 104 - 135, Bound the total duration of the per-image retry loop, not just each docker pull attempt. Update the loop around the pull array and MAX_ATTEMPTS to enforce a shared deadline that includes retries and backoff, ensuring failure reaches the existing curated error path before the job timeout; preserve the current retry behavior within that deadline.
78-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse a Bash 4+ requirement or replace
mapfile.macOS’s default Bash 3.2 does not provide
mapfile. With#!/usr/bin/env bash, this line fails unless a newer Bash appears first inPATH. Replace it with awhile IFS= read -rloop, or document and enforce the Bash 4+ requirement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ci/E2E-tests/prepull-exercise-images.sh at line 78, Update the configured_images output handling around CONFIGURED to avoid Bash 4-only mapfile usage: replace it with a Bash 3.2-compatible while IFS= read -r loop that preserves each image entry, or explicitly enforce a Bash 4+ interpreter requirement for the script.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ci/E2E-tests/prepull-exercise-images.sh:
- Around line 43-73: Update configured_images so build_indent is cleared when
the build: mapping ends, preventing later nested images: blocks from being
selected. Require the images: mapping to use exactly one indentation level
deeper than build_indent, while preserving parsing of the intended build-agent
block.
---
Nitpick comments:
In @.ci/E2E-tests/prepull-exercise-images.sh:
- Line 35: Update CONFIG_FILE in prepull-exercise-images.sh to resolve from the
repository or script location rather than the caller’s current working
directory, while preserving the existing application.yml target so the script
works when invoked locally from any directory.
- Around line 104-135: Bound the total duration of the per-image retry loop, not
just each docker pull attempt. Update the loop around the pull array and
MAX_ATTEMPTS to enforce a shared deadline that includes retries and backoff,
ensuring failure reaches the existing curated error path before the job timeout;
preserve the current retry behavior within that deadline.
- Line 78: Update the configured_images output handling around CONFIGURED to
avoid Bash 4-only mapfile usage: replace it with a Bash 3.2-compatible while
IFS= read -r loop that preserves each image entry, or explicitly enforce a Bash
4+ interpreter requirement for the script.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cefe2ac7-4472-49bd-b90b-d5d45dd8122e
⛔ Files ignored due to path filters (1)
.github/actions/e2e-setup/action.ymlis excluded by!**/*.yml
📒 Files selected for processing (1)
.ci/E2E-tests/prepull-exercise-images.sh
GitHub Actions
Inactive
GitHub Actions
Inactive
Three more points from the review. The one that mattered: `timeout` only kills the `docker pull` client, the daemon keeps pulling, and the next attempt attaches to that same pull — so against a genuinely stalled daemon every attempt burned its full cap. Worst case was 3 x 600s per image, near two hours across four, and the job timeout would have killed the step before it could report why. That is the failure this script exists to remove, so the retries now share a per-image deadline and give up inside it. The per-attempt cap also drops from 600s to 300s, informed by the run this change was written for: pulling all four images took 30 seconds and the Maven image 11 of them, so 300s is still a large multiple of the real cost. CONFIG_FILE is now resolved from the script's own location instead of the caller's working directory, with an explicit readability check, so running it from anywhere behaves the same. The configured-image list is collected with `while read` rather than `mapfile`, which macOS's default Bash 3.2 does not provide — the header claims local usability, so it should hold there too.
The fix is confirmed workingPhase 2 has now run the tests this PR exists to fix. Direct evidence from the Phase 2 job:
Phase 2: 342 ran, 333 passed, 7 skipped, 2 failed. Note the pull that repeatedly died mid-extraction for 25 minutes under test load takes 30 seconds when done alone — which is the contention argument made concrete. The 2 remaining failures are unrelated
Both are fast failures (44s, 55s), not the multi-minute build timeouts this PR addresses. Review round 2 addressed in 31f1a08All three were valid, and the retry-loop one was material:
Verified: runs correctly from another directory, runs correctly under |
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ci/E2E-tests/prepull-exercise-images.sh:
- Around line 142-147: Update the retry loop around the shared deadline and
pull/backoff logic in the prepull script so each attempt’s timeout, including
the 30-second kill-after grace period, cannot exceed the seconds remaining
before deadline. Also cap each backoff sleep to the remaining deadline budget,
preventing either pulls or sleeps from running past the shared timeout.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e6df478-fc34-4c53-aeba-f39f27c091b7
📒 Files selected for processing (1)
.ci/E2E-tests/prepull-exercise-images.sh
GitHub Actions
Inactive
|
@Claudia-Anthropica review |
|
@krusche on it! Taking a look now 👀 |
…scoping The per-image deadline was only checked between attempts, so a single hanging pull could overrun it by a further PULL_TIMEOUT_SECONDS and the backoff could push past it again — the budget was advisory rather than a bound. Each attempt is now capped by whatever is left of the budget, and the backoff is clamped to it, so the time spent on one image cannot exceed PULL_DEADLINE_SECONDS however long an individual pull hangs. Verified with a stalled pull: a 5s budget with a 10s per-attempt cap finishes in 5s. The extractor's comment claimed the `images:` block had to be nested under `build:`, but `build_indent` was only cleared when an images block ended. A `build:` block without an images child therefore left it set, and an `images:` key in any later section would have been read. It is now cleared on leaving the build block, so the code enforces what the comment says. Covered both ways with synthetic configurations: a `build:` block with no images child followed by a deeply nested `images:` yields nothing, while a real `build.images` block is still read in full and an unrelated `k8s.images` block is ignored. The timeout binary is also detected once instead of per image, since it does not vary.
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@krusche [medium] The latest commit still gives timeout the full remaining budget before the 30-second kill-after grace, so the per-image deadline can be exceeded. [medium] The extractor also still accepts any deeper images: mapping under build: instead of requiring the direct child; both defects are already covered by unresolved CodeRabbit threads, so I have not duplicated inline comments. The captured CI snapshot has no required checks pending or failing.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ci/E2E-tests/prepull-exercise-images.sh:
- Around line 125-132: Update the missing-image pull flow in
prepull-exercise-images.sh to check whether timeout_bin is empty before invoking
docker pull. Preserve the cached-image path, but exit with an appropriate
failure message instead of starting an unbounded pull when neither timeout nor
gtimeout is available.
- Around line 152-157: Update the timeout calculation near attempt_cap and the
pull command so the --kill-after grace period is reserved within the shared
deadline. Ensure each timeout invocation cannot run past deadline; when the
remaining budget cannot accommodate the grace period, use an immediate SIGKILL
timeout while preserving the existing docker pull retry behavior.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 90746e7b-1985-4f96-9cb6-48ed65b9dea6
📒 Files selected for processing (1)
.ci/E2E-tests/prepull-exercise-images.sh
GitHub Actions
Inactive
GitHub Actions
Inactive
krusche
changed the title
Development: Pre-pull the E2E exercise images before the tests runDevelopment: Pre-pull the e2e exercise images before the tests run
…he cap Two more review points, both about the bound not holding in cases the comments claimed it did. Without `timeout` or `gtimeout` the script previously fell through to an unbounded `docker pull`, which on a stall would hold the setup step open until the job timeout — the failure this script exists to remove. It now refuses, with a message naming the fix. The check sits where an image turns out to be missing rather than up front, so a machine with every image already cached still succeeds without coreutils. `timeout --kill-after=D T` sends SIGTERM at T and only escalates at T+D, so charging the 30s grace on top of the attempt cap let a pull that ignores SIGTERM run past the deadline. The grace now comes out of the attempt's own share: above 30s the cap is split into SIGTERM at cap-30 and SIGKILL at cap, and at or below 30s there is no room to be graceful, so it kills outright at the cap. Verified by inspecting the constructed command: a 100s cap yields `--kill-after=30s 70s`, a 20s cap yields `--signal=KILL 20s`.
Three review rounds went into a shared per-image deadline, a min() against the remaining budget, a clamped backoff and a SIGKILL grace period subtracted from the cap. All of it existed to bound the worst case, and a smaller per-attempt cap bounds it just as well without the bookkeeping. Two attempts of up to 120s replaces it. Measurement supports the number: the Maven image pulled in 11s on the runner and all four in 30s, so 120s is a wide margin, and 2 x 120s + 30s of kill grace puts a totally unreachable registry at roughly 8 minutes across four images before the step says why. That is well inside the job budget, which is the property the deadline machinery was there to guarantee. Kept: the drift guard, the refusal to start a pull that cannot be bounded, the retry, and the loud final error. Those carry their weight.
|
Trimmed the retry machinery back in 6d3c94a, per the point that this was drifting into unrealistic edge cases. Three review rounds had accumulated a shared per-image deadline, a The number comes from the run this PR was written for: the Maven image pulled in 11s, all four in 30s. So 120s is a wide margin, and 2 × 120s plus kill grace puts a totally unreachable registry at roughly 8 minutes across four images before the step reports why — comfortably inside the job budget, which is the only property the deadline arithmetic was guaranteeing. Kept, because they carry their weight:
Re-verified after the trim: all-cached path exits 0 under Bash 3.2 from an arbitrary directory; a missing image with no Also rebased onto the develop merge that landed on the branch. On mergingI would not merge this second. The E2E run that proved the fix was on After that the only real gap is human review: five commits of shell, so far read only by bots. |
The drift check entered the images block on any `images:` key indented deeper than `build:`, so a mapping nested further down - inside `default-docker-flags:`, say - was taken for the build agent's image list. It read that block instead of the real one: given both, the old extractor returned the nested entry and none of the configured tags, which would fail the drift check against a list that is actually correct. Take the indent of the first key below `build:` as the indent its direct children sit at, and accept `images:` only there.
|
Both findings are closed at 46cf55d. Per-image deadline could be exceeded. Resolved, though by a different route than suggested: 6d3c94a dropped the shared-budget arithmetic entirely in favour of a single per-attempt The extractor accepted any deeper build:
default-docker-flags:
images:
java:
default: "nested-too-deep:1"
images:
java:
default: "real-direct-child:2"the old extractor returned Fixed by taking the indent of the first key below |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.ci/E2E-tests/prepull-exercise-images.sh (1)
58-89: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winScope extraction to
continuous-integration.build.images.Line 60 accepts every
build:mapping. The function then emits values from every directbuild.imagesblock. An unrelated block can therefore supply a required tag and make drift validation pass when the build-agent block does not contain it.Track the
continuous-integration:parent and acceptbuild:only within that mapping. Based on learnings: scripts that validate build-agent Docker images must scope parsing tocontinuous-integration.build.images.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ci/E2E-tests/prepull-exercise-images.sh around lines 58 - 89, Update configured_images() so the awk parser tracks the continuous-integration: mapping and recognizes build: only when it is a direct child of that mapping. Keep extracting image values solely from continuous-integration.build.images, ignoring unrelated build/images blocks while preserving the existing indentation and comment handling.Source: Learnings
♻️ Duplicate comments (1)
.ci/E2E-tests/prepull-exercise-images.sh (1)
151-154: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep the grace period inside the 120-second cap.
--kill-after=30s 120ssendsSIGTERMafter 120 seconds and can wait 30 more seconds beforeSIGKILL. A pull that ignoresSIGTERMcan run for 150 seconds. This violates the stated 120-second attempt cap and reintroduces the previously resolved timeout-accounting issue.Reserve the grace period within
PULL_TIMEOUT_SECONDS. Use--signal=KILLwhen the cap cannot include the grace period.Proposed fix
+KILL_GRACE_SECONDS=30 + - if "$timeout_bin" --kill-after=30s "${PULL_TIMEOUT_SECONDS}s" docker pull --quiet "$image"; then + if [ "$PULL_TIMEOUT_SECONDS" -gt "$KILL_GRACE_SECONDS" ]; then + "$timeout_bin" --kill-after="${KILL_GRACE_SECONDS}s" \ + "$((PULL_TIMEOUT_SECONDS - KILL_GRACE_SECONDS))s" \ + docker pull --quiet "$image" + else + "$timeout_bin" --signal=KILL "${PULL_TIMEOUT_SECONDS}s" docker pull --quiet "$image" + fi + if [ "$?" -eq 0 ]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ci/E2E-tests/prepull-exercise-images.sh around lines 151 - 154, Update the timeout invocation in the image-pull retry flow around the docker pull command so each attempt never exceeds PULL_TIMEOUT_SECONDS, including any termination grace period. Reserve the grace interval within that cap, or use timeout’s --signal=KILL option when the configured cap cannot accommodate a grace period; do not retain --kill-after=30s with the full PULL_TIMEOUT_SECONDS value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.ci/E2E-tests/prepull-exercise-images.sh:
- Around line 58-89: Update configured_images() so the awk parser tracks the
continuous-integration: mapping and recognizes build: only when it is a direct
child of that mapping. Keep extracting image values solely from
continuous-integration.build.images, ignoring unrelated build/images blocks
while preserving the existing indentation and comment handling.
---
Duplicate comments:
In @.ci/E2E-tests/prepull-exercise-images.sh:
- Around line 151-154: Update the timeout invocation in the image-pull retry
flow around the docker pull command so each attempt never exceeds
PULL_TIMEOUT_SECONDS, including any termination grace period. Reserve the grace
interval within that cap, or use timeout’s --signal=KILL option when the
configured cap cannot accommodate a grace period; do not retain --kill-after=30s
with the full PULL_TIMEOUT_SECONDS value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f22575fc-5062-4f10-ab15-90558c014fab
📒 Files selected for processing (1)
.ci/E2E-tests/prepull-exercise-images.sh
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@krusche The current head resolves the previous blockers: image extraction is restricted to the direct build.images child, and missing images cannot enter an unbounded pull. The simplified retry loop is consistently bounded per attempt, and the captured CI snapshot contains no required check pending or failing.
GitHub Actions
Inactive
GitHub Actions
Inactive
github-project-automation
Bot
moved this from Ready For Review
to Merged
in Artemis Development
Summary
The E2E stack mounts the runner's Docker socket, so exercise builds run against the host daemon — but nothing provisions the exercise images. The first Java build of a run therefore discovers that the ~1 GB Maven image is missing and pulls it while the suite is already running, competing with the test workload. This adds a pre-pull step to the shared E2E setup action so no build job ever races an on-demand pull.
Checklist
General
Motivation and Context
A handful of E2E tests have been failing on every pull request, and the flakiness classifier scores them as real regressions, so they block the required gate. They are not regressions.
On run 31575061135 the app container log shows:
```
BuildJobManagementService : Error while executing build job 3151786522675528:
LocalCIException: Could not pull Docker image ls1tum/artemis-maven-template:java17-25
Caused by: ... BuildAgentDockerService.doPullDockerImage
Caused by: com.github.dockerjava.api.exception.DockerClientException:
Could not pull image: Extracting
```
docker-java's `PullImageResultCallback` throws with the last status it saw, so "Extracting" means the pull stream ended part-way through unpacking layers. In that run:
Because test order is stable, the same early-scheduled tests fail every time: both `ProgrammingExercisePracticeMode` submission tests and `Static code analysis tests › Verifies SCA feedback is displayed correctly after submission`. They fail on unrelated PRs too (#13493, #13492) and pass locally, where the images are already on the machine — I ran the practice-mode spec against a local stack and all 3 tests pass in 1.5 minutes with real Maven builds reaching 100%.
The symptom is misleading on its own: the assertion reports `Status 0%, Build failed`, which reads as a grading regression and says nothing about the missing image.
Ruled out along the way: Artemis's own pull timeout and stall detector never fire (zero `did not finish within` / `reported no progress` messages, so `image-pull-timeout-seconds` is not involved); the `java17-25` tag does exist on Docker Hub; and `.ci/E2E-tests/cleanup.sh` stops containers but does not remove images.
Description
This moves a cost that was already being paid in front of the suite, where it competes with nothing and can be retried, and turns an unobtainable image into one loud setup failure rather than a handful of unexplained assertion failures.
What this does not do: it does not explain why the host's pulls die mid-extraction — disk pressure, concurrent E2E jobs on shared self-hosted runners and registry flakiness are all candidates, and telling them apart needs host-level access. If the host cannot pull at all, the job now fails fast at setup with the real reason instead of producing 261 opaque build failures. Happy to downgrade that to a warning if you would rather a degraded run still executed.
Steps for Testing
This only affects CI, so the check is the E2E run on this PR itself:
Locally verified all four script paths: images already present (no-op), pull needed (pulled successfully), tag no longer in `application.yml` (fails with the stale-list error), and unpullable image (retries, then fails with the provisioning error). `shellcheck` and `bash -n` are clean.
Testserver States
Not applicable — this changes CI setup only and cannot be exercised on a test server.
Review Progress
Code Review
Summary by CodeRabbit