Skip to content

ci: establish workflow to clean up image artifacts - #390

Merged
crookedstorm merged 5 commits into
mainfrom
docker-cleanup/crookedstorm
Jun 22, 2026
Merged

ci: establish workflow to clean up image artifacts#390
crookedstorm merged 5 commits into
mainfrom
docker-cleanup/crookedstorm

Conversation

@crookedstorm

@crookedstorm crookedstorm commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Initially this is going to do a dry run on the PR, but the goal is to ensure we aren't adding lots of useless images to the ghcr registry, since most of the builds are purely for testing.

Summary by CodeRabbit

  • Chores
    • Added a scheduled and manual workflow to clean up stale container image versions in the registry, with configurable stale_hours and a dry_run mode to preview deletions.
  • CI
    • Updated Docker build/push/load behavior to pass through an allow=fs.read control via Bake flags.
    • Tightened the coverage comment trigger to run only on pull requests when relevant test jobs complete successfully or with failures.

Signed-off-by: Brooke Storm <brookes@nvidia.com>
@crookedstorm
crookedstorm requested a review from a team as a code owner June 22, 2026 19:08
@github-actions github-actions Bot added the ci label Jun 22, 2026
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds .github/scripts/cleanup-ghcr-ci-images.sh, a Bash script that queries GHCR package versions for nmp-api, nmp-core, and nmp-cpu-tasks, filters versions older than STALE_HOURS without a latest tag, and deletes them. Adds .github/workflows/ghcr-cleanup.yaml to invoke the script on a cron schedule and manual dispatch with dry-run support. Configures Makefile to pass optional filesystem read allowance flag through Docker builds. Refines .github/workflows/ci.yaml to export that flag and tighten coverage-comment job condition.

Changes

GHCR CI Image Cleanup

Layer / File(s) Summary
Script: validation, helpers, stale detection
.github/scripts/cleanup-ghcr-ci-images.sh
Validates STALE_HOURS is a positive integer, derives owner/repo from GITHUB_REPOSITORY, computes UTC cutoff, defines target image list (nmp-api, nmp-core, nmp-cpu-tasks), introduces url_encode() helper, implements find_stale_versions() to paginate GHCR versions endpoint and filter via jq by updated_at older than cutoff, excluding latest-tagged versions.
Script: deletion loop and result reporting
.github/scripts/cleanup-ghcr-ci-images.sh
Iterates target images, queries stale candidates, skips on HTTP 404 or empty results, deletes stale version IDs via gh api -X DELETE unless DRY_RUN=true, tracks candidate and deletion counters, prints final aggregate counts.
Workflow: triggers, permissions, and cleanup job
.github/workflows/ghcr-cleanup.yaml
Defines daily cron and manual workflow_dispatch triggers with dry_run and stale_hours inputs; applies restrictive permissions and concurrency keyed by repository; configures cleanup job with packages: write, maps DRY_RUN/STALE_HOURS from inputs, checks out code with credentials disabled, runs script with GH_TOKEN set.

Docker Build Filesystem Read Allowance

Layer / File(s) Summary
Makefile: filesystem read flag configuration
Makefile
Adds DOCKER_BAKE_ALLOW_FS_READ variable and derives DOCKER_BAKE_ALLOW_FS_READ_FLAG to construct conditional --allow=fs.read=... flag; updates docker-build, docker-load, docker-push targets to include flag in docker buildx bake invocations.
CI workflow: filesystem read allowance export
.github/workflows/ci.yaml
In build-cpu-smoke-images job's bake-variables step, exports DOCKER_BAKE_ALLOW_FS_READ environment variable with fastembed cache directory value, enabling the Makefile flag during Docker builds.

CI Coverage Comment Refinement

Layer / File(s) Summary
Coverage-comment job condition
.github/workflows/ci.yaml
Adds explicit github.event_name == 'pull_request' check and requires needs.python-unit-test.result or needs.python-integration-test.result to be success or failure instead of running on all pull_request events under always().

Possibly related PRs

  • NVIDIA-NeMo/nemo-platform#345: Adds/gates kube smoke image build using make docker-load/make docker-push targets that are updated in this PR to support the DOCKER_BAKE_ALLOW_FS_READ flag.

Suggested reviewers

  • svvarom
  • matthewgrossman
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately describes the main change: establishing a CI workflow for GHCR image cleanup, which is the core purpose of the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docker-cleanup/crookedstorm

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Brooke Storm <brookes@nvidia.com>
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 20910/27422 76.2% 61.3%
Integration Tests 12007/26191 45.8% 19.3%

Signed-off-by: Brooke Storm <brookes@nvidia.com>
@crookedstorm
crookedstorm requested review from a team as code owners June 22, 2026 19:29
Signed-off-by: Brooke Storm <brookes@nvidia.com>
@svvarom
svvarom self-requested a review June 22, 2026 19:48
@svvarom

svvarom commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Initially this is going to do a dry run on the PR, but the goal is to ensure we aren't adding lots of useless images to the ghcr registry, since most of the builds are purely for testing.

LGTM, but did we try a dry run on this PR?

@crookedstorm

crookedstorm commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Initially this is going to do a dry run on the PR, but the goal is to ensure we aren't adding lots of useless images to the ghcr registry, since most of the builds are purely for testing.

LGTM, but did we try a dry run on this PR?

I did https://github.com/NVIDIA-NeMo/nemo-platform/actions/runs/27977381912/job/82798482032

Signed-off-by: Brooke Storm <brookes@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Makefile`:
- Around line 36-37: The DOCKER_BAKE_ALLOW_FS_READ_FLAG variable definition uses
the incorrect syntax for the docker buildx bake allow flag. Change the flag from
--allow=fs.read=$(DOCKER_BAKE_ALLOW_FS_READ) to --allow
fs.read=$(DOCKER_BAKE_ALLOW_FS_READ) by replacing the equals sign with a space
after --allow to match the documented Docker syntax.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 65e3c9dd-b8fe-485c-8399-7f16eb1be015

📥 Commits

Reviewing files that changed from the base of the PR and between ebfec7d and 1c0276c.

📒 Files selected for processing (2)
  • .github/workflows/ci.yaml
  • Makefile

Comment thread Makefile

@svvarom svvarom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@crookedstorm
crookedstorm enabled auto-merge June 22, 2026 21:14
@crookedstorm
crookedstorm added this pull request to the merge queue Jun 22, 2026
Merged via the queue into main with commit e28fb1d Jun 22, 2026
53 checks passed
@crookedstorm
crookedstorm deleted the docker-cleanup/crookedstorm branch June 22, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants