From 6c1f1bdd4040195117b5b18a424c5903fbd4bbb2 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Sun, 17 May 2026 08:03:36 +0200 Subject: [PATCH] feat(metrics): add Prometheus GC metrics (#3863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(metrics): add Prometheus GC metrics Track garbage collection activity with three new metrics: - zot_gc_runs_total (counter, label: error) — GC run count - zot_gc_duration_seconds (summary) — GC run duration - zot_gc_deleted_total (counter, label: type) — items deleted by type: blob, manifest, upload MetricServer is added to GarbageCollect and wired through all callers (controller, verify-feature retention, tests). Signed-off-by: Benoit Tigeot * fix(test): add missing metrics var in GCS GC tests TestGCSGarbageCollectImageIndex and TestGCSGarbageCollectChainedImageIndexes were missing the metrics variable required by NewGarbageCollect after the MetricServer parameter was added. Signed-off-by: Benoit Tigeot * fix(test): add defer metrics.Stop() in GC tests Prevent goroutine/port leaks by stopping MetricsServer in storage_test.go (3 functions) and gcs_test.go (also add missing metrics declaration in TestGCSGarbageCollectImageManifest). Signed-off-by: Benoit Tigeot * fix(test): cover `CleanRepo` error path Add test that exercises the error branch in `CleanRepo` where `cleanRepo` fails, covering the metrics calls and log lines flagged by Codecov. Signed-off-by: Benoit Tigeot * test: Cover GC error paths for codecov Add three tests in gc_internal_test.go to cover previously untested error branches in `removeBlobUploads` and `removeUnreferencedBlobs`: `ListBlobUploads` failure, `addIndexBlobsToReferences` failure, and `PathNotFoundError` from `GetAllBlobs`. Signed-off-by: Benoit Tigeot * test(gc): cover remaining error paths Cover `StatBlobUpload`, `digest.Validate()`, `isBlobOlderThan`, and `CleanupRepo` error branches in `removeBlobUploads` and `removeUnreferencedBlobs`. `removeUnreferencedBlobs` now at 100% coverage, `removeBlobUploads` from 78.3% to 91.3%. Signed-off-by: Benoit Tigeot * test: cover `sanityChecks` label name mismatch Try to avoid -0.09% coverage regression on `minimal.go` by exercising the uncovered branch in `sanityChecks` where label names have correct count but wrong values. Signed-off-by: Benoit Tigeot * test(gc): exercise real GC path in metrics test TestGCMetrics was calling metric helpers directly instead of running actual garbage collection, so it couldn't catch wiring regressions where `CleanRepo` stops recording metrics. Now uploads an orphaned blob and runs `gc.CleanRepo` end-to-end, verifying metrics appear on the Prometheus endpoint. Suggestion from Copilot: https://github.com/project-zot/zot/pull/3863#discussion_r3129324719 Signed-off-by: Benoit Tigeot * fix(gc): skip deletion metrics when DryRun is enabled https://github.com/project-zot/zot/pull/3863#discussion_r3129324684 Signed-off-by: Benoit Tigeot * fix(test): stop leaked MetricsServer goroutines in GCS tests https://github.com/project-zot/zot/pull/3863#discussion_r3129324657 Signed-off-by: Benoit Tigeot * refactor(test): drop unnecessary zlog import alias Signed-off-by: Benoit Tigeot * fix(monitoring): expose metric types outside build tag `MetricsCopy` and related types were only visible under `\!metrics`, causing a typecheck failure when golangci-lint runs with `-tags metrics`. Moving the type definitions to `common.go` makes them unconditionally available. Signed-off-by: Benoit Tigeot * fix(monitoring): remove extra blank line for gci Signed-off-by: Benoit Tigeot * test(gc): cover both dry-run and real deletion metrics And fix issue with build tag with metrics Signed-off-by: Benoit Tigeot * Satisfy testpackage linter for gc metrics test The `testpackage` linter allows `package gc` only in files named `*_internal_test.go`; rename to follow that convention. Signed-off-by: Benoit Tigeot --------- Signed-off-by: Benoit Tigeot --- pkg/api/controller.go | 8 +- pkg/api/controller_test.go | 4 +- pkg/cli/server/verify_retention.go | 2 +- pkg/extensions/monitoring/common.go | 51 ++++ pkg/extensions/monitoring/extension.go | 44 ++++ pkg/extensions/monitoring/minimal.go | 85 +++---- .../monitoring/minimal_client_test.go | 10 + pkg/extensions/monitoring/monitoring_test.go | 56 +++++ pkg/storage/gc/gc.go | 59 +++-- pkg/storage/gc/gc_internal_test.go | 219 ++++++++++++++++-- pkg/storage/gc/gc_metrics_internal_test.go | 106 +++++++++ pkg/storage/gc/gc_test.go | 66 +++--- pkg/storage/gcs/gcs_test.go | 14 +- pkg/storage/local/local_test.go | 14 +- pkg/storage/storage_test.go | 18 +- 15 files changed, 614 insertions(+), 142 deletions(-) create mode 100644 pkg/storage/gc/gc_metrics_internal_test.go diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 05d86e96..749433d2 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -557,7 +557,7 @@ func (c *Controller) StartBackgroundTasks() { } // Run GC and retention tasks - RunGCTasks(c.Config, c.StoreController, c.MetaDB, c.taskScheduler, c.Log, c.Audit) + RunGCTasks(c.Config, c.StoreController, c.MetaDB, c.taskScheduler, c.Log, c.Audit, c.Metrics) // Enable running dedupe blobs both ways (dedupe or restore deduped blobs) c.StoreController.DefaultStore.RunDedupeBlobs(time.Duration(0), c.taskScheduler) @@ -617,7 +617,7 @@ func (c *Controller) StartBackgroundTasks() { // RunGCTasks runs minimal GC and retention tasks without full controller. func RunGCTasks(conf *config.Config, storeController storage.StoreController, metaDB mTypes.MetaDB, - taskScheduler *scheduler.Scheduler, logger log.Logger, audit *log.Logger, + taskScheduler *scheduler.Scheduler, logger log.Logger, audit *log.Logger, metrics monitoring.MetricServer, ) { // Enable running garbage-collect periodically for DefaultStore storageConfig := conf.CopyStorageConfig() @@ -626,7 +626,7 @@ func RunGCTasks(conf *config.Config, storeController storage.StoreController, me Delay: storageConfig.GCDelay, ImageRetention: storageConfig.Retention, MaxSchedulerDelay: storageConfig.GCMaxSchedulerDelay, - }, audit, logger) + }, audit, logger, metrics) gc.CleanImageStorePeriodically(storageConfig.GCInterval, taskScheduler) } @@ -641,7 +641,7 @@ func RunGCTasks(conf *config.Config, storeController storage.StoreController, me Delay: subStorageConfig.GCDelay, ImageRetention: subStorageConfig.Retention, MaxSchedulerDelay: subStorageConfig.GCMaxSchedulerDelay, - }, audit, logger) + }, audit, logger, metrics) gc.CleanImageStorePeriodically(subStorageConfig.GCInterval, taskScheduler) } diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index d79db295..45a08f0a 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -11816,7 +11816,7 @@ func TestGCSignaturesAndUntaggedManifestsWithMetaDB(t *testing.T) { gc.Options{ Delay: ctlr.Config.Storage.GCDelay, ImageRetention: ctlr.Config.Storage.Retention, - }, ctlr.Audit, ctlr.Log) + }, ctlr.Audit, ctlr.Log, ctlr.Metrics) resp, err := resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, tag)) So(err, ShouldBeNil) @@ -12051,7 +12051,7 @@ func TestGCSignaturesAndUntaggedManifestsWithMetaDB(t *testing.T) { gc.Options{ Delay: ctlr.Config.Storage.GCDelay, ImageRetention: ctlr.Config.Storage.Retention, - }, ctlr.Audit, ctlr.Log) + }, ctlr.Audit, ctlr.Log, ctlr.Metrics) resp, err := resty.R().Get(baseURL + fmt.Sprintf("/v2/%s/manifests/%s", repoName, tag)) So(err, ShouldBeNil) diff --git a/pkg/cli/server/verify_retention.go b/pkg/cli/server/verify_retention.go index a4b256be..2638f6a1 100644 --- a/pkg/cli/server/verify_retention.go +++ b/pkg/cli/server/verify_retention.go @@ -156,7 +156,7 @@ func newVerifyFeatureRetentionCmd(conf *config.Config) *cobra.Command { logger.Info().Msg("garbage collection and retention tasks will be submitted to the scheduler") // Run GC and retention tasks - api.RunGCTasks(conf, storeController, metaDB, taskScheduler, logger, nil) + api.RunGCTasks(conf, storeController, metaDB, taskScheduler, logger, nil, metricsServer) // Wait for tasks to complete with optional timeout timeout, err := cmd.PersistentFlags().GetDuration("timeout") diff --git a/pkg/extensions/monitoring/common.go b/pkg/extensions/monitoring/common.go index 0390b160..53e4a2b2 100644 --- a/pkg/extensions/monitoring/common.go +++ b/pkg/extensions/monitoring/common.go @@ -18,6 +18,57 @@ type MetricServer interface { Stop() } +type MetricsInfo struct { + Counters []*CounterValue + Gauges []*GaugeValue + Summaries []*SummaryValue + Histograms []*HistogramValue +} + +type MetricsCopy struct { + Counters []CounterValue + Gauges []GaugeValue + Summaries []SummaryValue + Histograms []HistogramValue +} + +// CounterValue stores info about a metric that is incremented over time, +// such as the number of requests to an HTTP endpoint. +type CounterValue struct { + Name string + Count int + LabelNames []string + LabelValues []string +} + +// GaugeValue stores one value that is updated as time goes on, such as +// the amount of memory allocated. +type GaugeValue struct { + Name string + Value float64 + LabelNames []string + LabelValues []string +} + +// SummaryValue stores info about a metric that is incremented over time, +// such as the number of requests to an HTTP endpoint. +type SummaryValue struct { + Name string + Count int + Sum float64 + LabelNames []string + LabelValues []string +} + +type HistogramValue struct { + Name string + Count int + Sum float64 + Buckets map[string]int + LabelNames []string + LabelValues []string +} + func GetDirSize(path string) (int64, error) { var size int64 diff --git a/pkg/extensions/monitoring/extension.go b/pkg/extensions/monitoring/extension.go index 5360243f..e63fe333 100644 --- a/pkg/extensions/monitoring/extension.go +++ b/pkg/extensions/monitoring/extension.go @@ -4,6 +4,7 @@ package monitoring import ( "path" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -129,6 +130,29 @@ var ( }, []string{"name"}, ) + gcRuns = promauto.NewCounterVec( //nolint: gochecknoglobals + prometheus.CounterOpts{ + Namespace: metricsNamespace, + Name: "gc_runs_total", + Help: "Total number of garbage collection runs", + }, + []string{"error"}, + ) + gcDuration = promauto.NewSummary( //nolint: gochecknoglobals + prometheus.SummaryOpts{ + Namespace: metricsNamespace, + Name: "gc_duration_seconds", + Help: "Duration of garbage collection runs", + }, + ) + gcDeleted = promauto.NewCounterVec( //nolint: gochecknoglobals + prometheus.CounterOpts{ + Namespace: metricsNamespace, + Name: "gc_deleted_total", + Help: "Total number of items deleted by garbage collection", + }, + []string{"type"}, + ) ) type metricServer struct { @@ -293,3 +317,23 @@ func ObserveWorkersTasksDuration(ms MetricServer, taskName string, duration time workersTasksDuration.WithLabelValues(taskName).Observe(duration.Seconds()) }) } + +func IncGCRuns(ms MetricServer, hasError bool) { + ms.SendMetric(func() { + gcRuns.WithLabelValues(strconv.FormatBool(hasError)).Inc() + }) +} + +func ObserveGCDuration(ms MetricServer, latency time.Duration) { + ms.SendMetric(func() { + gcDuration.Observe(latency.Seconds()) + }) +} + +func IncGCDeleted(ms MetricServer, artifactType string, count int) { + if count > 0 { + ms.SendMetric(func() { + gcDeleted.WithLabelValues(artifactType).Add(float64(count)) + }) + } +} diff --git a/pkg/extensions/monitoring/minimal.go b/pkg/extensions/monitoring/minimal.go index a973c38a..4af84393 100644 --- a/pkg/extensions/monitoring/minimal.go +++ b/pkg/extensions/monitoring/minimal.go @@ -35,6 +35,10 @@ const ( httpMethodLatencySeconds = metricsNamespace + ".http.method.latency.seconds" storageLockLatencySeconds = metricsNamespace + ".storage.lock.latency.seconds" workersTasksDuration = metricsNamespace + ".scheduler.workers.tasks.duration.seconds" + // GC metrics. + gcRuns = metricsNamespace + ".gc.runs" + gcDuration = metricsNamespace + ".gc.duration.seconds" + gcDeleted = metricsNamespace + ".gc.deleted" metricsScrapeTimeout = 2 * time.Minute metricsScrapeCheckInterval = 30 * time.Second @@ -52,56 +56,6 @@ type metricServer struct { stopChan chan struct{} // Channel to signal shutdown } -type MetricsInfo struct { - Counters []*CounterValue - Gauges []*GaugeValue - Summaries []*SummaryValue - Histograms []*HistogramValue -} -type MetricsCopy struct { - Counters []CounterValue - Gauges []GaugeValue - Summaries []SummaryValue - Histograms []HistogramValue -} - -// CounterValue stores info about a metric that is incremented over time, -// such as the number of requests to an HTTP endpoint. -type CounterValue struct { - Name string - Count int - LabelNames []string - LabelValues []string -} - -// GaugeValue stores one value that is updated as time goes on, such as -// the amount of memory allocated. -type GaugeValue struct { - Name string - Value float64 - LabelNames []string - LabelValues []string -} - -// SummaryValue stores info about a metric that is incremented over time, -// such as the number of requests to an HTTP endpoint. -type SummaryValue struct { - Name string - Count int - Sum float64 - LabelNames []string - LabelValues []string -} - -type HistogramValue struct { - Name string - Count int - Sum float64 - Buckets map[string]int - LabelNames []string - LabelValues []string -} - func GetDefaultBuckets() []float64 { return []float64{.05, .5, 1, 5, 30, 60, 600, math.MaxFloat64} } @@ -274,6 +228,8 @@ func GetCounters() map[string][]string { repoDownloads: {"repo"}, repoUploads: {"repo"}, schedulerGenerators: {}, + gcRuns: {"error"}, + gcDeleted: {"type"}, } } @@ -291,6 +247,7 @@ func GetGauges() map[string][]string { func GetSummaries() map[string][]string { return map[string][]string{ httpRepoLatencySeconds: {"repo"}, + gcDuration: {}, } } @@ -643,3 +600,31 @@ func SetSchedulerWorkers(ms MetricServer, w map[string]int) { ms.SendMetric(workers) } } + +func IncGCRuns(ms MetricServer, hasError bool) { + req := CounterValue{ + Name: gcRuns, + LabelNames: []string{"error"}, + LabelValues: []string{strconv.FormatBool(hasError)}, + } + ms.SendMetric(req) +} + +func ObserveGCDuration(ms MetricServer, latency time.Duration) { + sv := SummaryValue{ + Name: gcDuration, + Sum: latency.Seconds(), + } + ms.SendMetric(sv) +} + +func IncGCDeleted(ms MetricServer, artifactType string, count int) { + for range count { + req := CounterValue{ + Name: gcDeleted, + LabelNames: []string{"type"}, + LabelValues: []string{artifactType}, + } + ms.SendMetric(req) + } +} diff --git a/pkg/extensions/monitoring/minimal_client_test.go b/pkg/extensions/monitoring/minimal_client_test.go index 10fb014e..6da8ef8f 100644 --- a/pkg/extensions/monitoring/minimal_client_test.go +++ b/pkg/extensions/monitoring/minimal_client_test.go @@ -143,6 +143,16 @@ func TestNewMetricsClientFallbackKeepsTLSHardening(t *testing.T) { } } +func TestSanityChecksLabelNameMismatch(t *testing.T) { + t.Parallel() + + err := sanityChecks("test.metric", []string{"method", "code"}, true, + []string{"method", "wrong"}, []string{"GET", "200"}) + if err == nil { + t.Fatal("expected error when label names don't match known labels") + } +} + func generateServerCertificateChain() ([]byte, []byte, []byte, error) { now := time.Now() diff --git a/pkg/extensions/monitoring/monitoring_test.go b/pkg/extensions/monitoring/monitoring_test.go index c63f8589..86bac829 100644 --- a/pkg/extensions/monitoring/monitoring_test.go +++ b/pkg/extensions/monitoring/monitoring_test.go @@ -3,6 +3,8 @@ package monitoring_test import ( + "bytes" + "context" "fmt" "io" "math/rand" @@ -12,6 +14,7 @@ import ( "testing" "time" + godigest "github.com/opencontainers/go-digest" . "github.com/smartystreets/goconvey/convey" "gopkg.in/resty.v1" @@ -22,8 +25,10 @@ import ( "zotregistry.dev/zot/v2/pkg/log" "zotregistry.dev/zot/v2/pkg/scheduler" common "zotregistry.dev/zot/v2/pkg/storage/common" + "zotregistry.dev/zot/v2/pkg/storage/gc" test "zotregistry.dev/zot/v2/pkg/test/common" . "zotregistry.dev/zot/v2/pkg/test/image-utils" + "zotregistry.dev/zot/v2/pkg/test/mocks" ociutils "zotregistry.dev/zot/v2/pkg/test/oci-utils" ) @@ -502,6 +507,57 @@ func TestPopulateStorageMetrics(t *testing.T) { }) } +func TestGCMetrics(t *testing.T) { + Convey("GC metrics should be emitted after garbage collection", t, func() { + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + rootDir := t.TempDir() + conf.Storage.RootDirectory = rootDir + conf.Storage.GC = false + enabled := true + conf.Extensions = &extconf.ExtensionConfig{} + conf.Extensions.Metrics = &extconf.MetricsConfig{ + BaseConfig: extconf.BaseConfig{Enable: &enabled}, + Prometheus: &extconf.PrometheusConfig{Path: "/metrics"}, + } + + ctlr := api.NewController(conf) + + srcStorageCtlr := ociutils.GetDefaultStoreController(rootDir, ctlr.Log) + err := WriteImageToFileSystem(CreateDefaultImage(), "gc-metrics-test", "0.0.1", srcStorageCtlr) + So(err, ShouldBeNil) + + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(port) + defer cm.StopServer() + + imgStore := ctlr.StoreController.DefaultStore + + orphanBlob := []byte("orphaned-blob-content") + _, _, err = imgStore.FullBlobUpload("gc-metrics-test", bytes.NewReader(orphanBlob), godigest.FromBytes(orphanBlob)) + So(err, ShouldBeNil) + + audit := log.NewAuditLogger("debug", "/dev/null") + gcObj := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{Delay: 0}, + audit, ctlr.Log, ctlr.Metrics) + + err = gcObj.CleanRepo(context.Background(), "gc-metrics-test") + So(err, ShouldBeNil) + + resp, err := resty.R().Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + respStr := string(resp.Body()) + So(respStr, ShouldContainSubstring, "zot_gc_runs_total") + So(respStr, ShouldContainSubstring, "zot_gc_duration_seconds") + So(respStr, ShouldContainSubstring, "zot_gc_deleted_total{type=\"blob\"}") + }) +} + func generateRandomString() string { //nolint: gosec seededRand := rand.New(rand.NewSource(time.Now().UnixNano())) diff --git a/pkg/storage/gc/gc.go b/pkg/storage/gc/gc.go index bd8c971c..d4dc76b2 100644 --- a/pkg/storage/gc/gc.go +++ b/pkg/storage/gc/gc.go @@ -19,6 +19,7 @@ import ( "zotregistry.dev/zot/v2/pkg/api/config" zcommon "zotregistry.dev/zot/v2/pkg/common" "zotregistry.dev/zot/v2/pkg/compat" + "zotregistry.dev/zot/v2/pkg/extensions/monitoring" zlog "zotregistry.dev/zot/v2/pkg/log" mTypes "zotregistry.dev/zot/v2/pkg/meta/types" "zotregistry.dev/zot/v2/pkg/retention" @@ -52,10 +53,11 @@ type GarbageCollect struct { policyMgr rTypes.PolicyManager auditLog *zlog.Logger log zlog.Logger + metrics monitoring.MetricServer } func NewGarbageCollect(imgStore types.ImageStore, metaDB mTypes.MetaDB, opts Options, - auditLog *zlog.Logger, log zlog.Logger, + auditLog *zlog.Logger, log zlog.Logger, metrics monitoring.MetricServer, ) GarbageCollect { return GarbageCollect{ imgStore: imgStore, @@ -64,6 +66,7 @@ func NewGarbageCollect(imgStore types.ImageStore, metaDB mTypes.MetaDB, opts Opt policyMgr: retention.NewPolicyManager(opts.ImageRetention, log, auditLog), auditLog: auditLog, log: log, + metrics: metrics, } } @@ -99,7 +102,12 @@ func (gc GarbageCollect) CleanRepo(ctx context.Context, repo string) error { gc.log.Info().Str("module", "gc"). Msg("executing gc of orphaned blobs for " + path.Join(gc.imgStore.RootDir(), repo)) + start := time.Now() + if err := gc.cleanRepo(ctx, repo); err != nil { + monitoring.ObserveGCDuration(gc.metrics, time.Since(start)) + monitoring.IncGCRuns(gc.metrics, true) + errMessage := "failed to run GC for " + path.Join(gc.imgStore.RootDir(), repo) gc.log.Error().Err(err).Str("module", "gc").Msg(errMessage) gc.log.Info().Str("module", "gc"). @@ -108,6 +116,9 @@ func (gc GarbageCollect) CleanRepo(ctx context.Context, repo string) error { return err } + monitoring.ObserveGCDuration(gc.metrics, time.Since(start)) + monitoring.IncGCRuns(gc.metrics, false) + gc.log.Info().Str("module", "gc"). Msg("gc successfully completed for " + path.Join(gc.imgStore.RootDir(), repo)) @@ -141,6 +152,8 @@ func (gc GarbageCollect) cleanRepo(ctx context.Context, repo string) error { return err } + manifestsBefore := len(index.Manifests) + // apply tags retention if err := gc.removeTagsPerRetentionPolicy(ctx, repo, &index); err != nil { return err @@ -160,16 +173,26 @@ func (gc GarbageCollect) cleanRepo(ctx context.Context, repo string) error { } } + manifestsDeleted := manifestsBefore - len(index.Manifests) + // gc unreferenced blobs - if err := gc.removeUnreferencedBlobs(repo, gc.opts.Delay, gc.log); err != nil { + blobsDeleted, err := gc.removeUnreferencedBlobs(repo, gc.opts.Delay, gc.log) + if err != nil { return err } // gc old blob uploads - if err := gc.removeBlobUploads(repo, gc.opts.Delay); err != nil { + uploadsDeleted, err := gc.removeBlobUploads(repo, gc.opts.Delay) + if err != nil { return err } + if !gc.opts.ImageRetention.DryRun { + monitoring.IncGCDeleted(gc.metrics, "manifest", manifestsDeleted) + monitoring.IncGCDeleted(gc.metrics, "blob", blobsDeleted) + monitoring.IncGCDeleted(gc.metrics, "upload", uploadsDeleted) + } + return nil } @@ -609,23 +632,25 @@ func (gc GarbageCollect) identifyManifestsReferencedInIndex(index ispec.Index, r } // removeBlobUploads gc all temporary uploads which are past their gc delay. -func (gc GarbageCollect) removeBlobUploads(repo string, delay time.Duration) error { +func (gc GarbageCollect) removeBlobUploads(repo string, delay time.Duration) (int, error) { gc.log.Debug().Str("module", "gc").Str("repository", repo).Msg("cleaning unclaimed blob uploads") if dir := path.Join(gc.imgStore.RootDir(), repo); !gc.imgStore.DirExists(dir) { // The repository was already cleaned up by a different codepath - return nil + return 0, nil } blobUploads, err := gc.imgStore.ListBlobUploads(repo) if err != nil { gc.log.Error().Err(err).Str("module", "gc").Str("repository", repo).Msg("failed to get list of blob uploads") - return err + return 0, err } var aggregatedErr error + deleted := 0 + for _, uuid := range blobUploads { _, size, modtime, err := gc.imgStore.StatBlobUpload(repo, uuid) if err != nil { @@ -648,15 +673,17 @@ func (gc GarbageCollect) removeBlobUploads(repo string, delay time.Duration) err Str("size", strconv.FormatInt(size, 10)).Str("modified", modtime.String()).Msg("failed to delete blob upload") aggregatedErr = errors.Join(aggregatedErr, err) + } else { + deleted++ } } - return aggregatedErr + return deleted, aggregatedErr } // removeUnreferencedBlobs gc all blobs which are not referenced by any manifest found in repo's index.json. func (gc GarbageCollect) removeUnreferencedBlobs(repo string, delay time.Duration, log zlog.Logger, -) error { +) (int, error) { gc.log.Debug().Str("module", "gc").Str("repository", repo).Msg("cleaning orphan blobs") refBlobs := map[string]bool{} @@ -665,26 +692,26 @@ func (gc GarbageCollect) removeUnreferencedBlobs(repo string, delay time.Duratio if err != nil { log.Error().Err(err).Str("module", "gc").Str("repository", repo).Msg("failed to read index.json in repo") - return err + return 0, err } err = gc.addIndexBlobsToReferences(repo, index, refBlobs) if err != nil { log.Error().Err(err).Str("module", "gc").Str("repository", repo).Msg("failed to get referenced blobs in repo") - return err + return 0, err } allBlobs, err := gc.imgStore.GetAllBlobs(repo) if err != nil { // /blobs/sha256/ may be empty in the case of s3, no need to return err, we want to skip if errors.As(err, &driver.PathNotFoundError{}) { - return nil + return 0, nil } log.Error().Err(err).Str("module", "gc").Str("repository", repo).Msg("failed to get all blobs") - return err + return 0, err } gcBlobs := make([]godigest.Digest, 0) @@ -694,7 +721,7 @@ func (gc GarbageCollect) removeUnreferencedBlobs(repo string, delay time.Duratio log.Error().Err(err).Str("module", "gc").Str("repository", repo). Str("digest", digest.String()).Msg("failed to parse digest") - return err + return 0, err } if _, ok := refBlobs[digest.String()]; !ok { @@ -703,7 +730,7 @@ func (gc GarbageCollect) removeUnreferencedBlobs(repo string, delay time.Duratio log.Error().Err(err).Str("module", "gc").Str("repository", repo). Str("digest", digest.String()).Msg("failed to determine GC delay") - return err + return 0, err } if canGC { @@ -717,13 +744,13 @@ func (gc GarbageCollect) removeUnreferencedBlobs(repo string, delay time.Duratio reaped, err := gc.imgStore.CleanupRepo(repo, gcBlobs, removeRepo) if err != nil { - return err + return 0, err } log.Info().Str("module", "gc").Str("repository", repo).Int("count", reaped). Msg("garbage collected blobs") - return nil + return reaped, nil } // used by removeUnreferencedBlobs() diff --git a/pkg/storage/gc/gc_internal_test.go b/pkg/storage/gc/gc_internal_test.go index 04cc30b1..17b77006 100644 --- a/pkg/storage/gc/gc_internal_test.go +++ b/pkg/storage/gc/gc_internal_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/distribution/distribution/v3/registry/storage/driver" godigest "github.com/opencontainers/go-digest" ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" @@ -62,7 +63,7 @@ func TestGarbageCollectManifestErrors(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) Convey("trigger missing blob in addImageIndexBlobsToReferences()", func() { // GC should continue when blobs are missing (not found), not return an error @@ -193,7 +194,7 @@ func TestGarbageCollectIndexErrors(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) content := []byte("this is a blob") bdgst := godigest.FromBytes(content) @@ -296,6 +297,8 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { Convey("Cover gc error paths", t, func(c C) { log := zlog.NewTestLogger() audit := zlog.NewAuditLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, log) + defer metrics.Stop() gcOptions := Options{ Delay: storageConstants.DefaultGCDelay, @@ -315,7 +318,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { GetRepoMetaFn: func(ctx context.Context, repo string) (types.RepoMeta, error) { return types.RepoMeta{}, errGC }, - }, gcOptions, audit, log) + }, gcOptions, audit, log, metrics) err := gc.cleanRepo(ctx, repoName) So(err, ShouldNotBeNil) @@ -326,9 +329,9 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { GetRepoMetaFn: func(ctx context.Context, repo string) (types.RepoMeta, error) { return types.RepoMeta{}, errGC }, - }, gcOptions, audit, log) + }, gcOptions, audit, log, metrics) - err := gc.removeUnreferencedBlobs("repo", time.Hour, log) + _, err := gc.removeUnreferencedBlobs("repo", time.Hour, log) So(err, ShouldNotBeNil) }) @@ -337,7 +340,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { GetRepoMetaFn: func(ctx context.Context, repo string) (types.RepoMeta, error) { return types.RepoMeta{}, errGC }, - }, gcOptions, audit, log) + }, gcOptions, audit, log, metrics) _, err := gc.removeManifest("", &ispec.Index{}, ispec.DescriptorEmptyJSON, "tag", "", "") So(err, ShouldNotBeNil) @@ -365,7 +368,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { GetRepoMetaFn: func(ctx context.Context, repo string) (types.RepoMeta, error) { return types.RepoMeta{}, errGC }, - }, gcOptions, audit, log) + }, gcOptions, audit, log, metrics) err := gc.removeTagsPerRetentionPolicy(ctx, "name", &ispec.Index{}) So(err, ShouldNotBeNil) @@ -389,7 +392,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(mocks.MockedImageStore{}, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(mocks.MockedImageStore{}, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) ctx, cancel := context.WithCancel(ctx) cancel() @@ -420,7 +423,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) err = gc.cleanRepo(ctx, repoName) So(err, ShouldNotBeNil) @@ -444,7 +447,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) err = gc.cleanRepo(ctx, repoName) So(err, ShouldNotBeNil) @@ -457,7 +460,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) err := gc.cleanRepo(ctx, repoName) So(err, ShouldNotBeNil) @@ -497,7 +500,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) err = gc.removeManifestsPerRepoPolicy(ctx, repoName, &ispec.Index{ Manifests: []ispec.Descriptor{ @@ -526,7 +529,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) err := gc.removeManifestsPerRepoPolicy(ctx, repoName, &ispec.Index{ Manifests: []ispec.Descriptor{ @@ -551,7 +554,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) ctx, cancel := context.WithCancel(ctx) cancel() @@ -595,7 +598,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, }, } - gc := NewGarbageCollect(imgStore, metaDB, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, metaDB, gcOptions, audit, log, metrics) err = gc.removeManifestsPerRepoPolicy(ctx, repoName, &ispec.Index{ Manifests: []ispec.Descriptor{ @@ -629,7 +632,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { } gcOptions.ImageRetention = config.ImageRetention{} - gc := NewGarbageCollect(imgStore, metaDB, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, metaDB, gcOptions, audit, log, metrics) desc := ispec.Descriptor{ MediaType: ispec.MediaTypeImageManifest, @@ -673,7 +676,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) err = gc.removeManifestsPerRepoPolicy(ctx, repoName, &returnedIndexImage) So(err, ShouldNotBeNil) @@ -704,7 +707,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) err = gc.removeManifestsPerRepoPolicy(ctx, repoName, &ispec.Index{ Manifests: []ispec.Descriptor{ @@ -744,7 +747,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) // removeIndexReferrers should skip the missing nested index and continue gced, err := gc.removeIndexReferrers(repoName, &topLevelIndex, topLevelIndex) @@ -773,7 +776,7 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { }, } - gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log) + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) // identifyManifestsReferencedInIndex should skip the missing nested index and continue referenced := make(map[godigest.Digest]bool) @@ -782,5 +785,181 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) { // No manifests should be marked as referenced since the nested index is missing So(len(referenced), ShouldEqual, 0) }) + + Convey("Error on ListBlobUploads in removeBlobUploads", func() { + imgStore := mocks.MockedImageStore{ + DirExistsFn: func(d string) bool { + return true + }, + ListBlobUploadsFn: func(repo string) ([]string, error) { + return nil, errGC + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + deleted, err := gc.removeBlobUploads(repoName, time.Hour) + So(err, ShouldNotBeNil) + So(deleted, ShouldEqual, 0) + }) + + Convey("Error on addIndexBlobsToReferences in removeUnreferencedBlobs", func() { + returnedIndex := ispec.Index{ + Manifests: []ispec.Descriptor{ + { + MediaType: ispec.MediaTypeImageManifest, + Digest: godigest.FromBytes([]byte("manifest-content")), + }, + }, + } + returnedIndexBuf, err := json.Marshal(returnedIndex) + So(err, ShouldBeNil) + + imgStore := mocks.MockedImageStore{ + GetIndexContentFn: func(repo string) ([]byte, error) { + return returnedIndexBuf, nil + }, + GetBlobContentFn: func(repo string, digest godigest.Digest) ([]byte, error) { + return nil, errGC + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + deleted, err := gc.removeUnreferencedBlobs(repoName, time.Hour, log) + So(err, ShouldNotBeNil) + So(deleted, ShouldEqual, 0) + }) + + Convey("PathNotFoundError on GetAllBlobs in removeUnreferencedBlobs", func() { + returnedIndex := ispec.Index{} + returnedIndexBuf, err := json.Marshal(returnedIndex) + So(err, ShouldBeNil) + + imgStore := mocks.MockedImageStore{ + GetIndexContentFn: func(repo string) ([]byte, error) { + return returnedIndexBuf, nil + }, + GetAllBlobsFn: func(repo string) ([]godigest.Digest, error) { + return nil, driver.PathNotFoundError{Path: "/blobs/sha256", DriverName: "local"} + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + deleted, err := gc.removeUnreferencedBlobs(repoName, time.Hour, log) + So(err, ShouldBeNil) + So(deleted, ShouldEqual, 0) + }) + + Convey("StatBlobUpload error in removeBlobUploads", func() { + imgStore := mocks.MockedImageStore{ + DirExistsFn: func(d string) bool { + return true + }, + ListBlobUploadsFn: func(repo string) ([]string, error) { + return []string{"upload-1"}, nil + }, + StatBlobUploadFn: func(repo string, uuid string) (bool, int64, time.Time, error) { + return false, 0, time.Time{}, errGC + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + deleted, err := gc.removeBlobUploads(repoName, time.Hour) + So(err, ShouldNotBeNil) + So(deleted, ShouldEqual, 0) + }) + + Convey("Invalid digest from GetAllBlobs in removeUnreferencedBlobs", func() { + returnedIndex := ispec.Index{} + returnedIndexBuf, err := json.Marshal(returnedIndex) + So(err, ShouldBeNil) + + imgStore := mocks.MockedImageStore{ + GetIndexContentFn: func(repo string) ([]byte, error) { + return returnedIndexBuf, nil + }, + GetAllBlobsFn: func(repo string) ([]godigest.Digest, error) { + return []godigest.Digest{godigest.Digest("invalid")}, nil + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + deleted, err := gc.removeUnreferencedBlobs(repoName, time.Hour, log) + So(err, ShouldNotBeNil) + So(deleted, ShouldEqual, 0) + }) + + Convey("StatBlob error in removeUnreferencedBlobs", func() { + blobDigest := godigest.FromBytes([]byte("blob-content")) + + returnedIndex := ispec.Index{} + returnedIndexBuf, err := json.Marshal(returnedIndex) + So(err, ShouldBeNil) + + imgStore := mocks.MockedImageStore{ + GetIndexContentFn: func(repo string) ([]byte, error) { + return returnedIndexBuf, nil + }, + GetAllBlobsFn: func(repo string) ([]godigest.Digest, error) { + return []godigest.Digest{blobDigest}, nil + }, + StatBlobFn: func(repo string, digest godigest.Digest) (bool, int64, time.Time, error) { + return false, 0, time.Time{}, errGC + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + deleted, err := gc.removeUnreferencedBlobs(repoName, time.Hour, log) + So(err, ShouldNotBeNil) + So(deleted, ShouldEqual, 0) + }) + + Convey("CleanupRepo error in removeUnreferencedBlobs", func() { + blobDigest := godigest.FromBytes([]byte("blob-content")) + + returnedIndex := ispec.Index{} + returnedIndexBuf, err := json.Marshal(returnedIndex) + So(err, ShouldBeNil) + + imgStore := mocks.MockedImageStore{ + GetIndexContentFn: func(repo string) ([]byte, error) { + return returnedIndexBuf, nil + }, + GetAllBlobsFn: func(repo string) ([]godigest.Digest, error) { + return []godigest.Digest{blobDigest}, nil + }, + StatBlobFn: func(repo string, digest godigest.Digest) (bool, int64, time.Time, error) { + return true, 100, time.Now().Add(-2 * time.Hour), nil + }, + CleanupRepoFn: func(repo string, blobs []godigest.Digest, removeRepo bool) (int, error) { + return 0, errGC + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + deleted, err := gc.removeUnreferencedBlobs(repoName, time.Hour, log) + So(err, ShouldNotBeNil) + So(deleted, ShouldEqual, 0) + }) + + Convey("CleanRepo records error metrics when cleanRepo fails", func() { + imgStore := mocks.MockedImageStore{ + DirExistsFn: func(d string) bool { + return false + }, + } + + gc := NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gcOptions, audit, log, metrics) + + err := gc.CleanRepo(ctx, repoName) + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrRepoNotFound), ShouldBeTrue) + }) }) } diff --git a/pkg/storage/gc/gc_metrics_internal_test.go b/pkg/storage/gc/gc_metrics_internal_test.go new file mode 100644 index 00000000..a8764994 --- /dev/null +++ b/pkg/storage/gc/gc_metrics_internal_test.go @@ -0,0 +1,106 @@ +//go:build !metrics + +package gc + +import ( + "context" + "testing" + "time" + + . "github.com/smartystreets/goconvey/convey" + + "zotregistry.dev/zot/v2/pkg/api/config" + "zotregistry.dev/zot/v2/pkg/extensions/monitoring" + zlog "zotregistry.dev/zot/v2/pkg/log" + "zotregistry.dev/zot/v2/pkg/storage" + "zotregistry.dev/zot/v2/pkg/storage/cache" + "zotregistry.dev/zot/v2/pkg/storage/local" + . "zotregistry.dev/zot/v2/pkg/test/image-utils" +) + +func TestGCDeletedMetrics(t *testing.T) { + trueVal := true + + Convey("Given a repo with a kept and a deletable tag", t, func() { + dir := t.TempDir() + + log := zlog.NewTestLogger() + audit := zlog.NewAuditLogger("debug", "") + + metrics := monitoring.NewMetricsServer(true, log) + defer metrics.Stop() + + cacheDriver, _ := storage.Create("boltdb", cache.BoltDBDriverParameters{ + RootDir: dir, + Name: "cache", + UseRelPaths: true, + }, log) + imgStore := local.NewImageStore(dir, true, true, log, metrics, nil, cacheDriver, nil, nil) + + err := WriteImageToFileSystem(CreateDefaultImage(), repoName, "keep-me", + storage.StoreController{DefaultStore: imgStore}) + So(err, ShouldBeNil) + err = WriteImageToFileSystem(CreateDefaultImage(), repoName, "delete-me", + storage.StoreController{DefaultStore: imgStore}) + So(err, ShouldBeNil) + + retentionPolicies := []config.RetentionPolicy{ + { + Repositories: []string{"**"}, + DeleteUntagged: &trueVal, + KeepTags: []config.KeepTagsPolicy{ + {Patterns: []string{"keep-me"}}, + }, + }, + } + + Convey("DryRun should not emit deleted metrics", func() { + gc := NewGarbageCollect(imgStore, nil, Options{ + Delay: 1 * time.Millisecond, + ImageRetention: config.ImageRetention{ + Delay: 1 * time.Millisecond, + DryRun: true, + Policies: retentionPolicies, + }, + }, audit, log, metrics) + + err = gc.CleanRepo(context.Background(), repoName) + So(err, ShouldBeNil) + + So(gcDeletedCount(metrics, "manifest"), ShouldEqual, 0) + }) + + Convey("Real GC should emit deleted metrics", func() { + gc := NewGarbageCollect(imgStore, nil, Options{ + Delay: 1 * time.Millisecond, + ImageRetention: config.ImageRetention{ + Delay: 1 * time.Millisecond, + Policies: retentionPolicies, + }, + }, audit, log, metrics) + + err = gc.CleanRepo(context.Background(), repoName) + So(err, ShouldBeNil) + + So(gcDeletedCount(metrics, "manifest"), ShouldBeGreaterThan, 0) + }) + }) +} + +func gcDeletedCount(metrics monitoring.MetricServer, artifactType string) int { + data := metrics.ReceiveMetrics() + + metricsCopy, ok := data.(monitoring.MetricsCopy) + if !ok { + return -1 + } + + for _, counter := range metricsCopy.Counters { + if counter.Name == "zot.gc.deleted" && + len(counter.LabelValues) > 0 && counter.LabelValues[0] == artifactType { + return counter.Count + } + } + + return 0 +} diff --git a/pkg/storage/gc/gc_test.go b/pkg/storage/gc/gc_test.go index 70d38465..e4406a88 100644 --- a/pkg/storage/gc/gc_test.go +++ b/pkg/storage/gc/gc_test.go @@ -358,7 +358,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -474,7 +474,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -570,7 +570,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -618,7 +618,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-docker1") So(err, ShouldBeNil) @@ -660,7 +660,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) processedRepos := make(map[string]struct{}) expectedRepos := []string{"gc-docker1", "gc-docker2", "gc-test1", "gc-test2", "gc-test3", "gc-test4", "retention"} @@ -730,7 +730,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -779,7 +779,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -840,7 +840,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -910,7 +910,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -948,7 +948,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -986,7 +986,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -1025,7 +1025,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -1070,7 +1070,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -1102,7 +1102,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -1148,7 +1148,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -1199,7 +1199,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) ctx, cancel := context.WithCancel(ctx) cancel() @@ -1227,7 +1227,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) blobUploadID, err := imgStore.NewBlobUpload(repoName) So(err, ShouldBeNil) @@ -1410,7 +1410,7 @@ func TestGarbageCollectDeletion(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, repoName) So(err, ShouldBeNil) @@ -1479,7 +1479,7 @@ func TestGarbageCollectDeletion(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = deleteTagInStorage(rootDir, repoName, "topindex") @@ -1552,7 +1552,7 @@ func TestGarbageCollectDeletion(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = deleteTagInStorage(rootDir, repoName, "topindex") @@ -1630,7 +1630,7 @@ func TestGarbageCollectDeletion(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, repoName) So(err, ShouldBeNil) @@ -1948,7 +1948,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -2052,7 +2052,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -2136,7 +2136,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -2182,7 +2182,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) processedRepos := make(map[string]struct{}) expectedRepos := []string{"gc-test1", "gc-test2", "gc-test3", "gc-test4", "retention"} @@ -2249,7 +2249,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -2296,7 +2296,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -2351,7 +2351,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -2411,7 +2411,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -2451,7 +2451,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err = gc.CleanRepo(ctx, "retention") So(err, ShouldBeNil) @@ -2483,7 +2483,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) err := gc.CleanRepo(ctx, "gc-test1") So(err, ShouldBeNil) @@ -2523,7 +2523,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) ctx, cancel := context.WithCancel(ctx) cancel() @@ -2551,7 +2551,7 @@ func TestGarbageCollectAndRetentionNoMetaDB(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) blobUploadID, err := imgStore.NewBlobUpload(repoName) So(err, ShouldBeNil) diff --git a/pkg/storage/gcs/gcs_test.go b/pkg/storage/gcs/gcs_test.go index b0b54203..fc49ce5b 100644 --- a/pkg/storage/gcs/gcs_test.go +++ b/pkg/storage/gcs/gcs_test.go @@ -423,6 +423,7 @@ func createObjectsStore(rootDir string, cacheDir string, dedupe bool) ( log := log.NewTestLogger() metrics := monitoring.NewMetricsServer(false, log) + defer metrics.Stop() var cacheDriver storageTypes.Cache @@ -1840,6 +1841,7 @@ func TestGCSMandatoryAnnotations(t *testing.T) { testLog := log.NewTestLogger() metrics := monitoring.NewMetricsServer(false, testLog) + defer metrics.Stop() storeDriver, imgStore, err := createObjectsStore(testDir, tdir, true) if err != nil { @@ -2012,6 +2014,8 @@ func TestGCSGarbageCollectImageManifest(t *testing.T) { testLog := log.NewTestLogger() audit := log.NewAuditLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, testLog) + defer metrics.Stop() ctx := context.Background() @@ -2045,7 +2049,7 @@ func TestGCSGarbageCollectImageManifest(t *testing.T) { }, }, }, - }, audit, testLog) + }, audit, testLog, metrics) // upload orphan blob upload, err := imgStore.NewBlobUpload(repoName) @@ -2229,6 +2233,8 @@ func TestGCSGarbageCollectImageIndex(t *testing.T) { testLog := log.NewTestLogger() audit := log.NewAuditLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, testLog) + defer metrics.Stop() ctx := context.Background() @@ -2263,7 +2269,7 @@ func TestGCSGarbageCollectImageIndex(t *testing.T) { }, }, }, - }, audit, testLog) + }, audit, testLog, metrics) // upload orphan blob upload, err := imgStore.NewBlobUpload(repoName) @@ -2381,6 +2387,8 @@ func TestGCSGarbageCollectChainedImageIndexes(t *testing.T) { testLog := log.NewTestLogger() audit := log.NewAuditLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, testLog) + defer metrics.Stop() ctx := context.Background() @@ -2415,7 +2423,7 @@ func TestGCSGarbageCollectChainedImageIndexes(t *testing.T) { }, }, }, - }, audit, testLog) + }, audit, testLog, metrics) // upload orphan blob upload, err := imgStore.NewBlobUpload(repoName) diff --git a/pkg/storage/local/local_test.go b/pkg/storage/local/local_test.go index 630016ac..78d15d9f 100644 --- a/pkg/storage/local/local_test.go +++ b/pkg/storage/local/local_test.go @@ -1151,7 +1151,7 @@ func FuzzRunGCRepo(f *testing.F) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) if err := gc.CleanRepo(context.Background(), data); err != nil { t.Error(err) @@ -2075,7 +2075,7 @@ func TestGarbageCollectForImageStore(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: 1 * time.Second, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) image := CreateDefaultVulnerableImage() err := WriteImageToFileSystem(image, repoName, "0.0.1", storage.StoreController{ @@ -2124,7 +2124,7 @@ func TestGarbageCollectForImageStore(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: 1 * time.Second, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) image := CreateDefaultVulnerableImage() err := WriteImageToFileSystem(image, repoName, "0.0.1", storage.StoreController{ @@ -2162,7 +2162,7 @@ func TestGarbageCollectForImageStore(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: 1 * time.Second, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) storeController := storage.StoreController{DefaultStore: imgStore} img := CreateRandomImage() @@ -2238,7 +2238,7 @@ func TestGarbageCollectForImageStore(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: 1 * time.Second, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) blobUploadID, err := imgStore.NewBlobUpload(repoName) So(err, ShouldBeNil) @@ -2316,7 +2316,7 @@ func TestGarbageCollectImageUnknownManifest(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: 1 * time.Second, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) unsupportedMediaType := "application/vnd.oci.artifact.manifest.v1+json" @@ -2496,7 +2496,7 @@ func TestGarbageCollectErrors(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: 500 * time.Millisecond, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) // create a blob/layer upload, err := imgStore.NewBlobUpload(repoName) diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index a76005a3..e9c891d2 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -2219,6 +2219,8 @@ func TestGarbageCollectImageManifest(t *testing.T) { t.Run(testcase.testCaseName, func(t *testing.T) { log := zlog.NewTestLogger() audit := zlog.NewAuditLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, log) + defer metrics.Stop() ctx := context.Background() @@ -2273,7 +2275,7 @@ func TestGarbageCollectImageManifest(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) repoName := "gc-long" @@ -2437,7 +2439,7 @@ func TestGarbageCollectImageManifest(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) // upload orphan blob upload, err := imgStore.NewBlobUpload(repoName) @@ -2705,7 +2707,7 @@ func TestGarbageCollectImageManifest(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: gcDelay, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) // first upload an image to the first repo and wait for GC timeout @@ -2908,6 +2910,8 @@ func TestGarbageCollectImageIndex(t *testing.T) { t.Run(testcase.testCaseName, func(t *testing.T) { log := zlog.NewTestLogger() audit := zlog.NewAuditLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, log) + defer metrics.Stop() ctx := context.Background() @@ -2954,7 +2958,7 @@ func TestGarbageCollectImageIndex(t *testing.T) { gc := gc.NewGarbageCollect(imgStore, mocks.MetaDBMock{}, gc.Options{ Delay: storageConstants.DefaultGCDelay, ImageRetention: DeleteReferrers, - }, audit, log) + }, audit, log, metrics) repoName := "gc-long" @@ -3084,7 +3088,7 @@ func TestGarbageCollectImageIndex(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) // upload orphan blob upload, err := imgStore.NewBlobUpload(repoName) @@ -3319,6 +3323,8 @@ func TestGarbageCollectChainedImageIndexes(t *testing.T) { t.Run(testcase.testCaseName, func(t *testing.T) { log := zlog.NewTestLogger() audit := zlog.NewAuditLogger("debug", "") + metrics := monitoring.NewMetricsServer(false, log) + defer metrics.Stop() ctx := context.Background() @@ -3376,7 +3382,7 @@ func TestGarbageCollectChainedImageIndexes(t *testing.T) { }, }, }, - }, audit, log) + }, audit, log, metrics) // upload orphan blob upload, err := imgStore.NewBlobUpload(repoName)