fix(gc): sync repodb when gc'ing manifests (#1819)

fix(gc): fix cleaning deduped blobs because they have the modTime of the original blobs, fixed by updating the modTime when hard linking the blobs. fix(gc): failing to parse rootDir at zot startup when using s3 storage because there are no files under rootDir and we can not create empty dirs on s3, fixed by creating an empty file under rootDir. Signed-off-by: Petu Eusebiu <[email protected]>
project-zot · Sep 22, 2023 · 1df743f · 1df743f
1 parent 7c78f80
commit 1df743f
Show file tree

Hide file tree

Showing 39 changed files with 2,050 additions and 1,266 deletions.
diff --git a/pkg/api/controller.go b/pkg/api/controller.go
@@ -29,6 +29,7 @@ import (
 	mTypes "zotregistry.io/zot/pkg/meta/types"
 	"zotregistry.io/zot/pkg/scheduler"
 	"zotregistry.io/zot/pkg/storage"
+	"zotregistry.io/zot/pkg/storage/gc"
 )
 
 const (
@@ -338,7 +339,13 @@ func (c *Controller) StartBackgroundTasks(reloadCtx context.Context) {
 
 	// Enable running garbage-collect periodically for DefaultStore
 	if c.Config.Storage.GC {
-		c.StoreController.DefaultStore.RunGCPeriodically(c.Config.Storage.GCInterval, taskScheduler)
+		gc := gc.NewGarbageCollect(c.StoreController.DefaultStore, c.MetaDB, gc.Options{
+			Referrers:      c.Config.Storage.GCReferrers,
+			Delay:          c.Config.Storage.GCDelay,
+			RetentionDelay: c.Config.Storage.UntaggedImageRetentionDelay,
+		}, c.Log)
+
+		gc.CleanImageStorePeriodically(c.Config.Storage.GCInterval, taskScheduler)
 	}
 
 	// Enable running dedupe blobs both ways (dedupe or restore deduped blobs)
@@ -354,7 +361,14 @@ func (c *Controller) StartBackgroundTasks(reloadCtx context.Context) {
 		for route, storageConfig := range c.Config.Storage.SubPaths {
 			// Enable running garbage-collect periodically for subImageStore
 			if storageConfig.GC {
-				c.StoreController.SubStore[route].RunGCPeriodically(storageConfig.GCInterval, taskScheduler)
+				gc := gc.NewGarbageCollect(c.StoreController.SubStore[route], c.MetaDB,
+					gc.Options{
+						Referrers:      storageConfig.GCReferrers,
+						Delay:          storageConfig.GCDelay,
+						RetentionDelay: storageConfig.UntaggedImageRetentionDelay,
+					}, c.Log)
+
+				gc.CleanImageStorePeriodically(storageConfig.GCInterval, taskScheduler)
 			}
 
 			// Enable extensions if extension config is provided for subImageStore

diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go
@@ -60,6 +60,7 @@ import (
 	"zotregistry.io/zot/pkg/meta"
 	"zotregistry.io/zot/pkg/storage"
 	storageConstants "zotregistry.io/zot/pkg/storage/constants"
+	"zotregistry.io/zot/pkg/storage/gc"
 	"zotregistry.io/zot/pkg/test"
 	testc "zotregistry.io/zot/pkg/test/common"
 	. "zotregistry.io/zot/pkg/test/image-utils"
@@ -2338,11 +2339,9 @@ func TestBearerAuthWrongAuthorizer(t *testing.T) {
 			},
 		}
 		ctlr := makeController(conf, t.TempDir())
-		cm := test.NewControllerManager(ctlr)
 
 		So(func() {
-			ctx := context.Background()
-			cm.RunServer(ctx)
+			api.AuthHandler(ctlr)
 		}, ShouldPanic)
 	})
 }
@@ -7665,7 +7664,7 @@ func TestInjectTooManyOpenFiles(t *testing.T) {
 	})
 }
 
-func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
+func TestGCSignaturesAndUntaggedManifestsWithMetaDB(t *testing.T) {
 	Convey("Make controller", t, func() {
 		Convey("Garbage collect signatures without subject and manifests without tags", func(c C) {
 			repoName := "testrepo" //nolint:goconst
@@ -7676,6 +7675,16 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 			conf := config.New()
 			conf.HTTP.Port = port
 
+			value := true
+			searchConfig := &extconf.SearchConfig{
+				BaseConfig: extconf.BaseConfig{Enable: &value},
+			}
+
+			// added search extensions so that metaDB is initialized and its tested in GC logic
+			conf.Extensions = &extconf.ExtensionConfig{
+				Search: searchConfig,
+			}
+
 			ctlr := makeController(conf, t.TempDir())
 
 			dir := t.TempDir()
@@ -7686,15 +7695,24 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 
 			ctlr.Config.Storage.Dedupe = false
 
-			err := test.WriteImageToFileSystem(CreateDefaultImage(), repoName, tag,
-				test.GetDefaultStoreController(dir, ctlr.Log))
-			So(err, ShouldBeNil)
-
 			cm := test.NewControllerManager(ctlr)
 			cm.StartServer()
 			cm.WaitServerToBeReady(port)
 			defer cm.StopServer()
 
+			img := CreateDefaultImage()
+
+			err := UploadImage(img, baseURL, repoName, tag)
+			So(err, ShouldBeNil)
+
+			gc := gc.NewGarbageCollect(ctlr.StoreController.DefaultStore, ctlr.MetaDB,
+				gc.Options{
+					Referrers:      ctlr.Config.Storage.GCReferrers,
+					Delay:          ctlr.Config.Storage.GCDelay,
+					RetentionDelay: ctlr.Config.Storage.UntaggedImageRetentionDelay,
+				},
+				ctlr.Log)
+
 			resp, err := resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, tag))
 			So(err, ShouldBeNil)
 			So(resp.StatusCode(), ShouldEqual, http.StatusOK)
@@ -7748,6 +7766,9 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 			So(err, ShouldBeNil)
 			So(resp.StatusCode(), ShouldEqual, http.StatusOK)
 
+			cosignDigest := resp.Header().Get(constants.DistContentDigestKey)
+			So(cosignDigest, ShouldNotBeEmpty)
+
 			// get notation signature manifest
 			resp, err = resty.R().SetQueryParam("artifactType", notreg.ArtifactTypeNotation).Get(
 				fmt.Sprintf("%s/v2/%s/referrers/%s", baseURL, repoName, digest.String()))
@@ -7760,6 +7781,20 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 			So(err, ShouldBeNil)
 			So(len(index.Manifests), ShouldEqual, 1)
 
+			// shouldn't do anything
+			err = gc.CleanRepo(repoName)
+			So(err, ShouldBeNil)
+
+			// make sure both signatures are stored in repodb
+			repoMeta, err := ctlr.MetaDB.GetRepoMeta(repoName)
+			So(err, ShouldBeNil)
+
+			sigMeta := repoMeta.Signatures[digest.String()]
+			So(len(sigMeta[storage.CosignType]), ShouldEqual, 1)
+			So(len(sigMeta[storage.NotationType]), ShouldEqual, 1)
+			So(sigMeta[storage.CosignType][0].SignatureManifestDigest, ShouldEqual, cosignDigest)
+			So(sigMeta[storage.NotationType][0].SignatureManifestDigest, ShouldEqual, index.Manifests[0].Digest.String())
+
 			Convey("Trigger gcNotationSignatures() error", func() {
 				var refs ispec.Index
 				err = json.Unmarshal(resp.Body(), &refs)
@@ -7773,7 +7808,7 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 				err = UploadImage(img, baseURL, repoName, img.DigestStr())
 				So(err, ShouldBeNil)
 
-				err = ctlr.StoreController.DefaultStore.RunGCRepo(repoName)
+				err = gc.CleanRepo(repoName)
 				So(err, ShouldNotBeNil)
 
 				err = os.Chmod(path.Join(dir, repoName, "blobs", "sha256", refs.Manifests[0].Digest.Encoded()), 0o755)
@@ -7787,7 +7822,7 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 				err = UploadImage(img, baseURL, repoName, tag)
 				So(err, ShouldBeNil)
 
-				err = ctlr.StoreController.DefaultStore.RunGCRepo(repoName)
+				err = gc.CleanRepo(repoName)
 				So(err, ShouldNotBeNil)
 
 				err = os.WriteFile(path.Join(dir, repoName, "blobs", "sha256", refs.Manifests[0].Digest.Encoded()), content, 0o600)
@@ -7811,6 +7846,17 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 					}, baseURL, repoName, untaggedManifestDigest.String())
 				So(err, ShouldBeNil)
 
+				// make sure repoDB reference was added
+				repoMeta, err := ctlr.MetaDB.GetRepoMeta(repoName)
+				So(err, ShouldBeNil)
+
+				_, ok := repoMeta.Referrers[untaggedManifestDigest.String()]
+				So(ok, ShouldBeTrue)
+				_, ok = repoMeta.Signatures[untaggedManifestDigest.String()]
+				So(ok, ShouldBeTrue)
+				_, ok = repoMeta.Statistics[untaggedManifestDigest.String()]
+				So(ok, ShouldBeTrue)
+
 				// overwrite image so that signatures will get invalidated and gc'ed
 				cfg, layers, manifest, err = test.GetImageComponents(3) //nolint:staticcheck
 				So(err, ShouldBeNil)
@@ -7827,9 +7873,24 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 				So(err, ShouldBeNil)
 				newManifestDigest := godigest.FromBytes(manifestBuf)
 
-				err = ctlr.StoreController.DefaultStore.RunGCRepo(repoName)
+				err = gc.CleanRepo(repoName)
+				So(err, ShouldBeNil)
+
+				// make sure both signatures are removed from repodb and repo reference for untagged is removed
+				repoMeta, err = ctlr.MetaDB.GetRepoMeta(repoName)
 				So(err, ShouldBeNil)
 
+				sigMeta := repoMeta.Signatures[digest.String()]
+				So(len(sigMeta[storage.CosignType]), ShouldEqual, 0)
+				So(len(sigMeta[storage.NotationType]), ShouldEqual, 0)
+
+				_, ok = repoMeta.Referrers[untaggedManifestDigest.String()]
+				So(ok, ShouldBeFalse)
+				_, ok = repoMeta.Signatures[untaggedManifestDigest.String()]
+				So(ok, ShouldBeFalse)
+				_, ok = repoMeta.Statistics[untaggedManifestDigest.String()]
+				So(ok, ShouldBeFalse)
+
 				// both signatures should be gc'ed
 				resp, err = resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, cosignTag))
 				So(err, ShouldBeNil)
@@ -7885,6 +7946,13 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 			cm.StartAndWait(port)
 			defer cm.StopServer()
 
+			gc := gc.NewGarbageCollect(ctlr.StoreController.DefaultStore, ctlr.MetaDB,
+				gc.Options{
+					Referrers:      ctlr.Config.Storage.GCReferrers,
+					Delay:          ctlr.Config.Storage.GCDelay,
+					RetentionDelay: ctlr.Config.Storage.UntaggedImageRetentionDelay,
+				}, ctlr.Log)
+
 			resp, err := resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, tag))
 			So(err, ShouldBeNil)
 			So(resp.StatusCode(), ShouldEqual, http.StatusOK)
@@ -7934,7 +8002,7 @@ func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
 			So(err, ShouldBeNil)
 			So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
 
-			err = ctlr.StoreController.DefaultStore.RunGCRepo(repoName)
+			err = gc.CleanRepo(repoName)
 			So(err, ShouldBeNil)
 
 			resp, err = resty.R().SetHeader("Content-Type", ispec.MediaTypeImageIndex).

diff --git a/pkg/cli/client/cve_cmd_internal_test.go b/pkg/cli/client/cve_cmd_internal_test.go
@@ -470,7 +470,7 @@ func TestNegativeServerResponse(t *testing.T) {
 
 		dir := t.TempDir()
 
-		imageStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imageStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), nil, nil)
 
 		storeController := storage.StoreController{

diff --git a/pkg/extensions/extension_image_trust_test.go b/pkg/extensions/extension_image_trust_test.go
@@ -195,7 +195,7 @@ func RunSignatureUploadAndVerificationTests(t *testing.T, cacheDriverParams map[
 		writers := io.MultiWriter(os.Stdout, logFile)
 		logger.Logger = logger.Output(writers)
 
-		imageStore := local.NewImageStore(globalDir, false, false, 0, 0, false, false,
+		imageStore := local.NewImageStore(globalDir, false, false,
 			logger, monitoring.NewMetricsServer(false, logger), nil, nil)
 
 		storeController := storage.StoreController{
@@ -315,7 +315,7 @@ func RunSignatureUploadAndVerificationTests(t *testing.T, cacheDriverParams map[
 		writers := io.MultiWriter(os.Stdout, logFile)
 		logger.Logger = logger.Output(writers)
 
-		imageStore := local.NewImageStore(globalDir, false, false, 0, 0, false, false,
+		imageStore := local.NewImageStore(globalDir, false, false,
 			logger, monitoring.NewMetricsServer(false, logger), nil, nil)
 
 		storeController := storage.StoreController{
@@ -422,7 +422,7 @@ func RunSignatureUploadAndVerificationTests(t *testing.T, cacheDriverParams map[
 		writers := io.MultiWriter(os.Stdout, logFile)
 		logger.Logger = logger.Output(writers)
 
-		imageStore := local.NewImageStore(globalDir, false, false, 0, 0, false, false,
+		imageStore := local.NewImageStore(globalDir, false, false,
 			logger, monitoring.NewMetricsServer(false, logger), nil, nil)
 
 		storeController := storage.StoreController{
@@ -584,7 +584,7 @@ func RunSignatureUploadAndVerificationTests(t *testing.T, cacheDriverParams map[
 		writers := io.MultiWriter(os.Stdout, logFile)
 		logger.Logger = logger.Output(writers)
 
-		imageStore := local.NewImageStore(globalDir, false, false, 0, 0, false, false,
+		imageStore := local.NewImageStore(globalDir, false, false,
 			logger, monitoring.NewMetricsServer(false, logger), nil, nil)
 
 		storeController := storage.StoreController{
@@ -845,7 +845,7 @@ func RunSignatureUploadAndVerificationTests(t *testing.T, cacheDriverParams map[
 		writers := io.MultiWriter(os.Stdout, logFile)
 		logger.Logger = logger.Output(writers)
 
-		imageStore := local.NewImageStore(globalDir, false, false, 0, 0, false, false,
+		imageStore := local.NewImageStore(globalDir, false, false,
 			logger, monitoring.NewMetricsServer(false, logger), nil, nil)
 
 		storeController := storage.StoreController{

diff --git a/pkg/extensions/lint/lint_test.go b/pkg/extensions/lint/lint_test.go
@@ -492,7 +492,7 @@ func TestVerifyMandatoryAnnotationsFunction(t *testing.T) {
 		var index ispec.Index
 
 		linter := lint.NewLinter(lintConfig, log.NewLogger("debug", ""))
-		imgStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imgStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), linter, nil)
 
 		indexContent, err := imgStore.GetIndexContent("zot-test")
@@ -524,7 +524,7 @@ func TestVerifyMandatoryAnnotationsFunction(t *testing.T) {
 		var index ispec.Index
 
 		linter := lint.NewLinter(lintConfig, log.NewLogger("debug", ""))
-		imgStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imgStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), linter, nil)
 
 		indexContent, err := imgStore.GetIndexContent("zot-test")
@@ -594,7 +594,7 @@ func TestVerifyMandatoryAnnotationsFunction(t *testing.T) {
 		index.Manifests = append(index.Manifests, manifestDesc)
 
 		linter := lint.NewLinter(lintConfig, log.NewLogger("debug", ""))
-		imgStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imgStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), linter, nil)
 
 		pass, err := linter.CheckMandatoryAnnotations("zot-test", digest, imgStore)
@@ -656,7 +656,7 @@ func TestVerifyMandatoryAnnotationsFunction(t *testing.T) {
 		index.Manifests = append(index.Manifests, manifestDesc)
 
 		linter := lint.NewLinter(lintConfig, log.NewLogger("debug", ""))
-		imgStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imgStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), linter, nil)
 
 		pass, err := linter.CheckMandatoryAnnotations("zot-test", digest, imgStore)
@@ -720,7 +720,7 @@ func TestVerifyMandatoryAnnotationsFunction(t *testing.T) {
 		index.Manifests = append(index.Manifests, manifestDesc)
 
 		linter := lint.NewLinter(lintConfig, log.NewLogger("debug", ""))
-		imgStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imgStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), linter, nil)
 
 		pass, err := linter.CheckMandatoryAnnotations("zot-test", digest, imgStore)
@@ -783,7 +783,7 @@ func TestVerifyMandatoryAnnotationsFunction(t *testing.T) {
 		index.Manifests = append(index.Manifests, manifestDesc)
 
 		linter := lint.NewLinter(lintConfig, log.NewLogger("debug", ""))
-		imgStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imgStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), linter, nil)
 
 		err = os.Chmod(path.Join(dir, "zot-test", "blobs"), 0o000)
@@ -881,7 +881,7 @@ func TestVerifyMandatoryAnnotationsFunction(t *testing.T) {
 		index.Manifests = append(index.Manifests, manifestDesc)
 
 		linter := lint.NewLinter(lintConfig, log.NewLogger("debug", ""))
-		imgStore := local.NewImageStore(dir, false, false, 0, 0, false, false,
+		imgStore := local.NewImageStore(dir, false, false,
 			log.NewLogger("debug", ""), monitoring.NewMetricsServer(false, log.NewLogger("debug", "")), linter, nil)
 
 		err = os.Chmod(path.Join(dir, "zot-test", "blobs", "sha256", manifest.Config.Digest.Encoded()), 0o000)

diff --git a/pkg/extensions/scrub/scrub_test.go b/pkg/extensions/scrub/scrub_test.go
@@ -200,7 +200,7 @@ func TestRunScrubRepo(t *testing.T) {
 			Name:        "cache",
 			UseRelPaths: true,
 		}, log)
-		imgStore := local.NewImageStore(dir, true, true, 1*time.Second, 1*time.Second, true,
+		imgStore := local.NewImageStore(dir, true,
 			true, log, metrics, nil, cacheDriver)
 
 		srcStorageCtlr := test.GetDefaultStoreController(dir, log)
@@ -236,7 +236,7 @@ func TestRunScrubRepo(t *testing.T) {
 			Name:        "cache",
 			UseRelPaths: true,
 		}, log)
-		imgStore := local.NewImageStore(dir, true, true, 1*time.Second, 1*time.Second, true,
+		imgStore := local.NewImageStore(dir, true,
 			true, log, metrics, nil, cacheDriver)
 
 		srcStorageCtlr := test.GetDefaultStoreController(dir, log)
@@ -278,9 +278,7 @@ func TestRunScrubRepo(t *testing.T) {
 			Name:        "cache",
 			UseRelPaths: true,
 		}, log)
-		imgStore := local.NewImageStore(dir, true, true, 1*time.Second,
-			1*time.Second, true, true, log, metrics, nil, cacheDriver,
-		)
+		imgStore := local.NewImageStore(dir, true, true, log, metrics, nil, cacheDriver)
 
 		srcStorageCtlr := test.GetDefaultStoreController(dir, log)
 		image := CreateDefaultVulnerableImage()

diff --git a/pkg/extensions/search/cve/cve_test.go b/pkg/extensions/search/cve/cve_test.go
@@ -37,7 +37,6 @@ import (
 	"zotregistry.io/zot/pkg/meta/boltdb"
 	mTypes "zotregistry.io/zot/pkg/meta/types"
 	"zotregistry.io/zot/pkg/storage"
-	storageConstants "zotregistry.io/zot/pkg/storage/constants"
 	"zotregistry.io/zot/pkg/storage/local"
 	. "zotregistry.io/zot/pkg/test"
 	. "zotregistry.io/zot/pkg/test/image-utils"
@@ -323,8 +322,7 @@ func TestImageFormat(t *testing.T) {
 		dbDir := t.TempDir()
 
 		metrics := monitoring.NewMetricsServer(false, log)
-		defaultStore := local.NewImageStore(imgDir, false, false, storageConstants.DefaultGCDelay,
-			storageConstants.DefaultUntaggedImgeRetentionDelay, false, false, log, metrics, nil, nil)
+		defaultStore := local.NewImageStore(imgDir, false, false, log, metrics, nil, nil)
 		storeController := storage.StoreController{DefaultStore: defaultStore}
 
 		params := boltdb.DBParameters{