mirror of
https://github.com/gmeligio/flutter-docker-image.git
synced 2026-05-24 12:30:34 +00:00
docs: archive p3-parallelize-image-validation (#454)
Archive the `p3-parallelize-image-validation` change. Implementation
shipped in #453 (commit 87eb48f) on 2026-05-18.
- Promotes the `ci-parallel-image-validation` capability spec to
`openspec/specs/`.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1 @@
|
||||
passed
|
||||
+1
-1
@@ -24,7 +24,7 @@ The non-trivial questions are: (a) how do consumer jobs get the bits onto their
|
||||
|
||||
**Decision**: The two consumers use different strategies because the two actions support different things:
|
||||
|
||||
- `docker/scout-action`: pass `image: registry://ghcr.io/<owner>/flutter-android:pr-N`. The `registry://` prefix tells Scout to read from the registry directly with no daemon involvement — confirmed in the action's README image-prefix table. No `docker pull` step on the registry path for `scan_image`.
|
||||
- `docker/scout-action`: SHALL `docker pull` the GHCR image, re-tag it as `<owner>/flutter-android:<flutter_version>` (the Docker Hub repo path), and pass `image: local://<owner>/flutter-android:<flutter_version>`. The `registry://` prefix was the initial choice (no daemon involvement per the action README) but was rejected during implementation: Scout's `compare` command looks up the image's repo in its stream-environment records, and those records exist only for the Docker Hub repo path — passing a `registry://ghcr.io/...` ref fails with "not in stream environment:prod". The re-tag-and-`local://` trick is carried over from the previous serial implementation.
|
||||
- `container-structure-test` (via `plexsystems/container-structure-test-action`): the action invokes `container-structure-test test --image <input>` with no `--pull` flag and no driver override. The CLI's default `docker` driver inspects the local Docker daemon only. So `test_image` SHALL run an explicit `docker pull "$IMAGE_REF"` on the registry path before invoking the action. The earlier draft of this design assumed CST would stream-pull on demand; it does not.
|
||||
|
||||
**Alternatives considered**:
|
||||
+1
-1
@@ -12,7 +12,7 @@ The existing `build.yml:108` TODO explicitly notes this:
|
||||
|
||||
- **Rename** `test_image` job → `build_image`. Its responsibility is now: build the image, push the handoff (per p2), and stop. Remove the `Test image` and `Scan with Docker Scout` steps from this job.
|
||||
- **New job** `test_image` (`needs: build_image`): materializes the handoff into the local Docker daemon (registry-path: `docker pull <image_ref>`; artifact-path: `download-artifact` + `docker load`), runs container-structure-test against the loaded image.
|
||||
- **New job** `scan_image` (`needs: build_image`): runs `docker/scout-action` against the handoff. Registry-path uses `image: registry://<image_ref>` so no local pull is needed; artifact-path uses `image: local://<image_local_tag>` after `download-artifact` + `docker load`. Gated `if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository` (Scout needs the Docker Hub org secret + PR-comment write, neither available to fork PRs — matches the existing gate at `build.yml:155`).
|
||||
- **New job** `scan_image` (`needs: build_image`): runs `docker/scout-action` against the handoff. Registry-path `docker pull`s the GHCR image, re-tags it to the Docker Hub repo path (`<owner>/flutter-android:<flutter_version>`), and passes `image: local://<repo-path>` — Scout's `compare` requires the Docker Hub repo path to look up stream-environment records (`registry://ghcr.io/...` fails with "not in stream environment:prod"). Artifact-path uses `image: local://<image_local_tag>` after `download-artifact` + `docker load`. Gated `if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository` (Scout needs the Docker Hub org secret + PR-comment write, neither available to fork PRs — matches the existing gate at `build.yml:155`).
|
||||
- Both consumer jobs run on `ubuntu-24.04` with a thin checkout + login + pull-or-load + the existing validation step. No `setup-buildx` (neither consumer builds; buildx adds ~10 s per job for no benefit). No `clean-runner-disk` on the registry path (a 5 GB pull fits in the runner's ~14 GB free); the artifact path runs `clean-runner-disk` first because the 2 GB tarball + 5 GB extracted image is tight.
|
||||
- Pin `docker/scout-action` to **v1.20.4** (current as of Apr 2026; today's pin is v1.18.2) while the surrounding job is being rewritten — incidental cleanup, not the goal of this change.
|
||||
- `build_image` keeps `clean-runner-disk` (the build still needs the headroom).
|
||||
+5
-5
@@ -34,11 +34,11 @@
|
||||
|
||||
## 5. Verify on a real PR before merge
|
||||
|
||||
- [ ] 5.1 PR-A (non-fork): confirm `build_image`, `test_image`, `scan_image` all run, the two consumers start within ~10 s of `build_image` completing, and overall wall-clock is ≤ 15 min (target: ~12-13 min once p1 is in).
|
||||
- [ ] 5.2 PR-A (fork): confirm the artifact path works end-to-end: `build_image` uploads `image-<run_id>`, `test_image` and `scan_image` (scan only if scan would run for forks — it does not today) download and `docker load` and validate.
|
||||
- [ ] 5.3 Confirm that if `test_image` fails but `scan_image` passes (or vice versa), the PR shows the partial failure correctly and re-run-failed reruns only the failed job.
|
||||
- [x] 5.1 PR-A (non-fork): confirm `build_image`, `test_image`, `scan_image` all run, the two consumers start within ~10 s of `build_image` completing, and overall wall-clock is ≤ 15 min (target: ~12-13 min once p1 is in).
|
||||
- [x] 5.2 PR-A (fork): confirm the artifact path works end-to-end: `build_image` uploads `image-<run_id>`, `test_image` and `scan_image` (scan only if scan would run for forks — it does not today) download and `docker load` and validate.
|
||||
- [x] 5.3 Confirm that if `test_image` fails but `scan_image` passes (or vice versa), the PR shows the partial failure correctly and re-run-failed reruns only the failed job.
|
||||
|
||||
## 6. Post-merge closure check
|
||||
|
||||
- [ ] 6.1 After 10 post-merge runs, query the median wall-clock of the longest job in `build.yml` and confirm it is ≤ 15 min (down from ~20 min). If above target, investigate which step regressed (likely the `docker pull` on consumers — preferable to fix the cache rather than re-merge).
|
||||
- [ ] 6.2 Sweep open PRs after merge: any in-flight PR built on the old job layout will show the old `test_image` check as missing on rebase. Document the rebase recipe in the merge commit message.
|
||||
- [x] 6.1 After 10 post-merge runs, query the median wall-clock of the longest job in `build.yml` and confirm it is ≤ 15 min (down from ~20 min). If above target, investigate which step regressed (likely the `docker pull` on consumers — preferable to fix the cache rather than re-merge).
|
||||
- [x] 6.2 Sweep open PRs after merge: any in-flight PR built on the old job layout will show the old `test_image` check as missing on rebase. Document the rebase recipe in the merge commit message.
|
||||
@@ -0,0 +1,75 @@
|
||||
## Requirements
|
||||
|
||||
### Requirement: Test and scan run as sibling jobs, not as serial steps
|
||||
|
||||
Container-structure-test and Docker Scout SHALL run as two separate GitHub Actions jobs (`test_image` and `scan_image`), each with `needs: build_image`, in the same workflow. They SHALL NOT run as sequential steps inside the same job. The two jobs SHALL be schedulable in parallel by the runner — no `needs` relationship between them, no shared sentinel files, no implicit ordering.
|
||||
|
||||
The experience context is the maintainer watching the PR check page — they should see two checks start within seconds of `build_image` turning green, run in parallel, and the slower one (Scout) define the wall-clock rather than the sum.
|
||||
|
||||
#### Scenario: Test job runs in parallel with scan job
|
||||
|
||||
- **GIVEN** a PR run where `build_image` has just completed successfully
|
||||
- **WHEN** the runner picks up the downstream jobs
|
||||
- **THEN** `test_image` and `scan_image` both start within 30 seconds of `build_image` completion
|
||||
- **AND** their start times are within 30 seconds of each other (no implicit serialization)
|
||||
|
||||
#### Scenario: Scout scan runs in parallel with CST
|
||||
|
||||
- **GIVEN** a PR run with both consumer jobs running
|
||||
- **WHEN** the overall wall-clock is measured
|
||||
- **THEN** total time from `build_image` start to last consumer complete is approximately `build_image_duration + max(test_image_duration, scan_image_duration)` (not the sum)
|
||||
|
||||
### Requirement: Consumer jobs do not rebuild the image
|
||||
|
||||
`test_image` and `scan_image` SHALL consume the image via the handoff produced by `build_image` (per the `ci-image-handoff` capability). Neither job SHALL invoke `docker build`, `docker/build-push-action`, or any other Dockerfile build action. They SHALL only `pull` (registry path) or `download-artifact` + `docker load` (fork-PR path).
|
||||
|
||||
The experience context is the maintainer auditing CI cost — they expect each Dockerfile-touch PR to materialize the image bits exactly once, not three times.
|
||||
|
||||
#### Scenario: Consumer pulls registry image without rebuilding
|
||||
|
||||
- **GIVEN** a non-fork PR run
|
||||
- **WHEN** `test_image` or `scan_image` runs
|
||||
- **THEN** the job log shows a pull (or streaming pull by the action) of `ghcr.io/<owner>/flutter-android:pr-<N>`
|
||||
- **AND** the job log does not contain `docker build` or `FROM debian:`
|
||||
|
||||
#### Scenario: Fork PR consumer loads the artifact without rebuilding
|
||||
|
||||
- **GIVEN** a fork PR run
|
||||
- **WHEN** `test_image` or `scan_image` runs
|
||||
- **THEN** the job downloads `image-<run_id>`, runs `docker load`, and proceeds
|
||||
- **AND** the job log does not contain `docker build`
|
||||
|
||||
### Requirement: Scan job preserves the existing fork-PR gate
|
||||
|
||||
`scan_image` SHALL preserve the gate that today restricts `docker/scout-action` to non-fork PRs (Scout requires the Docker Hub org secret and writes a PR comment, neither available to fork PRs). The gate SHALL be applied at the job level (`if:` on the job), not as a per-step skip — fork-PR scan jobs SHALL be entirely skipped, not show as a no-op success step.
|
||||
|
||||
The experience context is the community contributor opening a fork PR — they see `build_image` and `test_image` run, and `scan_image` simply not appear (skipped), rather than appearing as a confusingly-empty job.
|
||||
|
||||
#### Scenario: Fork PR skips scan_image entirely
|
||||
|
||||
- **GIVEN** a `pull_request` event with `github.event.pull_request.head.repo.full_name != github.repository`
|
||||
- **WHEN** the workflow runs
|
||||
- **THEN** `scan_image` does not appear in the run's job list (job-level `if:` evaluates to false)
|
||||
- **AND** the run still completes successfully when `build_image` and `test_image` succeed
|
||||
|
||||
### Requirement: Renamed consumer preserves the existing required-check name
|
||||
|
||||
The CST consumer job's key SHALL be `test_image` — the same name as today's monolithic job — so the GitHub Actions check name `test_image` continues to appear and continues to satisfy branch-protection rules that require it. The previously-monolithic job SHALL be the one that is renamed to `build_image`, not the other way around.
|
||||
|
||||
The experience context is the maintainer merging this change — they do not need privileged admin access to update branch-protection rules to merge this PR, because the `test_image` check name is unchanged. Admin work (adding `build_image` and `scan_image` to required checks) happens as a follow-up after the new layout has proven stable.
|
||||
|
||||
#### Scenario: Renamed consumer satisfies the existing required check
|
||||
|
||||
- **GIVEN** branch-protection requires the check name `test_image`
|
||||
- **WHEN** the workflow runs after this change merges
|
||||
- **THEN** a check named `test_image` is reported (produced by the CST consumer job)
|
||||
- **AND** when `build_image` succeeds and the consumer succeeds, the `test_image` check is green
|
||||
- **AND** the PR can merge without admin intervention
|
||||
|
||||
#### Scenario: build_image failure correctly blocks merges
|
||||
|
||||
- **GIVEN** `build_image` fails
|
||||
- **WHEN** `test_image` runs
|
||||
- **THEN** `test_image` is skipped (its `needs:` failed)
|
||||
- **AND** the required check `test_image` is not reported
|
||||
- **AND** branch-protection treats the check as not-satisfied, blocking the merge
|
||||
Reference in New Issue
Block a user