feat(images): expose registry tags and improve rollback visibility#74
feat(images): expose registry tags and improve rollback visibility#74
Conversation
📝 WalkthroughWalkthroughAdds registry-tag awareness to image listing and pruning (keeps Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Service as ImageService
participant Runtime as RuntimeAPI
participant Manifest as ManifestStorage
CLI->>Service: ListImages(ctx)
Service->>Runtime: Fetch runtime images
Runtime-->>Service: Runtime image list
Service->>Manifest: ListRepositories()
Manifest-->>Service: Repositories
loop per repository
Service->>Manifest: ListTags(repo)
Manifest-->>Service: Tag list
Service->>Manifest: GetManifestModifiedTime(repo, tag)
Manifest-->>Service: ModifiedTime
end
Service->>Service: Merge runtime + registry tags, dedupe repo:tag
Service->>Service: Sort by dangling, repo, tag precedence (latest first), Created
Service-->>CLI: Aggregated image list (includes registry-only tags)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Pull request overview
This PR enhances Gordon’s image/rollback UX by surfacing registry-only tags alongside runtime-cached images, adjusting registry prune retention to improve rollback safety, and adding a non-deploying rollback inspection command.
Changes:
- Extend
ListImagesto merge runtime image details with registry tag listings (including registry-only tags). - Update registry prune retention to always keep
latestpluskeep_lastmost-recent non-latesttags. - Add
gordon rollback list <domain>to show available rollback tags and mark the currently running tag without deploying.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/usecase/images/service.go | Merges runtime images with registry tags; updates retention logic; adds sorting/normalization helpers. |
| internal/usecase/images/service_test.go | Updates/extends tests for merged list behavior and revised prune retention semantics. |
| internal/domain/images.go | Clarifies prune retention semantics and broadens ImageInfo description. |
| internal/boundaries/in/images.go | Updates interface doc to reflect registry tag inclusion in ListImages. |
| internal/adapters/in/cli/images.go | Updates CLI text/flags; formats missing runtime fields as -; adjusts dry-run messaging. |
| internal/adapters/in/cli/images_test.go | Updates assertion to match new prune messaging. |
| internal/adapters/in/cli/rollback.go | Adds rollback list subcommand and printing of available tags with current marker. |
| internal/adapters/in/cli/remote/client.go | Updates ListImages docstring to reflect expanded API semantics. |
| internal/adapters/dto/admin_images.go | Updates DTO comment to reflect image/tag rows (not runtime-only). |
| docs/config/images.md | Documents revised retention behavior (latest + N non-latest tags). |
| docs/cli/images.md | Documents registry-only rows and updated prune --keep semantics. |
| docs/cli/rollback.md | Documents new rollback list command and behavior distinction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| func formatImageSize(size int64) string { | ||
| if size <= 0 { |
There was a problem hiding this comment.
formatImageSize treats size==0 as unknown (prints "-"). If a real runtime image can legitimately report a 0-byte size, this will incorrectly hide that value. Consider reserving a distinct sentinel for “unknown” (e.g., -1) and keeping 0 formatted as "0 B", or add an explicit boolean/nullable field to indicate unknown size.
| if size <= 0 { | |
| if size < 0 { |
| func formatImageID(id string) string { | ||
| if strings.TrimSpace(id) == "" { | ||
| return "-" | ||
| } | ||
|
|
There was a problem hiding this comment.
New list output now depends on "unknown" formatting (e.g., empty ID -> "-", Size<=0 -> "-"), but there isn’t a targeted test asserting those placeholders in runImagesList output. Adding a small test case with an image that has empty ID and Size/Created unset would help prevent regressions in the CLI table formatting.
| repositories, err := s.manifestStorage.ListRepositories() | ||
| if err != nil { | ||
| return nil, log.WrapErr(err, "failed to list repositories") | ||
| } |
There was a problem hiding this comment.
ListImages now hard-fails if registry access fails (e.g., ListRepositories/ListTags). That makes images list unavailable when the registry storage is temporarily unhealthy, even though runtime images could still be returned. Consider degrading gracefully: return the runtime-derived images and log/wrap a warning for the registry portion (or include a partial-error signal) instead of returning an error here.
|
|
||
| images = append(images, domain.ImageInfo{ | ||
| Repository: displayRepository, | ||
| Tag: tag, |
There was a problem hiding this comment.
Registry-only entries are emitted with the zero values for Size (0) and ID (""). This makes it ambiguous for API/CLI consumers to distinguish “unknown/not present in runtime” from a legitimate 0-byte size, and requires the CLI to guess. Consider using an explicit sentinel (e.g., Size = -1) and documenting it, or switching these fields to pointers/nullable in the DTO so registry-only rows can omit runtime-specific fields cleanly.
| Tag: tag, | |
| Tag: tag, | |
| // Size is unknown for registry-only images (not present in runtime). | |
| // Use -1 as a sentinel to distinguish from a legitimate 0-byte size. | |
| Size: -1, |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/usecase/images/service.go (1)
47-148:⚠️ Potential issue | 🟠 MajorReduce
ListImagescyclomatic complexity to fix the gocyclo CI failure.CI reports complexity 17 (>15) for this function. Please split the runtime and registry flows into helpers to get under the threshold.
♻️ Suggested refactor sketch
func (s *Service) ListImages(ctx context.Context) ([]domain.ImageInfo, error) { ctx = zerowrap.CtxWithFields(ctx, map[string]any{ zerowrap.FieldLayer: "usecase", zerowrap.FieldUseCase: "ListImages", }) log := zerowrap.FromCtx(ctx) details, err := s.runtime.ListImagesDetailed(ctx) if err != nil { return nil, log.WrapErr(err, "failed to list images") } - images := make([]domain.ImageInfo, 0, len(details)) - seenRepoTags := make(map[string]struct{}, len(details)) - repoDisplayByNormalized := make(map[string]string) - // move existing runtime loop into buildRuntimeImages - // move existing registry loop into appendRegistryImages - // (logic unchanged) - ... - repositories, err := s.manifestStorage.ListRepositories() - if err != nil { - return nil, log.WrapErr(err, "failed to list repositories") - } - ... - for _, repository := range repositories { - ... - } + images, seenRepoTags, repoDisplayByNormalized := buildRuntimeImages(details) + if err := s.appendRegistryImages(ctx, &images, seenRepoTags, repoDisplayByNormalized); err != nil { + return nil, err + } sort.SliceStable(images, func(i, j int) bool { return lessImageInfo(images[i], images[j]) }) return images, nil }
🤖 Fix all issues with AI agents
In `@internal/adapters/in/cli/rollback.go`:
- Around line 115-124: Change printRollbackTags to accept an io.Writer (e.g., w
io.Writer) and write to that writer using fmt.Fprintf instead of printing to
stdout; update its signature (printRollbackTags) and all call sites (notably
runRollbackList) to pass the desired writer (e.g., cmd.OutOrStdout() or
os.Stdout in production, a bytes.Buffer in tests) so the output can be captured
in unit tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/adapters/in/cli/rollback.go`:
- Around line 90-124: The printRollbackTags function currently ignores
fmt.Fprintf errors; change printRollbackTags to return error, check and
propagate the error results of both fmt.Fprintf calls (the header and each tag
line), and update runRollbackList to capture and return any error from
printRollbackTags (so runRollbackList handles write failures instead of
discarding them). Ensure the signature becomes printRollbackTags(w io.Writer,
rollbackDomain, currentTag string, tags []string) error and adjust the call in
runRollbackList accordingly to return the propagated error.
- Add pruneWithSpinner that shows progress during image cleanup - Test for idempotent prune: running prune twice should not delete additional items or error - Spinner message adapts based on which scopes are enabled (runtime, registry, or both) - Handles missing child manifests gracefully (log warning, continue) Fixes #74
1 participant
Summary
gordon images list, merging runtime and registry views so version tags are visible even when not cached locallylatestplus the configured number of previous non-latest tags for safer rollbacksgordon rollback list <domain>to inspect available rollback tags (with current tag marker) without triggering a deployTesting
Summary by CodeRabbit
New Features
rollback list <domain>to list available tags without deployingDocumentation
latest;latestis always preserved when presentBug Fixes
Tests