fix: move container registry to nvcr#651
Conversation
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
WalkthroughThe container workflow now builds ChangesContainer publishing
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR migrates the published GPU container registry from GHCR to
Confidence Score: 5/5Safe to merge — the registry migration is well-structured with a fail-fast secret check, and all documentation is consistent with the workflow changes. The change is a straightforward registry migration: NVCR replaces GHCR as the publish target, GHCR is retained for cache only, and the split is clearly reflected in separate env vars. The fail-fast key-check step prevents silent failures if neither secret is present. Documentation across all four files is consistent and accurate. No files require special attention beyond the previously noted manual-dispatch build-time impact in .github/workflows/container-build.yml. Important Files Changed
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7770d127-e513-49f0-a3fc-0420b4f58d11
📒 Files selected for processing (4)
.github/workflows/README.md.github/workflows/container-build.ymlcontainers/README.mddocs/developer-guide/docker.md
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: Build cu129 image
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Unit Tests (3.11)
- GitHub Check: End-user Wheel Install
- GitHub Check: Typecheck
- GitHub Check: Analyze (Python)
- GitHub Check: Greptile Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{md,markdown,py}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings
Files:
containers/README.mddocs/developer-guide/docker.md
**/*.{md,markdown}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use##headers to segment markdown sections instead of bold text
Use--(em-dash) instead of-(hyphen) for asides in markdown
Files:
containers/README.mddocs/developer-guide/docker.md
**/*.md
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Do not use decorative bold in Markdown body text, list items, or docstrings; use single backticks for code identifiers, paths, and commands.
Use the repository's Markdown style and ensure Markdown source files have SPDX copyright headers unless excluded by
.copyrightignore.
Files:
containers/README.mddocs/developer-guide/docker.md
**/*
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
**/*: All merged commits must use Conventional Commits format:<type>(<scope>): <description>or<type>: <description>, with a lowercase valid type and a description no longer than 100 characters.
All contributions require a DCOSigned-off-bytrailer and a cryptographic commit signature.
Before submitting a pull request, run formatting, checks, and tests usingmise run format,mise run check, andmise run test.
Branches other thanmainmust use lowercase author-prefixed names in one of the documented forms, optionally including an issue ID and category.
Release tags must use avprefix and PEP 440 stable or release-candidate versions, such asv1.0.0orv0.1.0rc0; alpha versions and dashed-rcsuffixes are not used.
Do not move a published release tag; create and validate a new release candidate when code changes.
Usemise run <task>for project tasks; the Makefile only bootstraps mise and provides deprecated compatibility messages.
Files:
containers/README.mddocs/developer-guide/docker.md
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
containers/README.mddocs/developer-guide/docker.md
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All source files with
.py,.sh,.yaml,.yml, or.mdextensions must include SPDX copyright headers, except files listed in.copyrightignore.
Files:
containers/README.mddocs/developer-guide/docker.md
containers/**
⚙️ CodeRabbit configuration file
Review container changes against STYLE_GUIDE.md and containers/README.md. Check cache mounts, uv usage, non-root runtime behavior, CUDA settings, and copy order.
Files:
containers/README.md
.github/workflows/*.yml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.github/workflows/*.yml: Documentation deployment is defined in.github/workflows/docs.ymland runs whendocs/,mkdocs.yml, orsrc/changes are pushed tomain.
GitHub Actions workflows must use the repository's atomic, read-only mise tasks for CI checks.
Files:
.github/workflows/container-build.yml
.github/**
⚙️ CodeRabbit configuration file
Review GitHub configuration for branch protection expectations, CODEOWNERS alignment, least privilege permissions, pinned actions where practical, and consistency with CONTRIBUTING.md.
Files:
.github/workflows/container-build.yml.github/workflows/README.md
docs/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/writing-docs.mdc)
docs/**/*.md: Use MkDocs Material admonition syntax (!!! note, !!! warning, ??? tip) for highlighting important information and collapsible sections in documentation
Use MkDocs Material tabs syntax (=== "Label") to present alternative views or language-specific examples in documentation
Use code block syntax with title and highlight line parameters (title="filename", hl_lines="2 3") for code examples in documentation
Use Mermaid diagram syntax (```mermaid flowchart, etc.) for visualizations in documentationClassify documentation using Diátaxis and use MkDocs Material syntax for admonitions, tabs, and titled or highlighted code blocks.
Documentation pages must be Markdown files under
docs/, placed in the appropriate Diátaxis subdirectory, and added to thenav:section ofmkdocs.yml.
Files:
docs/developer-guide/docker.md
docs/**
⚙️ CodeRabbit configuration file
Review documentation as MkDocs Material content. Check Diataxis fit, accurate commands, internal links, code fences, and markdown style from STYLE_GUIDE.md.
Files:
docs/developer-guide/docker.md
🔇 Additional comments (7)
.github/workflows/container-build.yml (3)
33-38: LGTM!
73-95: LGTM!
136-137: LGTM!.github/workflows/README.md (2)
16-16: LGTM!
92-93: LGTM!Also applies to: 108-108
docs/developer-guide/docker.md (1)
190-193: LGTM! Consistent with theREGISTRY_IMAGE/multi-platform changes incontainer-build.ymland theCONTAINER_GPU_REGISTRYexample incontainers/README.md.Also applies to: 204-204, 213-215, 286-297
containers/README.md (1)
116-121: LGTM!
| - variant: cu129 | ||
| extra: cu129 | ||
| platforms: linux/amd64 | ||
| platforms: linux/amd64,linux/arm64 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## container-build workflow outline\n'
ast-grep outline .github/workflows/container-build.yml --view expanded || true
printf '\n## container-build workflow with line numbers (selected ranges)\n'
nl -ba .github/workflows/container-build.yml | sed -n '1,220p'
printf '\n## search for timeout, qemu, buildx, arm64, and cache-related settings in workflow docs\n'
rg -n --hidden -S 'timeout-minutes|qemu|buildx|arm64|platforms:|cache|cuda' .github/workflows .github README.md CONTRIBUTING.md mkdocs.yml docs src || true
printf '\n## list workflow files for context\n'
git ls-files .github/workflowsRepository: NVIDIA-NeMo/Safe-Synthesizer
Length of output: 360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## line count\n'
wc -l .github/workflows/container-build.yml
printf '\n## selected workflow ranges\n'
awk 'NR>=1 && NR<=220 { printf "%4d %s\n", NR, $0 }' .github/workflows/container-build.yml
printf '\n## relevant search hits\n'
rg -n -S 'timeout-minutes|qemu|buildx|arm64|platforms:|cache|cuda' .github/workflows/container-build.yml .github/workflows || true
printf '\n## related docs mentions\n'
rg -n -S 'cache|CUDA|arm64|QEMU|timeout|buildx|platforms' .github README.md CONTRIBUTING.md mkdocs.yml docs src || trueRepository: NVIDIA-NeMo/Safe-Synthesizer
Length of output: 50384
Avoid emulating the arm64 CUDA leg
platforms: linux/amd64,linux/arm64 plus setup-qemu-action for arm64 runs the arm64 build under QEMU in this same job. For a CUDA Dockerfile, that makes the release path much slower and more fragile; split amd64/arm64 into separate jobs or move arm64 to a native runner.
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
2 participants
Summary
Pre-Review Checklist
Ensure that the following pass:
mise run format && mise run checkor via prek validation.mise run testpasses locallymise run test:e2epasses locallymise run test:ci-containerpasses locally (recommended)/syncon this PR to trigger a run (auto-triggers on ready-for-review)Pre-Merge Checklist
Other Notes
Summary by CodeRabbit
New Features
Documentation