fix(images): address coderabbit review feedback
- commit
- e5e723797136b3b828413b44dc52a1c4b6c8612f
- author
- brice · · 2026-02-08 07:43 UTC
- committer
- brice · · 2026-02-08 09:40 UTC
- parent
- 6381d1b
- signature
- unsigned
9 files · +47 -27
M internal/adapters/in/http/admin/handler_test.go +1 -1
| @@ -1385,7 +1385,7 @@ func TestHandler_ImagesPrune_AcceptsOptionalKeepLast(t *testing.T) { | |||
| 1385 | 1385 | body string | |
| 1386 | 1386 | expectedKeepLast int | |
| 1387 | 1387 | }{ | |
| 1388 | - | {name: "missing keep_last uses zero", body: `{}`, expectedKeepLast: 0}, | |
| 1388 | + | {name: "missing keep_last uses default", body: `{}`, expectedKeepLast: domain.DefaultImagePruneKeepLast}, | |
| 1389 | 1389 | {name: "provided keep_last", body: `{"keep_last": 3}`, expectedKeepLast: 3}, | |
| 1390 | 1390 | } | |
| 1391 | 1391 | ||
| @@ -1385,7 +1385,7 @@ func TestHandler_ImagesPrune_AcceptsOptionalKeepLast(t *testing.T) { | |||||
| 1385 | body string | 1385 | body string | ||
| 1386 | expectedKeepLast int | 1386 | expectedKeepLast int | ||
| 1387 | }{ | 1387 | }{ | ||
| 1388 | - | {name: "missing keep_last uses zero", body: `{}`, expectedKeepLast: 0}, | 1388 | + | {name: "missing keep_last uses default", body: `{}`, expectedKeepLast: domain.DefaultImagePruneKeepLast}, |
| 1389 | {name: "provided keep_last", body: `{"keep_last": 3}`, expectedKeepLast: 3}, | 1389 | {name: "provided keep_last", body: `{"keep_last": 3}`, expectedKeepLast: 3}, | ||
| 1390 | } | 1390 | } | ||
| 1391 | 1391 | ||||
M internal/adapters/in/http/admin/images.go +1 -1
| @@ -97,7 +97,7 @@ func (h *Handler) handleImagesPrune(w http.ResponseWriter, r *http.Request) { | |||
| 97 | 97 | return | |
| 98 | 98 | } | |
| 99 | 99 | ||
| 100 | - | keepLast := 0 | |
| 100 | + | keepLast := domain.DefaultImagePruneKeepLast | |
| 101 | 101 | if req.KeepLast != nil { | |
| 102 | 102 | if *req.KeepLast < 0 { | |
| 103 | 103 | h.sendError(w, http.StatusBadRequest, "keep_last must be >= 0") | |
| @@ -97,7 +97,7 @@ func (h *Handler) handleImagesPrune(w http.ResponseWriter, r *http.Request) { | |||||
| 97 | return | 97 | return | ||
| 98 | } | 98 | } | ||
| 99 | 99 | ||||
| 100 | - | keepLast := 0 | 100 | + | keepLast := domain.DefaultImagePruneKeepLast |
| 101 | if req.KeepLast != nil { | 101 | if req.KeepLast != nil { | ||
| 102 | if *req.KeepLast < 0 { | 102 | if *req.KeepLast < 0 { | ||
| 103 | h.sendError(w, http.StatusBadRequest, "keep_last must be >= 0") | 103 | h.sendError(w, http.StatusBadRequest, "keep_last must be >= 0") | ||
M internal/adapters/out/docker/runtime.go +11 -7
| @@ -40,8 +40,6 @@ type Runtime struct { | |||
| 40 | 40 | client *client.Client | |
| 41 | 41 | } | |
| 42 | 42 | ||
| 43 | - | var errSpaceReclaimedOverflow = errors.New("space reclaimed exceeds int64 range") | |
| 44 | - | ||
| 45 | 43 | type pipeReadCloser struct { | |
| 46 | 44 | pr *io.PipeReader | |
| 47 | 45 | pw *io.PipeWriter | |
| @@ -598,7 +596,9 @@ func (r *Runtime) PruneImages(ctx context.Context, danglingOnly bool) (runtimepk | |||
| 598 | 596 | log := zerowrap.FromCtx(ctx) | |
| 599 | 597 | ||
| 600 | 598 | pruneFilters := filters.NewArgs() | |
| 601 | - | pruneFilters.Add("dangling", strconv.FormatBool(danglingOnly)) | |
| 599 | + | if danglingOnly { | |
| 600 | + | pruneFilters.Add("dangling", strconv.FormatBool(true)) | |
| 601 | + | } | |
| 602 | 602 | ||
| 603 | 603 | pruneResult, err := r.client.ImagesPrune(ctx, pruneFilters) | |
| 604 | 604 | if err != nil { | |
| @@ -615,14 +615,18 @@ func (r *Runtime) PruneImages(ctx context.Context, danglingOnly bool) (runtimepk | |||
| 615 | 615 | } | |
| 616 | 616 | } | |
| 617 | 617 | ||
| 618 | - | if pruneResult.SpaceReclaimed > math.MaxInt64 { | |
| 619 | - | err := fmt.Errorf("space reclaimed %d exceeds max int64: %w", pruneResult.SpaceReclaimed, errSpaceReclaimedOverflow) | |
| 620 | - | return runtimepkg.PruneReport{}, log.WrapErr(err, "failed to map prune result") | |
| 618 | + | spaceReclaimed := pruneResult.SpaceReclaimed | |
| 619 | + | if spaceReclaimed > math.MaxInt64 { | |
| 620 | + | log.Warn(). | |
| 621 | + | Uint64("space_reclaimed_bytes", spaceReclaimed). | |
| 622 | + | Int64("space_reclaimed_capped_bytes", math.MaxInt64). | |
| 623 | + | Msg("space reclaimed exceeds int64 max; capping value") | |
| 624 | + | spaceReclaimed = uint64(math.MaxInt64) | |
| 621 | 625 | } | |
| 622 | 626 | ||
| 623 | 627 | return runtimepkg.PruneReport{ | |
| 624 | 628 | DeletedIDs: deletedIDs, | |
| 625 | - | SpaceReclaimed: int64(pruneResult.SpaceReclaimed), | |
| 629 | + | SpaceReclaimed: int64(spaceReclaimed), | |
| 626 | 630 | }, nil | |
| 627 | 631 | } | |
| 628 | 632 | ||
| @@ -40,8 +40,6 @@ type Runtime struct { | |||||
| 40 | client *client.Client | 40 | client *client.Client | ||
| 41 | } | 41 | } | ||
| 42 | 42 | ||||
| 43 | - | var errSpaceReclaimedOverflow = errors.New("space reclaimed exceeds int64 range") | |||
| 44 | - | ||||
| 45 | type pipeReadCloser struct { | 43 | type pipeReadCloser struct { | ||
| 46 | pr *io.PipeReader | 44 | pr *io.PipeReader | ||
| 47 | pw *io.PipeWriter | 45 | pw *io.PipeWriter | ||
| @@ -598,7 +596,9 @@ func (r *Runtime) PruneImages(ctx context.Context, danglingOnly bool) (runtimepk | |||||
| 598 | log := zerowrap.FromCtx(ctx) | 596 | log := zerowrap.FromCtx(ctx) | ||
| 599 | 597 | ||||
| 600 | pruneFilters := filters.NewArgs() | 598 | pruneFilters := filters.NewArgs() | ||
| 601 | - | pruneFilters.Add("dangling", strconv.FormatBool(danglingOnly)) | 599 | + | if danglingOnly { |
| 600 | + | pruneFilters.Add("dangling", strconv.FormatBool(true)) | |||
| 601 | + | } | |||
| 602 | 602 | ||||
| 603 | pruneResult, err := r.client.ImagesPrune(ctx, pruneFilters) | 603 | pruneResult, err := r.client.ImagesPrune(ctx, pruneFilters) | ||
| 604 | if err != nil { | 604 | if err != nil { | ||
| @@ -615,14 +615,18 @@ func (r *Runtime) PruneImages(ctx context.Context, danglingOnly bool) (runtimepk | |||||
| 615 | } | 615 | } | ||
| 616 | } | 616 | } | ||
| 617 | 617 | ||||
| 618 | - | if pruneResult.SpaceReclaimed > math.MaxInt64 { | 618 | + | spaceReclaimed := pruneResult.SpaceReclaimed |
| 619 | - | err := fmt.Errorf("space reclaimed %d exceeds max int64: %w", pruneResult.SpaceReclaimed, errSpaceReclaimedOverflow) | 619 | + | if spaceReclaimed > math.MaxInt64 { |
| 620 | - | return runtimepkg.PruneReport{}, log.WrapErr(err, "failed to map prune result") | 620 | + | log.Warn(). |
| 621 | + | Uint64("space_reclaimed_bytes", spaceReclaimed). | |||
| 622 | + | Int64("space_reclaimed_capped_bytes", math.MaxInt64). | |||
| 623 | + | Msg("space reclaimed exceeds int64 max; capping value") | |||
| 624 | + | spaceReclaimed = uint64(math.MaxInt64) | |||
| 621 | } | 625 | } | ||
| 622 | 626 | ||||
| 623 | return runtimepkg.PruneReport{ | 627 | return runtimepkg.PruneReport{ | ||
| 624 | DeletedIDs: deletedIDs, | 628 | DeletedIDs: deletedIDs, | ||
| 625 | - | SpaceReclaimed: int64(pruneResult.SpaceReclaimed), | 629 | + | SpaceReclaimed: int64(spaceReclaimed), |
| 626 | }, nil | 630 | }, nil | ||
| 627 | } | 631 | } | ||
| 628 | 632 | ||||
M internal/adapters/out/docker/runtime_image_cleanup_test.go +6 -7
| @@ -37,14 +37,14 @@ func TestRuntime_PruneImages_DanglingOnlyFilter(t *testing.T) { | |||
| 37 | 37 | assert.EqualValues(t, 1234, report.SpaceReclaimed) | |
| 38 | 38 | } | |
| 39 | 39 | ||
| 40 | - | func TestRuntime_PruneImages_FullUnusedFilter(t *testing.T) { | |
| 40 | + | func TestRuntime_PruneImages_FullUnusedHasNoDanglingFilter(t *testing.T) { | |
| 41 | 41 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 42 | 42 | assert.Equal(t, http.MethodPost, r.Method) | |
| 43 | 43 | assert.Equal(t, "/v1.41/images/prune", r.URL.Path) | |
| 44 | 44 | ||
| 45 | 45 | parsedFilters, err := filters.FromJSON(r.URL.Query().Get("filters")) | |
| 46 | 46 | require.NoError(t, err) | |
| 47 | - | assert.Equal(t, []string{"false"}, parsedFilters.Get("dangling")) | |
| 47 | + | assert.Empty(t, parsedFilters.Get("dangling")) | |
| 48 | 48 | ||
| 49 | 49 | w.Header().Set("Content-Type", "application/json") | |
| 50 | 50 | _, _ = w.Write([]byte(`{"ImagesDeleted":[{"Deleted":"sha256:def"}],"SpaceReclaimed":5678}`)) | |
| @@ -91,7 +91,7 @@ func TestRuntime_PruneImages_ReturnsErrorOnInvalidPayload(t *testing.T) { | |||
| 91 | 91 | require.Error(t, err) | |
| 92 | 92 | } | |
| 93 | 93 | ||
| 94 | - | func TestRuntime_PruneImages_ReturnsErrorOnSpaceReclaimedOverflow(t *testing.T) { | |
| 94 | + | func TestRuntime_PruneImages_CapsSpaceReclaimedOverflow(t *testing.T) { | |
| 95 | 95 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 96 | 96 | assert.Equal(t, http.MethodPost, r.Method) | |
| 97 | 97 | assert.Equal(t, "/v1.41/images/prune", r.URL.Path) | |
| @@ -102,11 +102,10 @@ func TestRuntime_PruneImages_ReturnsErrorOnSpaceReclaimedOverflow(t *testing.T) | |||
| 102 | 102 | defer server.Close() | |
| 103 | 103 | ||
| 104 | 104 | runtime := newRuntimeForHTTPServer(t, server) | |
| 105 | - | _, err := runtime.PruneImages(context.Background(), true) | |
| 105 | + | report, err := runtime.PruneImages(context.Background(), true) | |
| 106 | 106 | ||
| 107 | - | require.Error(t, err) | |
| 108 | - | assert.ErrorContains(t, err, "space reclaimed") | |
| 109 | - | assert.ErrorContains(t, err, "int64") | |
| 107 | + | require.NoError(t, err) | |
| 108 | + | assert.EqualValues(t, math.MaxInt64, report.SpaceReclaimed) | |
| 110 | 109 | } | |
| 111 | 110 | ||
| 112 | 111 | func TestRuntime_ListImagesDetailed_MapsImageSummary(t *testing.T) { | |
| @@ -37,14 +37,14 @@ func TestRuntime_PruneImages_DanglingOnlyFilter(t *testing.T) { | |||||
| 37 | assert.EqualValues(t, 1234, report.SpaceReclaimed) | 37 | assert.EqualValues(t, 1234, report.SpaceReclaimed) | ||
| 38 | } | 38 | } | ||
| 39 | 39 | ||||
| 40 | - | func TestRuntime_PruneImages_FullUnusedFilter(t *testing.T) { | 40 | + | func TestRuntime_PruneImages_FullUnusedHasNoDanglingFilter(t *testing.T) { |
| 41 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 41 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 42 | assert.Equal(t, http.MethodPost, r.Method) | 42 | assert.Equal(t, http.MethodPost, r.Method) | ||
| 43 | assert.Equal(t, "/v1.41/images/prune", r.URL.Path) | 43 | assert.Equal(t, "/v1.41/images/prune", r.URL.Path) | ||
| 44 | 44 | ||||
| 45 | parsedFilters, err := filters.FromJSON(r.URL.Query().Get("filters")) | 45 | parsedFilters, err := filters.FromJSON(r.URL.Query().Get("filters")) | ||
| 46 | require.NoError(t, err) | 46 | require.NoError(t, err) | ||
| 47 | - | assert.Equal(t, []string{"false"}, parsedFilters.Get("dangling")) | 47 | + | assert.Empty(t, parsedFilters.Get("dangling")) |
| 48 | 48 | ||||
| 49 | w.Header().Set("Content-Type", "application/json") | 49 | w.Header().Set("Content-Type", "application/json") | ||
| 50 | _, _ = w.Write([]byte(`{"ImagesDeleted":[{"Deleted":"sha256:def"}],"SpaceReclaimed":5678}`)) | 50 | _, _ = w.Write([]byte(`{"ImagesDeleted":[{"Deleted":"sha256:def"}],"SpaceReclaimed":5678}`)) | ||
| @@ -91,7 +91,7 @@ func TestRuntime_PruneImages_ReturnsErrorOnInvalidPayload(t *testing.T) { | |||||
| 91 | require.Error(t, err) | 91 | require.Error(t, err) | ||
| 92 | } | 92 | } | ||
| 93 | 93 | ||||
| 94 | - | func TestRuntime_PruneImages_ReturnsErrorOnSpaceReclaimedOverflow(t *testing.T) { | 94 | + | func TestRuntime_PruneImages_CapsSpaceReclaimedOverflow(t *testing.T) { |
| 95 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 95 | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 96 | assert.Equal(t, http.MethodPost, r.Method) | 96 | assert.Equal(t, http.MethodPost, r.Method) | ||
| 97 | assert.Equal(t, "/v1.41/images/prune", r.URL.Path) | 97 | assert.Equal(t, "/v1.41/images/prune", r.URL.Path) | ||
| @@ -102,11 +102,10 @@ func TestRuntime_PruneImages_ReturnsErrorOnSpaceReclaimedOverflow(t *testing.T) | |||||
| 102 | defer server.Close() | 102 | defer server.Close() | ||
| 103 | 103 | ||||
| 104 | runtime := newRuntimeForHTTPServer(t, server) | 104 | runtime := newRuntimeForHTTPServer(t, server) | ||
| 105 | - | _, err := runtime.PruneImages(context.Background(), true) | 105 | + | report, err := runtime.PruneImages(context.Background(), true) |
| 106 | 106 | ||||
| 107 | - | require.Error(t, err) | 107 | + | require.NoError(t, err) |
| 108 | - | assert.ErrorContains(t, err, "space reclaimed") | 108 | + | assert.EqualValues(t, math.MaxInt64, report.SpaceReclaimed) |
| 109 | - | assert.ErrorContains(t, err, "int64") | |||
| 110 | } | 109 | } | ||
| 111 | 110 | ||||
| 112 | func TestRuntime_ListImagesDetailed_MapsImageSummary(t *testing.T) { | 111 | func TestRuntime_ListImagesDetailed_MapsImageSummary(t *testing.T) { | ||
M internal/app/run.go +23 -8
| @@ -232,7 +232,7 @@ func Run(ctx context.Context, configPath string) error { | |||
| 232 | 232 | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | |
| 233 | 233 | ||
| 234 | 234 | // Start servers, wait for listeners to bind, then sync/auto-start containers. | |
| 235 | - | return runServers(ctx, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, svc, log) | |
| 235 | + | return runServers(ctx, v, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, svc, log) | |
| 236 | 236 | } | |
| 237 | 237 | ||
| 238 | 238 | func ensureTLSConfig(cfg *Config, log zerowrap.Logger) error { | |
| @@ -1382,7 +1382,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||
| 1382 | 1382 | // - SIGUSR2: Triggers manual deploy for a specific route | |
| 1383 | 1383 | // The deferred signal.Stop calls ensure signal handlers are properly | |
| 1384 | 1384 | // cleaned up before program exit, preventing signal handler leaks. | |
| 1385 | - | func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, svc *services, log zerowrap.Logger) error { | |
| 1385 | + | func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, svc *services, log zerowrap.Logger) error { | |
| 1386 | 1386 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | |
| 1387 | 1387 | defer cancel() | |
| 1388 | 1388 | ||
| @@ -1446,7 +1446,9 @@ func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler h | |||
| 1446 | 1446 | } | |
| 1447 | 1447 | logEvent.Msg("Gordon is running") | |
| 1448 | 1448 | ||
| 1449 | - | imageScheduler, err := startImagePruneScheduler(ctx, cfg, svc, log) | |
| 1449 | + | imageScheduler, err := startImagePruneScheduler(ctx, cfg, svc, log, func() int { | |
| 1450 | + | return v.GetInt("images.prune.keep_last") | |
| 1451 | + | }) | |
| 1450 | 1452 | if err != nil { | |
| 1451 | 1453 | return err | |
| 1452 | 1454 | } | |
| @@ -1462,11 +1464,14 @@ func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler h | |||
| 1462 | 1464 | return nil | |
| 1463 | 1465 | } | |
| 1464 | 1466 | ||
| 1465 | - | func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, log zerowrap.Logger) (*cronSvc.Scheduler, error) { | |
| 1467 | + | func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, log zerowrap.Logger, keepLastGetter func() int) (*cronSvc.Scheduler, error) { | |
| 1466 | 1468 | if !cfg.Images.Prune.Enabled || svc == nil || svc.imageSvc == nil { | |
| 1467 | 1469 | return nil, nil | |
| 1468 | 1470 | } | |
| 1469 | - | if cfg.Images.Prune.KeepLast < 0 { | |
| 1471 | + | if keepLastGetter == nil { | |
| 1472 | + | keepLastGetter = func() int { return cfg.Images.Prune.KeepLast } | |
| 1473 | + | } | |
| 1474 | + | if keepLastGetter() < 0 { | |
| 1470 | 1475 | return nil, fmt.Errorf("images.prune.keep_last must be >= 0") | |
| 1471 | 1476 | } | |
| 1472 | 1477 | ||
| @@ -1481,12 +1486,22 @@ func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, lo | |||
| 1481 | 1486 | "Image prune", | |
| 1482 | 1487 | domain.CronSchedule{Preset: preset}, | |
| 1483 | 1488 | func(jobCtx context.Context) error { | |
| 1484 | - | report, err := svc.imageSvc.Prune(jobCtx, cfg.Images.Prune.KeepLast) | |
| 1489 | + | keepLast := keepLastGetter() | |
| 1490 | + | if keepLast < 0 { | |
| 1491 | + | log.Warn(). | |
| 1492 | + | Int("configured_keep_last", keepLast). | |
| 1493 | + | Int("fallback_keep_last", domain.DefaultImagePruneKeepLast). | |
| 1494 | + | Msg("invalid images.prune.keep_last; using default") | |
| 1495 | + | keepLast = domain.DefaultImagePruneKeepLast | |
| 1496 | + | } | |
| 1497 | + | ||
| 1498 | + | report, err := svc.imageSvc.Prune(jobCtx, keepLast) | |
| 1485 | 1499 | if err != nil { | |
| 1486 | 1500 | return err | |
| 1487 | 1501 | } | |
| 1488 | 1502 | ||
| 1489 | 1503 | log.Info(). | |
| 1504 | + | Int("keep_last", keepLast). | |
| 1490 | 1505 | Int("runtime_deleted", report.Runtime.DeletedCount). | |
| 1491 | 1506 | Int64("runtime_reclaimed_bytes", report.Runtime.SpaceReclaimed). | |
| 1492 | 1507 | Int("registry_tags_removed", report.Registry.TagsRemoved). | |
| @@ -1502,7 +1517,7 @@ func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, lo | |||
| 1502 | 1517 | scheduler.Start(ctx) | |
| 1503 | 1518 | log.Info(). | |
| 1504 | 1519 | Str("schedule", string(preset)). | |
| 1505 | - | Int("keep_last", cfg.Images.Prune.KeepLast). | |
| 1520 | + | Int("keep_last", keepLastGetter()). | |
| 1506 | 1521 | Msg("image prune scheduler enabled") | |
| 1507 | 1522 | ||
| 1508 | 1523 | return scheduler, nil | |
| @@ -1943,7 +1958,7 @@ func loadConfig(v *viper.Viper, configPath string) error { | |||
| 1943 | 1958 | v.SetDefault("backups.retention.monthly", 0) | |
| 1944 | 1959 | v.SetDefault("images.prune.enabled", false) | |
| 1945 | 1960 | v.SetDefault("images.prune.schedule", string(domain.ScheduleDaily)) | |
| 1946 | - | v.SetDefault("images.prune.keep_last", 3) | |
| 1961 | + | v.SetDefault("images.prune.keep_last", domain.DefaultImagePruneKeepLast) | |
| 1947 | 1962 | v.SetDefault("server.max_concurrent_connections", -1) // -1 = use default (10000), 0 = no limit | |
| 1948 | 1963 | v.SetDefault("server.registry_allowed_ips", []string{}) | |
| 1949 | 1964 | v.SetDefault("deploy.readiness_delay", "5s") | |
| @@ -232,7 +232,7 @@ func Run(ctx context.Context, configPath string) error { | |||||
| 232 | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | 232 | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | ||
| 233 | 233 | ||||
| 234 | // Start servers, wait for listeners to bind, then sync/auto-start containers. | 234 | // Start servers, wait for listeners to bind, then sync/auto-start containers. | ||
| 235 | - | return runServers(ctx, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, svc, log) | 235 | + | return runServers(ctx, v, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, svc, log) |
| 236 | } | 236 | } | ||
| 237 | 237 | ||||
| 238 | func ensureTLSConfig(cfg *Config, log zerowrap.Logger) error { | 238 | func ensureTLSConfig(cfg *Config, log zerowrap.Logger) error { | ||
| @@ -1382,7 +1382,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||||
| 1382 | // - SIGUSR2: Triggers manual deploy for a specific route | 1382 | // - SIGUSR2: Triggers manual deploy for a specific route | ||
| 1383 | // The deferred signal.Stop calls ensure signal handlers are properly | 1383 | // The deferred signal.Stop calls ensure signal handlers are properly | ||
| 1384 | // cleaned up before program exit, preventing signal handler leaks. | 1384 | // cleaned up before program exit, preventing signal handler leaks. | ||
| 1385 | - | func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, svc *services, log zerowrap.Logger) error { | 1385 | + | func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, svc *services, log zerowrap.Logger) error { |
| 1386 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | 1386 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | ||
| 1387 | defer cancel() | 1387 | defer cancel() | ||
| 1388 | 1388 | ||||
| @@ -1446,7 +1446,9 @@ func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler h | |||||
| 1446 | } | 1446 | } | ||
| 1447 | logEvent.Msg("Gordon is running") | 1447 | logEvent.Msg("Gordon is running") | ||
| 1448 | 1448 | ||||
| 1449 | - | imageScheduler, err := startImagePruneScheduler(ctx, cfg, svc, log) | 1449 | + | imageScheduler, err := startImagePruneScheduler(ctx, cfg, svc, log, func() int { |
| 1450 | + | return v.GetInt("images.prune.keep_last") | |||
| 1451 | + | }) | |||
| 1450 | if err != nil { | 1452 | if err != nil { | ||
| 1451 | return err | 1453 | return err | ||
| 1452 | } | 1454 | } | ||
| @@ -1462,11 +1464,14 @@ func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler h | |||||
| 1462 | return nil | 1464 | return nil | ||
| 1463 | } | 1465 | } | ||
| 1464 | 1466 | ||||
| 1465 | - | func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, log zerowrap.Logger) (*cronSvc.Scheduler, error) { | 1467 | + | func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, log zerowrap.Logger, keepLastGetter func() int) (*cronSvc.Scheduler, error) { |
| 1466 | if !cfg.Images.Prune.Enabled || svc == nil || svc.imageSvc == nil { | 1468 | if !cfg.Images.Prune.Enabled || svc == nil || svc.imageSvc == nil { | ||
| 1467 | return nil, nil | 1469 | return nil, nil | ||
| 1468 | } | 1470 | } | ||
| 1469 | - | if cfg.Images.Prune.KeepLast < 0 { | 1471 | + | if keepLastGetter == nil { |
| 1472 | + | keepLastGetter = func() int { return cfg.Images.Prune.KeepLast } | |||
| 1473 | + | } | |||
| 1474 | + | if keepLastGetter() < 0 { | |||
| 1470 | return nil, fmt.Errorf("images.prune.keep_last must be >= 0") | 1475 | return nil, fmt.Errorf("images.prune.keep_last must be >= 0") | ||
| 1471 | } | 1476 | } | ||
| 1472 | 1477 | ||||
| @@ -1481,12 +1486,22 @@ func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, lo | |||||
| 1481 | "Image prune", | 1486 | "Image prune", | ||
| 1482 | domain.CronSchedule{Preset: preset}, | 1487 | domain.CronSchedule{Preset: preset}, | ||
| 1483 | func(jobCtx context.Context) error { | 1488 | func(jobCtx context.Context) error { | ||
| 1484 | - | report, err := svc.imageSvc.Prune(jobCtx, cfg.Images.Prune.KeepLast) | 1489 | + | keepLast := keepLastGetter() |
| 1490 | + | if keepLast < 0 { | |||
| 1491 | + | log.Warn(). | |||
| 1492 | + | Int("configured_keep_last", keepLast). | |||
| 1493 | + | Int("fallback_keep_last", domain.DefaultImagePruneKeepLast). | |||
| 1494 | + | Msg("invalid images.prune.keep_last; using default") | |||
| 1495 | + | keepLast = domain.DefaultImagePruneKeepLast | |||
| 1496 | + | } | |||
| 1497 | + | ||||
| 1498 | + | report, err := svc.imageSvc.Prune(jobCtx, keepLast) | |||
| 1485 | if err != nil { | 1499 | if err != nil { | ||
| 1486 | return err | 1500 | return err | ||
| 1487 | } | 1501 | } | ||
| 1488 | 1502 | ||||
| 1489 | log.Info(). | 1503 | log.Info(). | ||
| 1504 | + | Int("keep_last", keepLast). | |||
| 1490 | Int("runtime_deleted", report.Runtime.DeletedCount). | 1505 | Int("runtime_deleted", report.Runtime.DeletedCount). | ||
| 1491 | Int64("runtime_reclaimed_bytes", report.Runtime.SpaceReclaimed). | 1506 | Int64("runtime_reclaimed_bytes", report.Runtime.SpaceReclaimed). | ||
| 1492 | Int("registry_tags_removed", report.Registry.TagsRemoved). | 1507 | Int("registry_tags_removed", report.Registry.TagsRemoved). | ||
| @@ -1502,7 +1517,7 @@ func startImagePruneScheduler(ctx context.Context, cfg Config, svc *services, lo | |||||
| 1502 | scheduler.Start(ctx) | 1517 | scheduler.Start(ctx) | ||
| 1503 | log.Info(). | 1518 | log.Info(). | ||
| 1504 | Str("schedule", string(preset)). | 1519 | Str("schedule", string(preset)). | ||
| 1505 | - | Int("keep_last", cfg.Images.Prune.KeepLast). | 1520 | + | Int("keep_last", keepLastGetter()). |
| 1506 | Msg("image prune scheduler enabled") | 1521 | Msg("image prune scheduler enabled") | ||
| 1507 | 1522 | ||||
| 1508 | return scheduler, nil | 1523 | return scheduler, nil | ||
| @@ -1943,7 +1958,7 @@ func loadConfig(v *viper.Viper, configPath string) error { | |||||
| 1943 | v.SetDefault("backups.retention.monthly", 0) | 1958 | v.SetDefault("backups.retention.monthly", 0) | ||
| 1944 | v.SetDefault("images.prune.enabled", false) | 1959 | v.SetDefault("images.prune.enabled", false) | ||
| 1945 | v.SetDefault("images.prune.schedule", string(domain.ScheduleDaily)) | 1960 | v.SetDefault("images.prune.schedule", string(domain.ScheduleDaily)) | ||
| 1946 | - | v.SetDefault("images.prune.keep_last", 3) | 1961 | + | v.SetDefault("images.prune.keep_last", domain.DefaultImagePruneKeepLast) |
| 1947 | v.SetDefault("server.max_concurrent_connections", -1) // -1 = use default (10000), 0 = no limit | 1962 | v.SetDefault("server.max_concurrent_connections", -1) // -1 = use default (10000), 0 = no limit | ||
| 1948 | v.SetDefault("server.registry_allowed_ips", []string{}) | 1963 | v.SetDefault("server.registry_allowed_ips", []string{}) | ||
| 1949 | v.SetDefault("deploy.readiness_delay", "5s") | 1964 | v.SetDefault("deploy.readiness_delay", "5s") | ||
M internal/app/run_images_test.go +1 -1
| @@ -17,7 +17,7 @@ func TestLoadConfigSetsImagePruneDefaults(t *testing.T) { | |||
| 17 | 17 | ||
| 18 | 18 | assert.False(t, v.GetBool("images.prune.enabled")) | |
| 19 | 19 | assert.Equal(t, string(domain.ScheduleDaily), v.GetString("images.prune.schedule")) | |
| 20 | - | assert.Equal(t, 3, v.GetInt("images.prune.keep_last")) | |
| 20 | + | assert.Equal(t, domain.DefaultImagePruneKeepLast, v.GetInt("images.prune.keep_last")) | |
| 21 | 21 | } | |
| 22 | 22 | ||
| 23 | 23 | func TestResolveImagePruneSchedule(t *testing.T) { | |
| @@ -17,7 +17,7 @@ func TestLoadConfigSetsImagePruneDefaults(t *testing.T) { | |||||
| 17 | 17 | ||||
| 18 | assert.False(t, v.GetBool("images.prune.enabled")) | 18 | assert.False(t, v.GetBool("images.prune.enabled")) | ||
| 19 | assert.Equal(t, string(domain.ScheduleDaily), v.GetString("images.prune.schedule")) | 19 | assert.Equal(t, string(domain.ScheduleDaily), v.GetString("images.prune.schedule")) | ||
| 20 | - | assert.Equal(t, 3, v.GetInt("images.prune.keep_last")) | 20 | + | assert.Equal(t, domain.DefaultImagePruneKeepLast, v.GetInt("images.prune.keep_last")) |
| 21 | } | 21 | } | ||
| 22 | 22 | ||||
| 23 | func TestResolveImagePruneSchedule(t *testing.T) { | 23 | func TestResolveImagePruneSchedule(t *testing.T) { | ||
M internal/domain/images.go +2 -0
| @@ -2,6 +2,8 @@ package domain | |||
| 2 | 2 | ||
| 3 | 3 | import "time" | |
| 4 | 4 | ||
| 5 | + | const DefaultImagePruneKeepLast = 3 | |
| 6 | + | ||
| 5 | 7 | // ImagePruneConfig defines scheduled image cleanup behavior. | |
| 6 | 8 | type ImagePruneConfig struct { | |
| 7 | 9 | Enabled bool | |
| @@ -2,6 +2,8 @@ package domain | |||||
| 2 | 2 | ||||
| 3 | import "time" | 3 | import "time" | ||
| 4 | 4 | ||||
| 5 | + | const DefaultImagePruneKeepLast = 3 | |||
| 6 | + | ||||
| 5 | // ImagePruneConfig defines scheduled image cleanup behavior. | 7 | // ImagePruneConfig defines scheduled image cleanup behavior. | ||
| 6 | type ImagePruneConfig struct { | 8 | type ImagePruneConfig struct { | ||
| 7 | Enabled bool | 9 | Enabled bool | ||
M internal/usecase/images/service.go +2 -0
| @@ -111,6 +111,8 @@ func (s *Service) PruneRuntime(ctx context.Context) (domain.ImagePruneReport, er | |||
| 111 | 111 | } | |
| 112 | 112 | ||
| 113 | 113 | // PruneRegistry applies tag retention and blob garbage collection. | |
| 114 | + | // It keeps the most recent keepLast tags from tagInfos and always keeps | |
| 115 | + | // the "latest" tag when present, so up to keepLast+1 tags can be retained. | |
| 114 | 116 | func (s *Service) PruneRegistry(ctx context.Context, keepLast int) (domain.ImagePruneReport, error) { | |
| 115 | 117 | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 116 | 118 | zerowrap.FieldLayer: "usecase", | |
| @@ -111,6 +111,8 @@ func (s *Service) PruneRuntime(ctx context.Context) (domain.ImagePruneReport, er | |||||
| 111 | } | 111 | } | ||
| 112 | 112 | ||||
| 113 | // PruneRegistry applies tag retention and blob garbage collection. | 113 | // PruneRegistry applies tag retention and blob garbage collection. | ||
| 114 | + | // It keeps the most recent keepLast tags from tagInfos and always keeps | |||
| 115 | + | // the "latest" tag when present, so up to keepLast+1 tags can be retained. | |||
| 114 | func (s *Service) PruneRegistry(ctx context.Context, keepLast int) (domain.ImagePruneReport, error) { | 116 | func (s *Service) PruneRegistry(ctx context.Context, keepLast int) (domain.ImagePruneReport, error) { | ||
| 115 | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | 117 | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | ||
| 116 | zerowrap.FieldLayer: "usecase", | 118 | zerowrap.FieldLayer: "usecase", | ||
M internal/usecase/images/service_test.go +0 -2
| @@ -4,7 +4,6 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | 6 | "errors" | |
| 7 | - | "sort" | |
| 8 | 7 | "testing" | |
| 9 | 8 | "time" | |
| 10 | 9 | ||
| @@ -612,7 +611,6 @@ func (f *fakeBlobStorage) DeleteBlob(digest string) error { | |||
| 612 | 611 | f.blobs = append(f.blobs[:i], f.blobs[i+1:]...) | |
| 613 | 612 | break | |
| 614 | 613 | } | |
| 615 | - | sort.Strings(f.deletedBlobs) | |
| 616 | 614 | return nil | |
| 617 | 615 | } | |
| 618 | 616 | ||
| @@ -4,7 +4,6 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | "errors" | 6 | "errors" | ||
| 7 | - | "sort" | |||
| 8 | "testing" | 7 | "testing" | ||
| 9 | "time" | 8 | "time" | ||
| 10 | 9 | ||||
| @@ -612,7 +611,6 @@ func (f *fakeBlobStorage) DeleteBlob(digest string) error { | |||||
| 612 | f.blobs = append(f.blobs[:i], f.blobs[i+1:]...) | 611 | f.blobs = append(f.blobs[:i], f.blobs[i+1:]...) | ||
| 613 | break | 612 | break | ||
| 614 | } | 613 | } | ||
| 615 | - | sort.Strings(f.deletedBlobs) | |||
| 616 | return nil | 614 | return nil | ||
| 617 | } | 615 | } | ||
| 618 | 616 | ||||