feat(images): add spinner during prune and verify idempotent behavior
- 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
- commit
- 808089b3b2571eeec68425a0f290f744bd2a52ab
- author
- brice · · 2026-02-13 18:09 UTC
- parent
- f7b9ef9
- signature
- unsigned
2 files · +85 -1
M internal/adapters/in/cli/images.go +54 -1
| @@ -133,6 +133,59 @@ func resolvePruneScopes(opts imagesPruneOptions) (pruneDangling, pruneRegistry b | |||
| 133 | 133 | return opts.Dangling, opts.Registry | |
| 134 | 134 | } | |
| 135 | 135 | ||
| 136 | + | func pruneWithSpinner(ctx context.Context, client imagesClient, req dto.ImagePruneRequest, pruneDangling, pruneRegistry bool) (*dto.ImagePruneResponse, error) { | |
| 137 | + | if !isInteractiveTerminal() { | |
| 138 | + | return client.PruneImages(ctx, req) | |
| 139 | + | } | |
| 140 | + | ||
| 141 | + | type pruneResult struct { | |
| 142 | + | resp *dto.ImagePruneResponse | |
| 143 | + | err error | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | done := make(chan pruneResult, 1) | |
| 147 | + | go func() { | |
| 148 | + | resp, err := client.PruneImages(ctx, req) | |
| 149 | + | done <- pruneResult{resp: resp, err: err} | |
| 150 | + | }() | |
| 151 | + | ||
| 152 | + | // Build spinner message based on scopes | |
| 153 | + | msg := "Pruning images" | |
| 154 | + | if pruneDangling && pruneRegistry { | |
| 155 | + | msg = "Pruning runtime and registry images" | |
| 156 | + | } else if pruneDangling { | |
| 157 | + | msg = "Pruning runtime images" | |
| 158 | + | } else if pruneRegistry { | |
| 159 | + | msg = "Pruning registry images" | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | model := components.NewSpinner( | |
| 163 | + | components.WithMessage(msg), | |
| 164 | + | components.WithSpinnerType(components.SpinnerMiniDot), | |
| 165 | + | ) | |
| 166 | + | ||
| 167 | + | for { | |
| 168 | + | select { | |
| 169 | + | case result := <-done: | |
| 170 | + | if result.err != nil { | |
| 171 | + | return nil, result.err | |
| 172 | + | } | |
| 173 | + | if result.resp == nil { | |
| 174 | + | return nil, fmt.Errorf("prune operation completed but no response received") | |
| 175 | + | } | |
| 176 | + | // Success - wait for final frame to render | |
| 177 | + | model.SetMessage(cliRenderSuccess("Pruning complete")) | |
| 178 | + | time.Sleep(100 * time.Millisecond) | |
| 179 | + | cmd := model.Init() | |
| 180 | + | fmt.Print(cmd) | |
| 181 | + | return result.resp, nil | |
| 182 | + | case <-time.After(100 * time.Millisecond): | |
| 183 | + | cmd := model.Init() | |
| 184 | + | fmt.Print(cmd) | |
| 185 | + | } | |
| 186 | + | } | |
| 187 | + | } | |
| 188 | + | ||
| 136 | 189 | func runImagesList(ctx context.Context, client imagesClient, out io.Writer) error { | |
| 137 | 190 | images, err := client.ListImages(ctx) | |
| 138 | 191 | if err != nil { | |
| @@ -214,7 +267,7 @@ func runImagesPrune(ctx context.Context, client imagesClient, opts imagesPruneOp | |||
| 214 | 267 | ||
| 215 | 268 | req := buildPruneRequest(opts.KeepReleases, pruneDangling, pruneRegistry) | |
| 216 | 269 | ||
| 217 | - | resp, err := client.PruneImages(ctx, req) | |
| 270 | + | resp, err := pruneWithSpinner(ctx, client, req, pruneDangling, pruneRegistry) | |
| 218 | 271 | if err != nil { | |
| 219 | 272 | return fmt.Errorf("failed to prune images: %w", err) | |
| 220 | 273 | } | |
| @@ -133,6 +133,59 @@ func resolvePruneScopes(opts imagesPruneOptions) (pruneDangling, pruneRegistry b | |||||
| 133 | return opts.Dangling, opts.Registry | 133 | return opts.Dangling, opts.Registry | ||
| 134 | } | 134 | } | ||
| 135 | 135 | ||||
| 136 | + | func pruneWithSpinner(ctx context.Context, client imagesClient, req dto.ImagePruneRequest, pruneDangling, pruneRegistry bool) (*dto.ImagePruneResponse, error) { | |||
| 137 | + | if !isInteractiveTerminal() { | |||
| 138 | + | return client.PruneImages(ctx, req) | |||
| 139 | + | } | |||
| 140 | + | ||||
| 141 | + | type pruneResult struct { | |||
| 142 | + | resp *dto.ImagePruneResponse | |||
| 143 | + | err error | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | done := make(chan pruneResult, 1) | |||
| 147 | + | go func() { | |||
| 148 | + | resp, err := client.PruneImages(ctx, req) | |||
| 149 | + | done <- pruneResult{resp: resp, err: err} | |||
| 150 | + | }() | |||
| 151 | + | ||||
| 152 | + | // Build spinner message based on scopes | |||
| 153 | + | msg := "Pruning images" | |||
| 154 | + | if pruneDangling && pruneRegistry { | |||
| 155 | + | msg = "Pruning runtime and registry images" | |||
| 156 | + | } else if pruneDangling { | |||
| 157 | + | msg = "Pruning runtime images" | |||
| 158 | + | } else if pruneRegistry { | |||
| 159 | + | msg = "Pruning registry images" | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | model := components.NewSpinner( | |||
| 163 | + | components.WithMessage(msg), | |||
| 164 | + | components.WithSpinnerType(components.SpinnerMiniDot), | |||
| 165 | + | ) | |||
| 166 | + | ||||
| 167 | + | for { | |||
| 168 | + | select { | |||
| 169 | + | case result := <-done: | |||
| 170 | + | if result.err != nil { | |||
| 171 | + | return nil, result.err | |||
| 172 | + | } | |||
| 173 | + | if result.resp == nil { | |||
| 174 | + | return nil, fmt.Errorf("prune operation completed but no response received") | |||
| 175 | + | } | |||
| 176 | + | // Success - wait for final frame to render | |||
| 177 | + | model.SetMessage(cliRenderSuccess("Pruning complete")) | |||
| 178 | + | time.Sleep(100 * time.Millisecond) | |||
| 179 | + | cmd := model.Init() | |||
| 180 | + | fmt.Print(cmd) | |||
| 181 | + | return result.resp, nil | |||
| 182 | + | case <-time.After(100 * time.Millisecond): | |||
| 183 | + | cmd := model.Init() | |||
| 184 | + | fmt.Print(cmd) | |||
| 185 | + | } | |||
| 186 | + | } | |||
| 187 | + | } | |||
| 188 | + | ||||
| 136 | func runImagesList(ctx context.Context, client imagesClient, out io.Writer) error { | 189 | func runImagesList(ctx context.Context, client imagesClient, out io.Writer) error { | ||
| 137 | images, err := client.ListImages(ctx) | 190 | images, err := client.ListImages(ctx) | ||
| 138 | if err != nil { | 191 | if err != nil { | ||
| @@ -214,7 +267,7 @@ func runImagesPrune(ctx context.Context, client imagesClient, opts imagesPruneOp | |||||
| 214 | 267 | ||||
| 215 | req := buildPruneRequest(opts.KeepReleases, pruneDangling, pruneRegistry) | 268 | req := buildPruneRequest(opts.KeepReleases, pruneDangling, pruneRegistry) | ||
| 216 | 269 | ||||
| 217 | - | resp, err := client.PruneImages(ctx, req) | 270 | + | resp, err := pruneWithSpinner(ctx, client, req, pruneDangling, pruneRegistry) |
| 218 | if err != nil { | 271 | if err != nil { | ||
| 219 | return fmt.Errorf("failed to prune images: %w", err) | 272 | return fmt.Errorf("failed to prune images: %w", err) | ||
| 220 | } | 273 | } | ||
M internal/usecase/images/service_test.go +31 -0
| @@ -458,6 +458,37 @@ func TestService_PruneRegistry_EmptyRepository(t *testing.T) { | |||
| 458 | 458 | assert.Empty(t, blobStorage.deletedBlobs) | |
| 459 | 459 | } | |
| 460 | 460 | ||
| 461 | + | func TestService_Prune_IdempotentMultiplePrunesDoNotAccumulate(t *testing.T) { | |
| 462 | + | manifestStorage := newFakeManifestStorage() | |
| 463 | + | manifestStorage.repositories = []string{"gordon/api"} | |
| 464 | + | manifestStorage.tagsByRepo["gordon/api"] = []string{"latest", "v2", "v1"} | |
| 465 | + | manifestStorage.modTimes[manifestRefKey("gordon/api", "latest")] = time.Date(2026, 2, 8, 12, 0, 0, 0, time.UTC) | |
| 466 | + | manifestStorage.modTimes[manifestRefKey("gordon/api", "v2")] = time.Date(2026, 2, 8, 11, 0, 0, 0, time.UTC) | |
| 467 | + | manifestStorage.modTimes[manifestRefKey("gordon/api", "v1")] = time.Date(2026, 2, 8, 10, 0, 0, 0, time.UTC) | |
| 468 | + | manifestStorage.manifests[manifestRefKey("gordon/api", "latest")] = mustManifestJSON(t, "sha256:cfg-live", "sha256:layer-live") | |
| 469 | + | manifestStorage.manifests[manifestRefKey("gordon/api", "v2")] = mustManifestJSON(t, "sha256:cfg-v2", "sha256:layer-v2") | |
| 470 | + | blobStorage := &fakeBlobStorage{blobs: []string{"sha256:cfg-live", "sha256:layer-live", "sha256:orphan"}} | |
| 471 | + | rt := &fakeRuntime{pruneReport: pkgruntime.PruneReport{DeletedIDs: []string{"sha256:img1"}, SpaceReclaimed: 1024}} | |
| 472 | + | ||
| 473 | + | svc := NewService(rt, manifestStorage, blobStorage, zerowrap.Default()) | |
| 474 | + | ||
| 475 | + | // First prune - should remove v1 and orphan | |
| 476 | + | report1, err := svc.Prune(context.Background(), domain.ImagePruneOptions{ | |
| 477 | + | KeepLast: 1, PruneDangling: true, PruneRegistry: true, | |
| 478 | + | }) | |
| 479 | + | require.NoError(t, err) | |
| 480 | + | assert.Equal(t, 1, report1.Registry.TagsRemoved, "first prune should remove v1") | |
| 481 | + | assert.Equal(t, 1, report1.Registry.BlobsRemoved, "first prune should remove orphan") | |
| 482 | + | ||
| 483 | + | // Second prune - should be idempotent (nothing else to remove) | |
| 484 | + | report2, err := svc.Prune(context.Background(), domain.ImagePruneOptions{ | |
| 485 | + | KeepLast: 1, PruneDangling: true, PruneRegistry: true, | |
| 486 | + | }) | |
| 487 | + | require.NoError(t, err) | |
| 488 | + | assert.Equal(t, 0, report2.Registry.TagsRemoved, "second prune should remove nothing (idempotent)") | |
| 489 | + | assert.Equal(t, 0, report2.Registry.BlobsRemoved, "second prune should remove nothing (idempotent)") | |
| 490 | + | } | |
| 491 | + | ||
| 461 | 492 | func TestService_Prune_RunsBothRuntimeAndRegistry(t *testing.T) { | |
| 462 | 493 | manifestStorage := newFakeManifestStorage() | |
| 463 | 494 | manifestStorage.repositories = []string{"gordon/api"} | |
| @@ -458,6 +458,37 @@ func TestService_PruneRegistry_EmptyRepository(t *testing.T) { | |||||
| 458 | assert.Empty(t, blobStorage.deletedBlobs) | 458 | assert.Empty(t, blobStorage.deletedBlobs) | ||
| 459 | } | 459 | } | ||
| 460 | 460 | ||||
| 461 | + | func TestService_Prune_IdempotentMultiplePrunesDoNotAccumulate(t *testing.T) { | |||
| 462 | + | manifestStorage := newFakeManifestStorage() | |||
| 463 | + | manifestStorage.repositories = []string{"gordon/api"} | |||
| 464 | + | manifestStorage.tagsByRepo["gordon/api"] = []string{"latest", "v2", "v1"} | |||
| 465 | + | manifestStorage.modTimes[manifestRefKey("gordon/api", "latest")] = time.Date(2026, 2, 8, 12, 0, 0, 0, time.UTC) | |||
| 466 | + | manifestStorage.modTimes[manifestRefKey("gordon/api", "v2")] = time.Date(2026, 2, 8, 11, 0, 0, 0, time.UTC) | |||
| 467 | + | manifestStorage.modTimes[manifestRefKey("gordon/api", "v1")] = time.Date(2026, 2, 8, 10, 0, 0, 0, time.UTC) | |||
| 468 | + | manifestStorage.manifests[manifestRefKey("gordon/api", "latest")] = mustManifestJSON(t, "sha256:cfg-live", "sha256:layer-live") | |||
| 469 | + | manifestStorage.manifests[manifestRefKey("gordon/api", "v2")] = mustManifestJSON(t, "sha256:cfg-v2", "sha256:layer-v2") | |||
| 470 | + | blobStorage := &fakeBlobStorage{blobs: []string{"sha256:cfg-live", "sha256:layer-live", "sha256:orphan"}} | |||
| 471 | + | rt := &fakeRuntime{pruneReport: pkgruntime.PruneReport{DeletedIDs: []string{"sha256:img1"}, SpaceReclaimed: 1024}} | |||
| 472 | + | ||||
| 473 | + | svc := NewService(rt, manifestStorage, blobStorage, zerowrap.Default()) | |||
| 474 | + | ||||
| 475 | + | // First prune - should remove v1 and orphan | |||
| 476 | + | report1, err := svc.Prune(context.Background(), domain.ImagePruneOptions{ | |||
| 477 | + | KeepLast: 1, PruneDangling: true, PruneRegistry: true, | |||
| 478 | + | }) | |||
| 479 | + | require.NoError(t, err) | |||
| 480 | + | assert.Equal(t, 1, report1.Registry.TagsRemoved, "first prune should remove v1") | |||
| 481 | + | assert.Equal(t, 1, report1.Registry.BlobsRemoved, "first prune should remove orphan") | |||
| 482 | + | ||||
| 483 | + | // Second prune - should be idempotent (nothing else to remove) | |||
| 484 | + | report2, err := svc.Prune(context.Background(), domain.ImagePruneOptions{ | |||
| 485 | + | KeepLast: 1, PruneDangling: true, PruneRegistry: true, | |||
| 486 | + | }) | |||
| 487 | + | require.NoError(t, err) | |||
| 488 | + | assert.Equal(t, 0, report2.Registry.TagsRemoved, "second prune should remove nothing (idempotent)") | |||
| 489 | + | assert.Equal(t, 0, report2.Registry.BlobsRemoved, "second prune should remove nothing (idempotent)") | |||
| 490 | + | } | |||
| 491 | + | ||||
| 461 | func TestService_Prune_RunsBothRuntimeAndRegistry(t *testing.T) { | 492 | func TestService_Prune_RunsBothRuntimeAndRegistry(t *testing.T) { | ||
| 462 | manifestStorage := newFakeManifestStorage() | 493 | manifestStorage := newFakeManifestStorage() | ||
| 463 | manifestStorage.repositories = []string{"gordon/api"} | 494 | manifestStorage.repositories = []string{"gordon/api"} | ||