fix(ci): use GHCR instead of ACR for workflow docker caches - #2264
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to move GitHub Actions Docker image caching from Azure Container Registry (ACR) to GitHub Container Registry (GHCR) so GitHub workflows don’t require mutable Azure access.
Changes:
- Update Docker image naming logic to stop hardcoding the
*.azurecr.iohostname. - Switch Linux GPU x64 and Linux CPU ARM64 GitHub workflows to use GHCR as the container registry for build-cache images.
- Add a new repository
.ignorefile.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/ci_build/get_docker_image.py | Changes how full_image_name is constructed when a container registry is provided (now assumes a full registry path). |
| .github/workflows/linux-gpu-x64-build.yml | Updates the workflow to authenticate and push/pull cached images from GHCR instead of ACR. |
| .github/workflows/linux-cpu-arm64-build.yml | Updates the workflow to authenticate and push/pull cached images from GHCR instead of ACR. |
| .ignore | Introduces a new ignore configuration file with only negated patterns. |
c031f8d to
b484991
Compare
tianleiwu
left a comment
There was a problem hiding this comment.
Reviewed the switch from ACR to GHCR for the GitHub-hosted docker build caches. The overall design is coherent and correct: GHCR gets read/write (push cache) via job-scoped packages: write, while the ADO pipelines keep pushing to ACR (onnxruntimebuildcache.azurecr.io), and all five in-repo --container-registry call sites were updated consistently for the new full-hostname semantics. Scoping permissions: {contents: read, packages: write} per job is a nice least-privilege touch.
One clarification for the record: the az acr login retained in the two GitHub workflows is not redundant. The Dockerfiles pull their base images FROM onnxruntimebuildcache.azurecr.io/... (e.g. Dockerfile.manylinux2_28_cuda_12.8 uses ARG BASEIMAGE=onnxruntimebuildcache.azurecr.io/..., and the aarch64 CPU Dockerfile has a hard-coded ACR FROM), so ACR pull access is still required. Removing those az login/az acr login steps would break the base-image pull. Keeping ACR pull-only (via the read-only managed identity, which the new object-id 161bbca8... appears to be) is exactly what "Azure resources read-only from GH workflows" means, so this looks intentional and fine.
Minor: the new .ignore file is missing a trailing newline and is unrelated to the CI cache change. Worth confirming which tool is meant to consume it (ripgrep/fd read .ignore and skip dot-dirs by default, so the negated entries do have an effect there, but the file reads as scope creep for this PR).
Suggestions inline. Nothing blocking.
b484991 to
f25ce92
Compare
Azure resources should be read-only from GH workflows.
Azure resources should be read-only from GH workflows.