Tracking issue for follow-up work on #16787 (Prebuild CLI E2E Docker image)
- Dominant language
- C#
- Stars
- 6.3k
- Forks
- 991
- Avg merge
- 2d 15h
- Merged PRs (30d)
- 196
Description
Tracking issue for follow-up work raised during code review of #16787 (Prebuild CLI E2E Docker image). Each task below is intentionally out of scope for #16787 so the PR can land without scope creep.
---
## Task 1 — Consolidate `Verify Docker is running` step into a composite action
The step
```yaml
- name: Verify Docker is running
run: docker info
```
(or a near-identical variant) is currently duplicated across at least 5 workflows:
- `.github/workflows/build-cli-e2e-image.yml:36-37`
- `.github/workflows/run-tests.yml:116-119`
- `.github/workflows/reproduce-flaky-tests.yml:180-182`
- `.github/workflows/tests-daily-smoke.yml:55-56`
- `.github/workflows/deployment-tests.yml:247-251`
Some are unconditional, some are gated on `runner.os == 'Linux'`. The cumulative impact:
- Five places to update if/when the runtime check needs to grow (e.g., add `docker buildx version`, free-disk-space sanity check, or capture diagnostics on failure).
- No single canonical place to document what the runner is expected to provide.
- Easy to drift out of sync as new workflows are added.
### Proposal
Extract into a composite action at `.github/actions/verify-docker/action.yml`:
```yaml
- uses: ./.github/actions/verify-docker
with:
diagnostics: true # optional: emit `docker version`, `docker buildx version`, df -h /
```
Replace each existing `docker info` step with the action. The composite action would:
- Always run `docker info` (the existing behavior).
- Optionally emit additional diagnostics under a single flag, so we have one place to add them when CI starts misbehaving.
- Be the documented contract for "Linux runner Docker setup expectations" (referenced from `docs/ci/`).
### Acceptance
- [ ] `.github/actions/verify-docker/action.yml` exists
- [ ] All five occurrences listed above migrated to `uses:`
- [ ] Brief documentation in `docs/ci/` (or as a comment in the action itself) covering what the action checks and when to extend it
---
## Task 2 — Replace substring-based Java image gate with a structural opt-in
`.github/workflows/run-tests.yml:135-141, 151` decides whether to download and require the prebuilt Java image based on a literal substring of the test job's short name:
```yaml
- name: Download prebuilt CLI E2E Java Docker image
if: ${{ ... && contains(inputs.testShortName, 'Java') }}
- name: Load prebuilt CLI E2E Docker images
run: |
eng/scripts/load-cli-e2e-images.sh \
--require-java "${{ contains(inputs.testShortName, 'Java') }}"
```
Today this works because every test class that uses `DockerfileVariant.PolyglotJava` happens to start with `Java` (`JavaEmptyAppHostTemplateTests`, `JavaPolyglotTests`, `JavaCodegenValidationTests`). The coupling is enforced only by reviewer vigilance.
### Failure mode
A future test class that uses `DockerfileVariant.PolyglotJava` but is named without the literal `Java` substring (e.g. `MavenIntegrationTests`, `KotlinPolyglotTests`, `JdkBundlingTests`) will:
1. Skip the artifact download.
2. Receive `--require-java false`.
3. The C# helper sees `ASPIRE_E2E_REQUIRE_POLYGLOT_JAVA_IMAGE` unset and silently falls back to building from `Dockerfile.e2e-polyglot-java` source on the test runner — exactly the slow/flaky path the prebuild was introduced to avoid, and the strict-mode flag was specifically introduced to make this kind of regression fail loudly.
This is a different concern from the asymmetry resolved in [`#16787 (review) r3192698415`](https://github.com/microsoft/aspire/pull/16787#discussion_r3192698415); that thread fixed "given `--require-java=true`, fail loudly when the tarball is missing." The remaining gap is one layer up: the value passed to `--require-java` is itself substring-derived.
### Proposal
Drive the Java-image gate from a property the test project owns rather than from the class-name substring. Two reasonable options:
- Add a per-test-class entry to the matrix that flags `requiresPolyglotJavaImage`, and gate the download/require step on that flag instead of `contains(testShortName, 'Java')`.
- Or, given the modest size delta, always download the Java tarball for any `Cli.EndToEnd*` short name and let the helper decide whether to use it.
### Acceptance
- [ ] Java image download/require gate no longer depends on a substring of the test class name
- [ ] A new test class using `DockerfileVariant.PolyglotJava` whose name does not contain `Java` is correctly served the prebuilt image (no fallback to source Dockerfile build)
---
## Task 3 — Java image build symmetry (`docker buildx build --load`)
In `.github/workflows/build-cli-e2e-image.yml:84-100`, `build_java_image` uses `DOCKER_BUILDKIT=1 docker build` while the other two builds (`build_image`) use `docker buildx build --load`. Today this works on GHA Linux runners because plain Docker Engine has no `builder→buildx` alias, so `docker build` bypasses the `--use`'d `cli-e2e-builder` and goes through the daemon's integrated BuildKit (build log shows `#0 building with "default" instance using docker driver` — [job 74633566536, line 1113](https://github.com/microsoft/aspire/actions/runs/25441231885/job/74633566536)). On Docker Desktop, where the alias is enabled by default, the same script would route through the docker-container builder, miss `--load`, and fail at the subsequent `docker save` with "No such image". Cheap to switch the Java build to the shared `build_image` helper for symmetry; not blocking.
### Acceptance
- [ ] All three image builds in `build-cli-e2e-image.yml` go through the same `docker buildx build --load` path
---
Contributor guide
Assessment
This issue has not been assessed yet.