diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index 62f9f963..9b0efc7c 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -290,9 +290,7 @@ func New() *Config { GC: true, GCDelay: storageConstants.DefaultGCDelay, GCInterval: storageConstants.DefaultGCInterval, - Retention: ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, - }, + Retention: ImageRetention{}, }, }, HTTP: HTTPConfig{Address: "127.0.0.1", Port: "8080", Auth: &AuthConfig{FailDelay: 0}}, diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go index 040a1dff..a1d1e7ef 100644 --- a/pkg/cli/server/root.go +++ b/pkg/cli/server/root.go @@ -724,6 +724,10 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper, logge if viperInstance.Get("storage::gcinterval") == nil { config.Storage.GCInterval = 0 } + } else if !viperInstance.IsSet("storage::retention::delay") { + // if GC is enabled, retentionDelay is set to gcDelay by default + // it could be default gcDelay or the custom value set in the config file + config.Storage.Retention.Delay = config.Storage.GCDelay } // apply deleteUntagged default @@ -802,7 +806,9 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper, logge // and retentionDelay is not set, it is set to default value if !viperInstance.IsSet("storage::subpaths::" + name + "::retention::delay") { - storageConfig.Retention.Delay = storageConstants.DefaultRetentionDelay + // retentionDelay is set to gcDelay by default + // it could be default gcDelay or the custom value set in the config file + storageConfig.Retention.Delay = storageConfig.GCDelay } // and gcInterval is not set, it is set to default value diff --git a/pkg/cli/server/root_test.go b/pkg/cli/server/root_test.go index 308dbc44..f49d8562 100644 --- a/pkg/cli/server/root_test.go +++ b/pkg/cli/server/root_test.go @@ -657,7 +657,7 @@ storage: "cve": { "updateInterval": "2h" } - }, + }, "ui": { "enable": true } @@ -693,7 +693,7 @@ storage: "cve": { "updateInterval": "2h" } - }, + }, "ui": { "enable": true } @@ -2735,3 +2735,230 @@ func runCLIWithConfig(tempDir string, config string) (string, error) { return logFile.Name(), nil } + +func TestRetentionDelayDefaults(t *testing.T) { + Convey("Test retention delay defaults to GC delay", t, func() { + Convey("When retention delay is not specified, it should default to GC delay", func() { + config := config.New() + tempDir := t.TempDir() + + // Config with GC enabled but no retention delay specified + content := []byte(`{ + "storage": { + "rootDirectory": "/tmp/zot", + "gc": true, + "gcDelay": "2h" + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + } + }`) + tmpfile := path.Join(tempDir, "config.json") + err := os.WriteFile(tmpfile, content, 0o0600) + So(err, ShouldBeNil) + + err = cli.LoadConfiguration(config, tmpfile) + So(err, ShouldBeNil) + + // Verify GC delay is set correctly + So(config.Storage.GCDelay, ShouldEqual, 2*time.Hour) + // Verify retention delay defaults to GC delay + So(config.Storage.Retention.Delay, ShouldEqual, 2*time.Hour) + }) + + Convey("When retention delay is explicitly specified, it should use that value", func() { + config := config.New() + tempDir := t.TempDir() + + // Config with explicit retention delay + content := []byte(`{ + "storage": { + "rootDirectory": "/tmp/zot", + "gc": true, + "gcDelay": "2h", + "retention": { + "delay": "3h" + } + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + } + }`) + tmpfile := path.Join(tempDir, "config.json") + err := os.WriteFile(tmpfile, content, 0o0600) + So(err, ShouldBeNil) + + err = cli.LoadConfiguration(config, tmpfile) + So(err, ShouldBeNil) + + // Verify GC delay is set correctly + So(config.Storage.GCDelay, ShouldEqual, 2*time.Hour) + // Verify retention delay uses explicit value + So(config.Storage.Retention.Delay, ShouldEqual, 3*time.Hour) + }) + + Convey("When GC is disabled, retention delay should be 0", func() { + config := config.New() + tempDir := t.TempDir() + + // Config with GC disabled + content := []byte(`{ + "storage": { + "rootDirectory": "/tmp/zot", + "gc": false + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + } + }`) + tmpfile := path.Join(tempDir, "config.json") + err := os.WriteFile(tmpfile, content, 0o0600) + So(err, ShouldBeNil) + + err = cli.LoadConfiguration(config, tmpfile) + So(err, ShouldBeNil) + + // Verify GC delay is 0 when GC is disabled + So(config.Storage.GCDelay, ShouldEqual, 0) + // Verify retention delay is 0 when GC is disabled + So(config.Storage.Retention.Delay, ShouldEqual, 0) + }) + + Convey("When GC delay is not specified, retention delay should default to default GC delay", func() { + config := config.New() + tempDir := t.TempDir() + + // Config with GC enabled but no gcDelay specified + content := []byte(`{ + "storage": { + "rootDirectory": "/tmp/zot", + "gc": true + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + } + }`) + tmpfile := path.Join(tempDir, "config.json") + err := os.WriteFile(tmpfile, content, 0o0600) + So(err, ShouldBeNil) + + err = cli.LoadConfiguration(config, tmpfile) + So(err, ShouldBeNil) + + // Verify GC delay defaults to default value + So(config.Storage.GCDelay, ShouldEqual, storageConstants.DefaultGCDelay) + // Verify retention delay defaults to default GC delay + So(config.Storage.Retention.Delay, ShouldEqual, storageConstants.DefaultGCDelay) + }) + }) + + Convey("Test subpath retention delay defaults to subpath GC delay", t, func() { + Convey("When subpath retention delay is not specified, it should default to subpath GC delay", func() { + config := config.New() + tempDir := t.TempDir() + + // Config with subpath GC enabled but no retention delay specified + content := []byte(`{ + "storage": { + "rootDirectory": "/tmp/zot", + "subPaths": { + "/a": { + "rootDirectory": "/tmp/zot-a", + "gc": true, + "gcDelay": "30m" + } + } + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + } + }`) + tmpfile := path.Join(tempDir, "config.json") + err := os.WriteFile(tmpfile, content, 0o0600) + So(err, ShouldBeNil) + + err = cli.LoadConfiguration(config, tmpfile) + So(err, ShouldBeNil) + + // Verify subpath GC delay is set correctly + So(config.Storage.SubPaths["/a"].GCDelay, ShouldEqual, 30*time.Minute) + // Verify subpath retention delay defaults to subpath GC delay + So(config.Storage.SubPaths["/a"].Retention.Delay, ShouldEqual, 30*time.Minute) + }) + + Convey("When subpath retention delay is explicitly specified, it should use that value", func() { + config := config.New() + tempDir := t.TempDir() + + // Config with explicit subpath retention delay + content := []byte(`{ + "storage": { + "rootDirectory": "/tmp/zot", + "subPaths": { + "/a": { + "rootDirectory": "/tmp/zot-a", + "gc": true, + "gcDelay": "30m", + "retention": { + "delay": "45m" + } + } + } + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + } + }`) + tmpfile := path.Join(tempDir, "config.json") + err := os.WriteFile(tmpfile, content, 0o0600) + So(err, ShouldBeNil) + + err = cli.LoadConfiguration(config, tmpfile) + So(err, ShouldBeNil) + + // Verify subpath GC delay is set correctly + So(config.Storage.SubPaths["/a"].GCDelay, ShouldEqual, 30*time.Minute) + // Verify subpath retention delay uses explicit value + So(config.Storage.SubPaths["/a"].Retention.Delay, ShouldEqual, 45*time.Minute) + }) + + Convey("When subpath GC is not specified, retention delay should default to default GC delay", func() { + config := config.New() + tempDir := t.TempDir() + + // Config with subpath but no GC settings + content := []byte(`{ + "storage": { + "rootDirectory": "/tmp/zot", + "subPaths": { + "/a": { + "rootDirectory": "/tmp/zot-a", + "gc": true + } + } + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + } + }`) + tmpfile := path.Join(tempDir, "config.json") + err := os.WriteFile(tmpfile, content, 0o0600) + So(err, ShouldBeNil) + + err = cli.LoadConfiguration(config, tmpfile) + So(err, ShouldBeNil) + + // Verify subpath GC delay defaults to default value + So(config.Storage.SubPaths["/a"].GCDelay, ShouldEqual, storageConstants.DefaultGCDelay) + // Verify subpath retention delay defaults to default GC delay + So(config.Storage.SubPaths["/a"].Retention.Delay, ShouldEqual, storageConstants.DefaultGCDelay) + }) + }) +} diff --git a/pkg/storage/constants/constants.go b/pkg/storage/constants/constants.go index 0aba30a2..14533c28 100644 --- a/pkg/storage/constants/constants.go +++ b/pkg/storage/constants/constants.go @@ -22,7 +22,6 @@ const ( RedisDriverName = "redis" RedisLocksBucket = "locks" DefaultGCDelay = 1 * time.Hour - DefaultRetentionDelay = 24 * time.Hour DefaultGCInterval = 1 * time.Hour S3StorageDriverName = "s3" LocalStorageDriverName = "local" diff --git a/pkg/storage/gc/gc_internal_test.go b/pkg/storage/gc/gc_internal_test.go index a104ba09..d92f4947 100644 --- a/pkg/storage/gc/gc_internal_test.go +++ b/pkg/storage/gc/gc_internal_test.go @@ -52,7 +52,7 @@ func TestGarbageCollectManifestErrors(t *testing.T) { gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -176,7 +176,7 @@ func TestGarbageCollectIndexErrors(t *testing.T) { gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -291,7 +291,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { gcOptions := Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -338,7 +338,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { gcOptions := Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -366,7 +366,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { gcOptions := Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, diff --git a/pkg/storage/gc/gc_test.go b/pkg/storage/gc/gc_test.go index ac59e85b..aa89e903 100644 --- a/pkg/storage/gc/gc_test.go +++ b/pkg/storage/gc/gc_test.go @@ -346,7 +346,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -765,7 +765,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -831,7 +831,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -894,7 +894,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -933,7 +933,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -971,7 +971,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -1009,7 +1009,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -1049,7 +1049,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -1134,7 +1134,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -1215,7 +1215,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: gcDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -1935,7 +1935,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -2282,7 +2282,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -2342,7 +2342,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -2397,7 +2397,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -2434,7 +2434,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -2539,7 +2539,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{ Delay: gcDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, diff --git a/pkg/storage/local/local_test.go b/pkg/storage/local/local_test.go index acd67712..00067558 100644 --- a/pkg/storage/local/local_test.go +++ b/pkg/storage/local/local_test.go @@ -48,7 +48,7 @@ const ( var trueVal bool = true //nolint: gochecknoglobals var DeleteReferrers = config.ImageRetention{ //nolint: gochecknoglobals - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index c6d26486..3ee7f463 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -48,7 +48,7 @@ import ( var trueVal bool = true //nolint: gochecknoglobals var DeleteReferrers = config.ImageRetention{ //nolint: gochecknoglobals - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"}, @@ -1812,7 +1812,7 @@ func TestGarbageCollectImageManifest(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: config.ImageRetention{ - Delay: storageConstants.DefaultRetentionDelay, + Delay: storageConstants.DefaultGCDelay, Policies: []config.RetentionPolicy{ { Repositories: []string{"**"},