Skip to content

ci(release): reuse verified pipeline for IDC images - #722

Open
gouhongshen wants to merge 5 commits into
matrixorigin:mainfrom
gouhongshen:codex/fix-idc-build-pipeline
Open

ci(release): reuse verified pipeline for IDC images#722
gouhongshen wants to merge 5 commits into
matrixorigin:mainfrom
gouhongshen:codex/fix-idc-build-pipeline

Conversation

@gouhongshen

@gouhongshen gouhongshen commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

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

  • Bug fix
  • Documentation
  • Refactor or performance improvement
  • Test
  • Build, CI, or maintenance

User and compatibility impact

build_push_to_idc keeps its existing manual entrypoint and source selection. It now offers amd64 and all, 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

  • Canonical owner changed or extended: .github/workflows/build_push_to_idc.yml remains the IDC publication controller; .github/workflows/idc-container-candidates.yml is the IDC destination copy of the established release-candidate workflow; scripts/copy-immutable-container-tag.sh owns final cross-repository immutable-tag reconciliation.
  • Existing implementations and callers searched: the current release.yml, release-container-candidates.yml, manifest reconciliation scripts, the merged IDC workflow, the failed IDC run, repository validation, and release documentation.
  • Superseded code, states, tables, shims, or self-only tests removed: removed the self-hosted single-builder path whose BuildKit could not pull base images, and replaced inline error-text classification with exact matching over a successfully enumerated repository tag set.
  • If parallel implementations remain, their boundary and retirement condition: release candidates publish only to Docker Hub; IDC candidates publish only to IDC. They intentionally share the same build/smoke/digest topology but have different credential and registry boundaries.

Verification

  • Commands and results: python3 scripts/ci/test_release_build_shells.py passed (16 tests); python3 scripts/ci/validate_repository.py passed with Python 3.12; git diff --check and Bash syntax validation passed. The preceding head also passed the complete GitHub Test Suite and Static Checks.
  • Public entrypoint exercised: the unmerged AMD64 path was executed end to end with source e5547ffae43391aa068069889863070898a4ca4e against 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 equal sha256:0fe4431805c6a80271c0eff0cc04209a36a62de7c19599bc70308b22b1752d42; the final manifest contains only linux/amd64.
  • Unhappy paths exercised: invalid source ref, historical source selection, non-main controller rejection, invalid architecture, missing IDC variables, missing credentials, malformed registry/image values, existing matching and conflicting runtime tags, confirmed missing tags, TLS timeout, HTTP 401/403, registry 5xx, and a token endpoint returning 404. The executable publisher test proves copy is never called for a conflicting target or whenever repository enumeration fails, regardless of error text.
  • Database verification: N/A.

Final checklist

  • I added or updated tests at the layer that owns the behavior, or explained why no test is needed.
  • I updated public or design documentation for contract changes, or the change needs no documentation update.
  • I checked the diff for credentials, private URLs, customer data, generated files, and other sensitive information.
  • The PR title follows the repository's Conventional Commit format.

@gouhongshen
gouhongshen force-pushed the codex/fix-idc-build-pipeline branch from 37d7c07 to 981babc Compare September 6, 2026 19:11
@gouhongshen
gouhongshen marked this pull request as ready for review September 6, 2026 19:12

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .github/workflows/build_push_to_idc.yml Outdated

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. The source serves verified manifest A; the target already serves a different manifest B under the release tag.
  2. 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.
  3. The token-service fault clears before copy. The script interprets the earlier authentication error as absence and invokes crane copy.
  4. 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.

Comment thread scripts/copy-immutable-container-tag.sh Outdated

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +31 to +34
target_exists=false
if ! crane ls "${target_repository}" > "${target_tags}"; then
echo "could not safely enumerate tags in ${target_repository}" >&2
exit 1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants