ci(release): reuse verified pipeline for IDC images - #722
Conversation
37d7c07 to
981babc
Compare
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep review of 51d58a7.
Requesting changes for the publication error-handling regression below. The native build -> digest smoke -> manifest assembly dependency chain is sensible, and separating candidates/cache from the runtime repository is a useful improvement. The final publication step, however, no longer preserves the existing fail-closed immutable-tag contract.
I executed the workflow's actual "Publish or verify immutable IDC image" Bash block with a stateful fake crane, without contacting a registry. With a target already pointing to another digest:
- A successful target lookup correctly exits 1 without copying.
- A TLS handshake timeout, HTTP 403, or HTTP 503 on that lookup causes crane copy to run, replaces the existing target in the fixture, and exits 0.
The equality check after copying validates the newly overwritten value, so it cannot detect that immutability has already been violated. Please restore the distinction between confirmed absence and an unreadable target before publication.
Validation:
- All 15 tests in scripts/ci/test_release_build_shells.py passed.
- test_release_manifest_contract.sh and test_release_candidate_tags.sh passed.
- git diff --check passed.
- GitHub Test Suite, Static Checks, and PR Title are green for this head.
- The broader local repository validator failed in the unchanged scripts/dev/test_edge_process_contract.sh; I am not attributing that failure to this PR.
- Verification here was offline; I did not independently repeat the reported live IDC build/smoke/publication run.
The new topology test checks string presence and therefore misses this executable failure path. A regression should run the real publisher with target states absent, identical, conflicting, and unreadable; only confirmed absence may invoke copy, and errors must preserve the prior tag.
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep re-review of df63766.
The revision is materially better: publication now has an executable owner, and the added behavioral test correctly covers the previously reported plain TLS/401/403/503 failures, an absent target, and matching/conflicting digests. Those earlier examples should not be treated as unchanged defects.
However, the immutable-tag guarantee is still bypassable through a different failure path: the new classifier treats a 404 from the authentication service as proof that the image tag is absent. I reproduced an actual overwrite with the checksum-verified crane v0.20.6 binary used by the workflow and two isolated local HTTP registry fixtures. No fake crane was involved in this final reproduction, and no external registry was modified.
Reproduction:
- The source serves verified manifest A; the target already serves a different manifest B under the release tag.
- During the initial target digest lookup, /v2/ returns a Bearer challenge and the token endpoint returns 404 for both crane's HEAD attempt and GET fallback. The terminal error identifies GET /token?...: unexpected status code 404 Not Found.
- The token-service fault clears before copy. The script interprets the earlier authentication error as absence and invokes crane copy.
- The target records PUT /v2/team/astra/manifests/release, changes from B to A, and the publisher exits 0. Its post-copy digest check passes after the overwrite.
This remains a blocking correctness issue; please distinguish an explicit missing target manifest from failure to authenticate or reach the registry. The inline comment identifies the unsafe classification and the regression needed.
I also reviewed the complete seven-file PR diff, source/controller selection, credential boundary, native platform matrix, digest artifact flow, smoke-to-stage-to-publish dependencies, and the differences from the existing release candidate workflow. I found no additional independently substantiated blocker in those areas.
Validation on this head:
- python3 scripts/ci/test_release_build_shells.py: all 16 tests passed.
- bash scripts/ci/test_release_manifest_contract.sh: passed.
- bash scripts/ci/test_release_candidate_tags.sh: passed.
- git diff --check against the PR base: passed.
- Actual publisher plus real crane/local registry failure-recovery reproduction: demonstrated the overwrite described above.
I did not rerun the full repository gate or the live IDC build/smoke/publication pipeline. The reported live AMD64 success does not establish this failure-path property.
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep re-review of d6e7ecc.
The previous P1 error-classification issue is fixed in the tested cases. Requiring successful tag enumeration removes the unsafe interpretation of token-service diagnostics. With the checksum-verified crane v0.20.6 binary and the actual publisher, I verified that token-service 404, tag-list 401/403/503, an existing target's digest-read failure, a conflicting tag on a later pagination page, and a failed later page all stop without a manifest PUT or modification of the existing target. Missing tags in an existing repository publish correctly, and identical targets remain idempotent.
Requesting changes for a separate P2 first-publication regression introduced by that prerequisite: a new runtime repository can legitimately return 404/NAME_UNKNOWN from its tags endpoint until its first image is pushed. The workflow never initializes that repository: all preceding build/stage writes go to the separate -candidates repository. Consequently, configuring a fresh CONTAINER_MIRROR_IMAGE can complete build/smoke/staging but cannot publish its first image.
Independent reproduction used the real crane binary against isolated local HTTP registry fixtures. For a fresh target, /v2/ succeeded and /v2/team/astra/tags/list returned the Distribution API's 404 NAME_UNKNOWN response. The actual publisher exited 1 with 'could not safely enumerate tags' and performed no PUT. A control invocation of crane copy against the same source and fresh target succeeded and created the verified manifest. No external registry was modified. This missing-repository case is distinct from the existing test's 'missing' state, which always returns a successful list containing other-tag.
Please provide a safe, explicit bootstrap path for a genuinely absent runtime repository without restoring generic stderr/404 matching. The previous P1 protection should remain intact. The inline comment describes the regression and coverage needed.
Scope: rechecked the full seven-file diff and surrounding controller/source selection, credentials, native platform builds, digest artifacts, smoke/stage/publication dependencies, and configuration documentation; no additional independently substantiated blocker found.
Validation on this head:
- All 16 release-shell tests passed.
- Release manifest and candidate-tag contract scripts passed.
- git diff --check against the PR base passed.
- Real crane/local registry cases verified missing/equal/conflicting targets, read failures, pagination, token-service failure, and the fresh-repository failure described above.
- The broader repository validator failed in unchanged scripts/dev/test_edge_process_contract.sh; I am not attributing that failure to this PR.
- The combined-status API returned no entries, so I cannot claim all CI checks are green. I did not rerun the live IDC build/smoke flow.
| target_exists=false | ||
| if ! crane ls "${target_repository}" > "${target_tags}"; then | ||
| echo "could not safely enumerate tags in ${target_repository}" >&2 | ||
| exit 1 |
There was a problem hiding this comment.
[P2] Preserve a safe first-publication path for a new runtime repository
Requiring a successful tag list makes the first push impossible on a registry that returns 404/NAME_UNKNOWN for a repository that has never been populated. That is a documented Distribution tags API response, not necessarily a configuration or authentication error. In this workflow, candidate builds and manifest assembly write only to ${IDC_IMAGE}-candidates, so they do not create ${IDC_IMAGE} before this check. A fresh CONTAINER_MIRROR_IMAGE therefore reaches the final job and fails on every retry.
I reproduced this with the actual script and real crane v0.20.6 against a local HTTP fixture: the fresh repository's tags endpoint returns structured NAME_UNKNOWN, this script exits 1 before any PUT, while a control crane copy to the same fresh repository succeeds. The current 'missing' test cannot detect the regression because its fake ls always succeeds with other-tag.
Add a safe bootstrap path for a positively identified missing repository (or an explicit supported provisioning step), while continuing to reject token-service, transport, authorization, and ambiguous failures. Add a publisher test for a never-pushed runtime repository, separately from a missing tag in an existing repository. Do not reintroduce generic 404/not-found diagnostic matching.
Summary
Fix the manual IDC image workflow after its self-hosted BuildKit failed while pulling Dockerfile base images. Reuse the current release pipeline's native per-platform build, digest-based smoke verification, manifest assembly, and immutable publication flow, while publishing only to IDC.
The workflow supports Linux AMD64 (default) or the existing AMD64+ARM64 matrix. Final publication now requires a successful enumeration of the target repository and exact tag matching; only a successfully enumerated repository that lacks the target tag may be copied to.
Failed run being fixed: https://github.com/matrixorigin/Astra/actions/runs/34051357192
Related issue
N/A
Change type
User and compatibility impact
build_push_to_idckeeps its existing manual entrypoint and source selection. It now offersamd64andall, uses native GitHub-hosted platform runners, and writes only to IDC. Docker Hub releases and their topology are unchanged. Existing IDC variables and Environment credentials remain valid; the self-hosted IDC runner variable is no longer used. If the runtime repository cannot be enumerated because of TLS, authentication, authorization, token-service, or registry failure, publication stops without copying or overwriting a tag.Architecture and complexity delta
.github/workflows/build_push_to_idc.ymlremains the IDC publication controller;.github/workflows/idc-container-candidates.ymlis the IDC destination copy of the established release-candidate workflow;scripts/copy-immutable-container-tag.showns final cross-repository immutable-tag reconciliation.release.yml,release-container-candidates.yml, manifest reconciliation scripts, the merged IDC workflow, the failed IDC run, repository validation, and release documentation.Verification
python3 scripts/ci/test_release_build_shells.pypassed (16 tests);python3 scripts/ci/validate_repository.pypassed with Python 3.12;git diff --checkand Bash syntax validation passed. The preceding head also passed the complete GitHub Test Suite and Static Checks.e5547ffae43391aa068069889863070898a4ca4eagainst isolated IDC validation repositories: base-image pull, build and push-by-digest, immutable candidate tagging, pull from IDC, all-in-one readiness/health, exact Memoria write-read-delete, manifest assembly, and final manifest copy all passed. Candidate and final digest both equalsha256:0fe4431805c6a80271c0eff0cc04209a36a62de7c19599bc70308b22b1752d42; the final manifest contains onlylinux/amd64.Final checklist