Merge pull request #17020 from legionus/dockerregistry-hard-prune-fix-layer-links

Automatic merge from submit-queue (batch tested with PRs 17020, 17026, 17000, 17010).

Dockerregistry hard prune fix layer links

Fixes [BZ#1483930](https://bugzilla.redhat.com/show_bug.cgi?id=1483930)

Author: openshift-merge-robot

pkg/cmd/dockerregistry/dockerregistry.go

@@ -104,7 +104,15 @@ func ExecutePruner(configFile io.Reader, dryRun bool) {
 		log.Fatalf("error creating registry: %s", err)
 	}
 
-	stats, err := prune.Prune(ctx, storageDriver, registry, registryClient, dryRun)
+	var pruner prune.Pruner
+
+	if dryRun {
+		pruner = &prune.DryRunPruner{}
+	} else {
+		pruner = &prune.RegistryPruner{storageDriver}
+	}
+
+	stats, err := prune.Prune(ctx, registry, registryClient, pruner)
 	if err != nil {
 		log.Error(err)
 	}

pkg/dockerregistry/server/prune/prune.go

@@ -19,6 +19,132 @@ import (
 	imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1"
 )
 
+// Pruner defines a common set of operations for pruning
+type Pruner interface {
+	DeleteRepository(ctx context.Context, reponame string) error
+	DeleteManifestLink(ctx context.Context, svc distribution.ManifestService, reponame string, dgst digest.Digest) error
+	DeleteBlob(ctx context.Context, dgst digest.Digest) error
+}
+
+// DryRunPruner prints information about each object that going to remove.
+type DryRunPruner struct{}
+
+var _ Pruner = &DryRunPruner{}
+
+func (p *DryRunPruner) DeleteRepository(ctx context.Context, reponame string) error {
+	logger := context.GetLogger(ctx)
+	logger.Printf("Would delete repository: %s", reponame)
+	return nil
+}
+
+func (p *DryRunPruner) DeleteManifestLink(ctx context.Context, svc distribution.ManifestService, reponame string, dgst digest.Digest) error {
+	logger := context.GetLogger(ctx)
+	logger.Printf("Would delete manifest link: %s@%s", reponame, dgst)
+	return nil
+}
+
+func (p *DryRunPruner) DeleteBlob(ctx context.Context, dgst digest.Digest) error {
+	logger := context.GetLogger(ctx)
+	logger.Printf("Would delete blob: %s", dgst)
+	return nil
+}
+
+// RegistryPruner deletes objects.
+type RegistryPruner struct {
+	StorageDriver driver.StorageDriver
+}
+
+var _ Pruner = &RegistryPruner{}
+
+// DeleteRepository removes a repository directory from the storage
+func (p *RegistryPruner) DeleteRepository(ctx context.Context, reponame string) error {
+	vacuum := storage.NewVacuum(ctx, p.StorageDriver)
+
+	// Log message will be generated by RemoveRepository with loglevel=info.
+	if err := vacuum.RemoveRepository(reponame); err != nil {
+		return fmt.Errorf("unable to remove the repository %s: %v", reponame, err)
+	}
+
+	return nil
+}
+
+// DeleteManifestLink removes a manifest link from the storage
+func (p *RegistryPruner) DeleteManifestLink(ctx context.Context, svc distribution.ManifestService, reponame string, dgst digest.Digest) error {
+	logger := context.GetLogger(ctx)
+
+	logger.Printf("Deleting manifest link: %s@%s", reponame, dgst)
+	if err := svc.Delete(ctx, dgst); err != nil {
+		return fmt.Errorf("failed to delete the manifest link %s@%s: %v", reponame, dgst, err)
+	}
+
+	return nil
+}
+
+// DeleteBlob removes a blob from the storage
+func (p *RegistryPruner) DeleteBlob(ctx context.Context, dgst digest.Digest) error {
+	vacuum := storage.NewVacuum(ctx, p.StorageDriver)
+
+	// Log message will be generated by RemoveBlob with loglevel=info.
+	if err := vacuum.RemoveBlob(string(dgst)); err != nil {
+		return fmt.Errorf("failed to delete the blob %s: %v", dgst, err)
+	}
+
+	return nil
+}
+
+// garbageCollector holds objects for later deletion. If the object is replaced,
+// then the previous one will be deleted.
+type garbageCollector struct {
+	Pruner Pruner
+	Ctx    context.Context
+
+	repoName string
+
+	manifestService distribution.ManifestService
+	manifestRepo    string
+	manifestLink    digest.Digest
+}
+
+func (gc *garbageCollector) AddRepository(repoName string) error {
+	// If the place is occupied, then it is necessary to clean it.
+	if err := gc.Collect(); err != nil {
+		return err
+	}
+
+	gc.repoName = repoName
+
+	return nil
+}
+
+func (gc *garbageCollector) AddManifestLink(svc distribution.ManifestService, repoName string, dgst digest.Digest) error {
+	// If the place is occupied, then it is necessary to clean it.
+	if err := gc.Collect(); err != nil {
+		return err
+	}
+
+	gc.manifestService = svc
+	gc.manifestRepo = repoName
+	gc.manifestLink = dgst
+
+	return nil
+}
+
+func (gc *garbageCollector) Collect() error {
+	if len(gc.manifestLink) > 0 {
+		if err := gc.Pruner.DeleteManifestLink(gc.Ctx, gc.manifestService, gc.manifestRepo, gc.manifestLink); err != nil {
+			return err
+		}
+		gc.manifestLink = ""
+	}
+	if len(gc.repoName) > 0 {
+		if err := gc.Pruner.DeleteRepository(gc.Ctx, gc.repoName); err != nil {
+			return err
+		}
+		gc.repoName = ""
+	}
+	return nil
+}
+
 func imageStreamHasManifestDigest(is *imageapiv1.ImageStream, dgst digest.Digest) bool {
 	for _, tagEventList := range is.Status.Tags {
 		for _, tagEvent := range tagEventList.Items {
@@ -42,7 +168,7 @@ type Summary struct {
 //
 // TODO(dmage): remove layer links to a blob if the blob is removed or it doesn't belong to the ImageStream.
 // TODO(dmage): keep young blobs (docker/distribution#2297).
-func Prune(ctx context.Context, storageDriver driver.StorageDriver, registry distribution.Namespace, registryClient client.RegistryClient, dryRun bool) (Summary, error) {
+func Prune(ctx context.Context, registry distribution.Namespace, registryClient client.RegistryClient, pruner Pruner) (Summary, error) {
 	logger := context.GetLogger(ctx)
 
 	repositoryEnumerator, ok := registry.(distribution.RepositoryEnumerator)
@@ -84,7 +210,15 @@ func Prune(ctx context.Context, storageDriver driver.StorageDriver, registry dis
 
 	var stats Summary
 
-	var reposToDelete []string
+	// The Enumerate calls a Stat() on each file or directory in the tree before call our handler.
+	// Therefore, we can not delete subdirectories from the handler. On some types of storage (S3),
+	// this can lead to an error in the Enumerate.
+	// We are waiting for the completion of our handler and perform deferred deletion of objects.
+	gc := &garbageCollector{
+		Ctx:    ctx,
+		Pruner: pruner,
+	}
+
 	err = repositoryEnumerator.Enumerate(ctx, func(repoName string) error {
 		logger.Debugln("Processing repository", repoName)
 
@@ -101,11 +235,7 @@ func Prune(ctx context.Context, storageDriver driver.StorageDriver, registry dis
 		is, err := oc.ImageStreams(ref.Namespace).Get(ref.Name, metav1.GetOptions{})
 		if kerrors.IsNotFound(err) {
 			logger.Printf("The image stream %s/%s is not found, will remove the whole repository", ref.Namespace, ref.Name)
-
-			// We cannot delete the repository at this point, because it would break Enumerate.
-			reposToDelete = append(reposToDelete, repoName)
-
-			return nil
+			return gc.AddRepository(repoName)
 		} else if err != nil {
 			return fmt.Errorf("failed to get the image stream %s: %v", repoName, err)
 		}
@@ -131,17 +261,7 @@ func Prune(ctx context.Context, storageDriver driver.StorageDriver, registry dis
 				return nil
 			}
 
-			if dryRun {
-				logger.Printf("Would delete manifest link: %s@%s", repoName, dgst)
-				return nil
-			}
-
-			logger.Printf("Deleting manifest link: %s@%s", repoName, dgst)
-			if err := manifestService.Delete(ctx, dgst); err != nil {
-				return fmt.Errorf("failed to delete the manifest link %s@%s: %v", repoName, dgst, err)
-			}
-
-			return nil
+			return gc.AddManifestLink(manifestService, repoName, dgst)
 		})
 		if e, ok := err.(driver.PathNotFoundError); ok {
 			logger.Printf("Skipped manifest link pruning for the repository %s: %v", repoName, e)
@@ -158,18 +278,8 @@ func Prune(ctx context.Context, storageDriver driver.StorageDriver, registry dis
 		return stats, err
 	}
 
-	vacuum := storage.NewVacuum(ctx, storageDriver)
-
-	logger.Debugln("Removing repositories")
-	for _, repoName := range reposToDelete {
-		if dryRun {
-			logger.Printf("Would delete repository: %s", repoName)
-			continue
-		}
-
-		if err = vacuum.RemoveRepository(repoName); err != nil {
-			return stats, fmt.Errorf("unable to remove the repository %s: %v", repoName, err)
-		}
+	if err := gc.Collect(); err != nil {
+		return stats, err
 	}
 
 	logger.Debugln("Processing blobs")
@@ -188,16 +298,7 @@ func Prune(ctx context.Context, storageDriver driver.StorageDriver, registry dis
 		stats.Blobs++
 		stats.DiskSpace += desc.Size
 
-		if dryRun {
-			logger.Printf("Would delete blob: %s", dgst)
-			return nil
-		}
-
-		if err := vacuum.RemoveBlob(string(dgst)); err != nil {
-			return fmt.Errorf("failed to delete the blob %s: %v", dgst, err)
-		}
-
-		return nil
+		return pruner.DeleteBlob(ctx, dgst)
 	})
 	return stats, err
 }