[codex] clean up Kubernetes snapshot registry images

[GodBlf]

Summary

• delete pushed OCI manifests when a Kubernetes SandboxSnapshot is deleted
• reuse the configured snapshot push secret for registry authentication
• keep the snapshot finalizer on cleanup failures so reconciliation can retry
• document the registry deletion permission requirement

Root cause

The Kubernetes snapshot finalizer only removed commit and unpause Jobs before removing itself. It never used status.containers[].imageUri or imageDigest to delete the pushed manifests, leaving orphaned images in the registry.

Implementation

The controller now deletes each unique snapshot manifest before removing the finalizer. It prefers the recorded digest, resolves legacy tag-only snapshots to a digest, treats missing manifests as an idempotent success, and supports the existing Docker config registry secret and insecure-registry setting.

Validation

• cd kubernetes && make test
• focused finalizer, credential parsing, and HTTP registry protocol tests
• cd docs && pnpm docs:build
• git diff --check

Fixes #1179

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bdd4c3d3a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: acf2fdf6e3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[GodBlf]

@Pangjiping Could you please review this PR when you have time? If the changes look correct and the required checks pass, would you be willing to merge it? I will address any requested changes promptly.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47782ffaac

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[Pangjiping]

The new e2e check is failing on all 8 Controller E2E PauseResume jobs (e.g. https://github.com/opensandbox-group/OpenSandbox/actions/runs/31482910826/job/93751835292): deterministic [FAILED] Expected <int>: 200 to equal <int>: 404 at pause_resume_test.go:1122 (the manifest HEAD check).

Timeline in the job log shows the snapshot CR was already gone (kubectl get sandboxsnapshot returned NotFound) ~100ms before the manifest check ran. Since the finalizer is only removed after deleteSnapshotImages succeeds, the deletion path completed without actually removing the tag's manifest.

Most likely root cause: a digest mismatch. image-committer records the local content-store digest via nerdctl inspect --format {{.Id}} (cmd/image-committer/main.go:245,663) before the push, but if the push changes the manifest (e.g. nerdctl OCI→docker v2s2 media-type conversion on nerdctl push), the registry's manifest digest differs from the recorded one. The deleter then skips the HEAD resolution (recorded digest present) and DELETE-by-recorded-digest returns 404, which isRegistryNotFound treats as idempotent success → finalizer removed, tag's manifest left behind. The `RemoteRegistryImageDeleter_* unit tests can't catch this because they inject the digest directly.

Suggestions:

1. In the e2e, log the recorded imageDigest from the snapshot status and the registry's Docker-Content-Digest for the tag, to confirm the mismatch.
2. Record the digest from the push response (Docker-Content-Digest) in image-committer, or push without media-type conversion.
3. In remoteRegistryImageDeleter.Delete, a 404 on the recorded-digest branch is currently indistinguishable from "already deleted" — log it, and consider falling back to tag resolution (with the documented tag-reuse caveat) when the recorded digest delete returns 404.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 179cae69ea

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve snapshot images used by resumed pods

When this finalizer runs for the internal pause snapshot after a successful resume, continueResume has already patched the BatchSandbox template to status.containers[].imageUri, and persistRuntimeView then deletes that internal snapshot as soon as the resumed pod is Ready. Deleting the registry manifest here therefore leaves the resumed sandbox dependent on an image that no longer exists: if the pod is later evicted/rescheduled or the node image is GC'd, Kubernetes will pull the now-deleted snapshot image and the sandbox will fail. Keep internal resume images until the template is no longer using them, or only run registry cleanup for user-requested snapshot deletions.

AGENTS.md reference: kubernetes/AGENTS.md:L170-L176

Useful? React with 👍 / 👎.

[GodBlf]

@Pangjiping The CI failure was caused by the recorded local content-store digest differing from the registry manifest digest after nerdctl media-type conversion. I pushed the fix in commits ca0354f8 and 179cae69: cleanup now falls back to resolving the original tag when digest deletion returns 404, with a regression test. The PR head is now 179cae69; local Kubernetes tests and docs build pass. GitHub currently shows the new workflows as action_required, so they may need maintainer approval before running. Could you please approve the workflows and take another look at the updated PR? Thanks.

[GodBlf]

@Pangjiping The latest CI run still failed in every PauseResume matrix at pause_resume_test.go:1122: the snapshot CR was deleted but the registry tag still returned 200. The recorded local digest cleanup could be reported as accepted even when it did not match the tag manifest. I pushed 85cb70a5, which verifies the tag digest after every recorded-digest DELETE and removes the registry-reported digest when it differs, with a regression test for an accepted DELETE. Local go test ./internal/controller/... and git diff --check pass. Please rerun/review the updated PR.

[GodBlf]

@Pangjiping The fork CI confirmed the manifest cleanup assertion now passes. All 8 PauseResume jobs failed later by timing out in AfterAll: teardown deleted the registry and credentials before synchronously deleting the intentionally tampered failure-path snapshot, whose strict cleanup finalizer could no longer complete. I pushed 4cd7bfce to request resource deletion while the registry is still available, use --wait=false, and apply the documented manual-finalizer escape hatch to any remaining failure snapshots before removing credentials/registry. The e2e package compile check and git diff --check pass; a new fork CI run should validate the complete matrix.

[GodBlf]

@Pangjiping The snapshot-related CI is fully green on head 4cd7bfce: all 8 PauseResume jobs, all 8 Core jobs, unit tests, and the registry manifest cleanup assertion passed in the fork Sandbox Kubernetes Tests run: https://github.com/GodBlf/OpenSandbox/actions/runs/31556283844

The separate fork Kubernetes nightly build (Monorepo) initially had one unrelated ingress-header SSE incomplete chunked read. I reran only the failed jobs without changing the PR; attempt 2 passed ingress-header, direct, ingress-uri, and the aggregate check: https://github.com/GodBlf/OpenSandbox/actions/runs/31556283641. This confirms the first failure was transient and no unrelated ingress/SDK changes were added to this PR.

[GodBlf]

@Pangjiping The current head 4cd7bfce is ready for maintainer review. The fork validation is fully green:

• Full Sandbox Kubernetes Tests: unit tests, all 8 PauseResume jobs, all 8 Core jobs, and the registry manifest cleanup assertion passed: https://github.com/GodBlf/OpenSandbox/actions/runs/31556283844
• Kubernetes nightly build (Monorepo) attempt 2: ingress-header, ingress-uri, direct, and the aggregate mini-E2E check all passed: https://github.com/GodBlf/OpenSandbox/actions/runs/31556283641

The upstream PR currently only shows auto-label, so its fork workflows may still require maintainer approval. The PR is also marked BEHIND the base branch. Could you please approve/run the upstream checks and review the PR, and let me know if you want me to merge the latest main first? Thanks.