From 898fdb48a1f694b4d317ad08e74d37254a5addfc Mon Sep 17 00:00:00 2001 From: Tony Holdstock-Brown Date: Mon, 25 Apr 2016 21:14:00 -0700 Subject: [PATCH 1/3] Ensure GC continues marking if _manifests is nonexistent Signed-off-by: Tony Holdstock-Brown --- docs/garbagecollect.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/garbagecollect.go b/docs/garbagecollect.go index 7e1d97d9..65d432e0 100644 --- a/docs/garbagecollect.go +++ b/docs/garbagecollect.go @@ -96,6 +96,17 @@ func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis return nil }) + if err != nil { + // In certain situations such as unfinished uploads, deleting all + // tags in S3 or removing the _manifests folder manually, this + // error may be of type PathNotFound. + // + // In these cases we can continue marking other manifests safely. + if _, ok := err.(driver.PathNotFoundError); ok { + return nil + } + } + return err }) From 3a034b477e827559fe72c0a01bed12f2f758488c Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Wed, 27 Apr 2016 11:49:01 -0700 Subject: [PATCH 2/3] Move garbage collect code into storage package Signed-off-by: Richard Scothern --- docs/root.go | 56 +++++++++++++ docs/{ => storage}/garbagecollect.go | 96 ++++++----------------- docs/{ => storage}/garbagecollect_test.go | 11 ++- 3 files changed, 85 insertions(+), 78 deletions(-) rename docs/{ => storage}/garbagecollect.go (60%) rename docs/{ => storage}/garbagecollect_test.go (96%) diff --git a/docs/root.go b/docs/root.go index 46338b46..7a7d44cb 100644 --- a/docs/root.go +++ b/docs/root.go @@ -1,7 +1,14 @@ package registry import ( + "fmt" + "os" + + "github.com/docker/distribution/context" + "github.com/docker/distribution/registry/storage" + "github.com/docker/distribution/registry/storage/driver/factory" "github.com/docker/distribution/version" + "github.com/docker/libtrust" "github.com/spf13/cobra" ) @@ -10,6 +17,7 @@ var showVersion bool func init() { RootCmd.AddCommand(ServeCmd) RootCmd.AddCommand(GCCmd) + GCCmd.Flags().BoolVarP(&dryRun, "dry-run", "d", false, "do everything except remove the blobs") RootCmd.Flags().BoolVarP(&showVersion, "version", "v", false, "show the version and exit") } @@ -26,3 +34,51 @@ var RootCmd = &cobra.Command{ cmd.Usage() }, } + +var dryRun bool + +// GCCmd is the cobra command that corresponds to the garbage-collect subcommand +var GCCmd = &cobra.Command{ + Use: "garbage-collect ", + Short: "`garbage-collect` deletes layers not referenced by any manifests", + Long: "`garbage-collect` deletes layers not referenced by any manifests", + Run: func(cmd *cobra.Command, args []string) { + config, err := resolveConfiguration(args) + if err != nil { + fmt.Fprintf(os.Stderr, "configuration error: %v\n", err) + cmd.Usage() + os.Exit(1) + } + + driver, err := factory.Create(config.Storage.Type(), config.Storage.Parameters()) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to construct %s driver: %v", config.Storage.Type(), err) + os.Exit(1) + } + + ctx := context.Background() + ctx, err = configureLogging(ctx, config) + if err != nil { + fmt.Fprintf(os.Stderr, "unable to configure logging with config: %s", err) + os.Exit(1) + } + + k, err := libtrust.GenerateECP256PrivateKey() + if err != nil { + fmt.Fprint(os.Stderr, err) + os.Exit(1) + } + + registry, err := storage.NewRegistry(ctx, driver, storage.DisableSchema1Signatures, storage.Schema1SigningKey(k)) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to construct registry: %v", err) + os.Exit(1) + } + + err = storage.MarkAndSweep(ctx, driver, registry, dryRun) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to garbage collect: %v", err) + os.Exit(1) + } + }, +} diff --git a/docs/garbagecollect.go b/docs/storage/garbagecollect.go similarity index 60% rename from docs/garbagecollect.go rename to docs/storage/garbagecollect.go index 65d432e0..be64b847 100644 --- a/docs/garbagecollect.go +++ b/docs/storage/garbagecollect.go @@ -1,8 +1,7 @@ -package registry +package storage import ( "fmt" - "os" "github.com/docker/distribution" "github.com/docker/distribution/context" @@ -10,21 +9,15 @@ import ( "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" "github.com/docker/distribution/reference" - "github.com/docker/distribution/registry/storage" "github.com/docker/distribution/registry/storage/driver" - "github.com/docker/distribution/registry/storage/driver/factory" - "github.com/docker/libtrust" - "github.com/spf13/cobra" ) func emit(format string, a ...interface{}) { - if dryRun { - fmt.Printf(format+"\n", a...) - } + fmt.Printf(format+"\n", a...) } -func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, registry distribution.Namespace) error { - +// MarkAndSweep performs a mark and sweep of registry data +func MarkAndSweep(ctx context.Context, storageDriver driver.StorageDriver, registry distribution.Namespace, dryRun bool) error { repositoryEnumerator, ok := registry.(distribution.RepositoryEnumerator) if !ok { return fmt.Errorf("unable to convert Namespace to RepositoryEnumerator") @@ -33,7 +26,9 @@ func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis // mark markSet := make(map[digest.Digest]struct{}) err := repositoryEnumerator.Enumerate(ctx, func(repoName string) error { - emit(repoName) + if dryRun { + emit(repoName) + } var err error named, err := reference.ParseNamed(repoName) @@ -57,7 +52,9 @@ func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis err = manifestEnumerator.Enumerate(ctx, func(dgst digest.Digest) error { // Mark the manifest's blob - emit("%s: marking manifest %s ", repoName, dgst) + if dryRun { + emit("%s: marking manifest %s ", repoName, dgst) + } markSet[dgst] = struct{}{} manifest, err := manifestService.Get(ctx, dgst) @@ -68,7 +65,9 @@ func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis descriptors := manifest.References() for _, descriptor := range descriptors { markSet[descriptor.Digest] = struct{}{} - emit("%s: marking blob %s", repoName, descriptor.Digest) + if dryRun { + emit("%s: marking blob %s", repoName, descriptor.Digest) + } } switch manifest.(type) { @@ -82,13 +81,17 @@ func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis return fmt.Errorf("failed to get signatures for signed manifest: %v", err) } for _, signatureDigest := range signatures { - emit("%s: marking signature %s", repoName, signatureDigest) + if dryRun { + emit("%s: marking signature %s", repoName, signatureDigest) + } markSet[signatureDigest] = struct{}{} } break case *schema2.DeserializedManifest: config := manifest.(*schema2.DeserializedManifest).Config - emit("%s: marking configuration %s", repoName, config.Digest) + if dryRun { + emit("%s: marking configuration %s", repoName, config.Digest) + } markSet[config.Digest] = struct{}{} break } @@ -127,13 +130,14 @@ func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis if err != nil { return fmt.Errorf("error enumerating blobs: %v", err) } - - emit("\n%d blobs marked, %d blobs eligible for deletion", len(markSet), len(deleteSet)) + if dryRun { + emit("\n%d blobs marked, %d blobs eligible for deletion", len(markSet), len(deleteSet)) + } // Construct vacuum - vacuum := storage.NewVacuum(ctx, storageDriver) + vacuum := NewVacuum(ctx, storageDriver) for dgst := range deleteSet { - emit("blob eligible for deletion: %s", dgst) if dryRun { + emit("blob eligible for deletion: %s", dgst) continue } err = vacuum.RemoveBlob(string(dgst)) @@ -144,55 +148,3 @@ func markAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis return err } - -func init() { - GCCmd.Flags().BoolVarP(&dryRun, "dry-run", "d", false, "do everything except remove the blobs") -} - -var dryRun bool - -// GCCmd is the cobra command that corresponds to the garbage-collect subcommand -var GCCmd = &cobra.Command{ - Use: "garbage-collect ", - Short: "`garbage-collect` deletes layers not referenced by any manifests", - Long: "`garbage-collect` deletes layers not referenced by any manifests", - Run: func(cmd *cobra.Command, args []string) { - config, err := resolveConfiguration(args) - if err != nil { - fmt.Fprintf(os.Stderr, "configuration error: %v\n", err) - cmd.Usage() - os.Exit(1) - } - - driver, err := factory.Create(config.Storage.Type(), config.Storage.Parameters()) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to construct %s driver: %v", config.Storage.Type(), err) - os.Exit(1) - } - - ctx := context.Background() - ctx, err = configureLogging(ctx, config) - if err != nil { - fmt.Fprintf(os.Stderr, "unable to configure logging with config: %s", err) - os.Exit(1) - } - - k, err := libtrust.GenerateECP256PrivateKey() - if err != nil { - fmt.Fprint(os.Stderr, err) - os.Exit(1) - } - - registry, err := storage.NewRegistry(ctx, driver, storage.DisableSchema1Signatures, storage.Schema1SigningKey(k)) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to construct registry: %v", err) - os.Exit(1) - } - - err = markAndSweep(ctx, driver, registry) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to garbage collect: %v", err) - os.Exit(1) - } - }, -} diff --git a/docs/garbagecollect_test.go b/docs/storage/garbagecollect_test.go similarity index 96% rename from docs/garbagecollect_test.go rename to docs/storage/garbagecollect_test.go index dd5fadd5..ff4a3df2 100644 --- a/docs/garbagecollect_test.go +++ b/docs/storage/garbagecollect_test.go @@ -1,4 +1,4 @@ -package registry +package storage import ( "io" @@ -8,7 +8,6 @@ import ( "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/reference" - "github.com/docker/distribution/registry/storage" "github.com/docker/distribution/registry/storage/driver" "github.com/docker/distribution/registry/storage/driver/inmemory" "github.com/docker/distribution/testutil" @@ -22,7 +21,7 @@ type image struct { func createRegistry(t *testing.T, driver driver.StorageDriver) distribution.Namespace { ctx := context.Background() - registry, err := storage.NewRegistry(ctx, driver, storage.EnableDelete) + registry, err := NewRegistry(ctx, driver, EnableDelete) if err != nil { t.Fatalf("Failed to construct namespace") } @@ -161,7 +160,7 @@ func TestNoDeletionNoEffect(t *testing.T) { } // Run GC - err = markAndSweep(context.Background(), inmemoryDriver, registry) + err = MarkAndSweep(context.Background(), inmemoryDriver, registry, false) if err != nil { t.Fatalf("Failed mark and sweep: %v", err) } @@ -193,7 +192,7 @@ func TestDeletionHasEffect(t *testing.T) { manifests.Delete(ctx, image3.manifestDigest) // Run GC - err = markAndSweep(context.Background(), inmemoryDriver, registry) + err = MarkAndSweep(context.Background(), inmemoryDriver, registry, false) if err != nil { t.Fatalf("Failed mark and sweep: %v", err) } @@ -327,7 +326,7 @@ func TestOrphanBlobDeleted(t *testing.T) { uploadRandomSchema2Image(t, repo) // Run GC - err = markAndSweep(context.Background(), inmemoryDriver, registry) + err = MarkAndSweep(context.Background(), inmemoryDriver, registry, false) if err != nil { t.Fatalf("Failed mark and sweep: %v", err) } From 63d28d3b81dda6fd95adf1244a36afe80dc32434 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Wed, 27 Apr 2016 13:24:22 -0700 Subject: [PATCH 3/3] Add a test with a missing _manifests directory Signed-off-by: Richard Scothern --- docs/storage/garbagecollect_test.go | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/storage/garbagecollect_test.go b/docs/storage/garbagecollect_test.go index ff4a3df2..a0ba154b 100644 --- a/docs/storage/garbagecollect_test.go +++ b/docs/storage/garbagecollect_test.go @@ -2,6 +2,7 @@ package storage import ( "io" + "path" "testing" "github.com/docker/distribution" @@ -176,6 +177,37 @@ func TestNoDeletionNoEffect(t *testing.T) { } } +func TestGCWithMissingManifests(t *testing.T) { + ctx := context.Background() + d := inmemory.New() + + registry := createRegistry(t, d) + repo := makeRepository(t, registry, "testrepo") + uploadRandomSchema1Image(t, repo) + + // Simulate a missing _manifests directory + revPath, err := pathFor(manifestRevisionsPathSpec{"testrepo"}) + if err != nil { + t.Fatal(err) + } + + _manifestsPath := path.Dir(revPath) + err = d.Delete(ctx, _manifestsPath) + if err != nil { + t.Fatal(err) + } + + err = MarkAndSweep(context.Background(), d, registry, false) + if err != nil { + t.Fatalf("Failed mark and sweep: %v", err) + } + + blobs := allBlobs(t, registry) + if len(blobs) > 0 { + t.Errorf("unexpected blobs after gc") + } +} + func TestDeletionHasEffect(t *testing.T) { ctx := context.Background() inmemoryDriver := inmemory.New()