fix: preserve images when all its blobs are intact

Signed-off-by: Michal Minář <[email protected]>
openshift · Jun 15, 2018 · 413e952 · 413e952
1 parent 39bffa5
commit 413e952
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 9 deletions.
diff --git a/pkg/oc/admin/prune/imageprune/prune_test.go b/pkg/oc/admin/prune/imageprune/prune_test.go
@@ -2,6 +2,7 @@ package imageprune
 
 import (
 	"bytes"
+	"errors"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -73,7 +74,7 @@ func TestImagePruning(t *testing.T) {
 		imageStreamDeleterErr         error
 		layerDeleterErr               error
 		manifestDeleterErr            error
-		blobDeleterErr                error
+		blobDeleterErrorGetter        errorForSHA
 		expectedImageDeletions        []string
 		expectedStreamUpdates         []string
 		expectedLayerLinkDeletions    []string
@@ -406,7 +407,7 @@ func TestImagePruning(t *testing.T) {
 				testutil.UnmanagedImage("sha256:0000000000000000000000000000000000000000000000000000000000000000", "otherregistry/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000000", false, "", ""),
 				testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000002", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000002"),
 				testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000003", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000003"),
-				testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000004", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000004"),
+				testutil.ImageWithLayers("sha256:0000000000000000000000000000000000000000000000000000000000000004", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000004", nil, "layer1", "layer2"),
 			),
 			streams: testutil.StreamList(
 				testutil.Stream(registryHost, "foo", "bar", testutil.Tags(
@@ -418,12 +419,49 @@ func TestImagePruning(t *testing.T) {
 					),
 				)),
 			),
-			blobDeleterErr:                fmt.Errorf("err"),
+			blobDeleterErrorGetter: func(dgst string) error {
+				if dgst == "layer1" {
+					return errors.New("err")
+				}
+				return nil
+			},
 			expectedImageDeletions:        []string{"sha256:0000000000000000000000000000000000000000000000000000000000000004"},
 			expectedStreamUpdates:         []string{"foo/bar|sha256:0000000000000000000000000000000000000000000000000000000000000004"},
 			expectedManifestLinkDeletions: []string{registryURL + "|foo/bar|sha256:0000000000000000000000000000000000000000000000000000000000000004"},
-			expectedBlobDeletions:         []string{registryURL + "|" + "sha256:0000000000000000000000000000000000000000000000000000000000000004"},
-			expectedFailures:              []string{registryURL + "|" + "sha256:0000000000000000000000000000000000000000000000000000000000000004|err"},
+			expectedLayerLinkDeletions:    []string{registryURL + "|foo/bar|layer1", registryURL + "|foo/bar|layer2"},
+			expectedBlobDeletions: []string{
+				registryURL + "|" + "layer1",
+				registryURL + "|" + "layer2",
+				registryURL + "|" + "sha256:0000000000000000000000000000000000000000000000000000000000000004",
+			},
+			expectedFailures: []string{registryURL + "|" + "layer1|err"},
+		},
+
+		{
+			name: "keep image when all blob deletions fail",
+			images: testutil.ImageList(
+				testutil.UnmanagedImage("sha256:0000000000000000000000000000000000000000000000000000000000000000", "otherregistry/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000000", false, "", ""),
+				testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000002", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000002"),
+				testutil.Image("sha256:0000000000000000000000000000000000000000000000000000000000000003", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000003"),
+				testutil.ImageWithLayers("sha256:0000000000000000000000000000000000000000000000000000000000000004", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000004", nil, "layer1", "layer2"),
+			),
+			streams: testutil.StreamList(
+				testutil.Stream(registryHost, "foo", "bar", testutil.Tags(
+					testutil.Tag("latest",
+						testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000000", "otherregistry/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000000"),
+						testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000002", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000002"),
+						testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000003", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000003"),
+						testutil.TagEvent("sha256:0000000000000000000000000000000000000000000000000000000000000004", registryHost+"/foo/bar@sha256:0000000000000000000000000000000000000000000000000000000000000004"),
+					),
+				)),
+			),
+			blobDeleterErrorGetter:        func(dgst string) error { return errors.New("err") },
+			expectedImageDeletions:        []string{},
+			expectedStreamUpdates:         []string{"foo/bar|sha256:0000000000000000000000000000000000000000000000000000000000000004"},
+			expectedManifestLinkDeletions: []string{registryURL + "|foo/bar|sha256:0000000000000000000000000000000000000000000000000000000000000004"},
+			expectedLayerLinkDeletions:    []string{registryURL + "|foo/bar|layer1", registryURL + "|foo/bar|layer2"},
+			expectedBlobDeletions:         []string{registryURL + "|layer1", registryURL + "|layer2", registryURL + "|" + "sha256:0000000000000000000000000000000000000000000000000000000000000004"},
+			expectedFailures:              []string{registryURL + "|" + "layer1|err", registryURL + "|" + "layer2|err", registryURL + "|sha256:0000000000000000000000000000000000000000000000000000000000000004|err"},
 		},
 
 		{
@@ -1195,7 +1233,7 @@ func TestImagePruning(t *testing.T) {
 			imageDeleter, imageDeleterFactory := newFakeImageDeleter(test.imageDeleterErr)
 			streamDeleter := &fakeImageStreamDeleter{err: test.imageStreamDeleterErr, invocations: sets.NewString()}
 			layerLinkDeleter := &fakeLayerLinkDeleter{err: test.layerDeleterErr, invocations: sets.NewString()}
-			blobDeleter := &fakeBlobDeleter{err: test.blobDeleterErr, invocations: sets.NewString()}
+			blobDeleter := &fakeBlobDeleter{getError: test.blobDeleterErrorGetter, invocations: sets.NewString()}
 			manifestDeleter := &fakeManifestDeleter{err: test.manifestDeleterErr, invocations: sets.NewString()}
 
 			deletions, failures := p.Prune(imageDeleterFactory, streamDeleter, layerLinkDeleter, blobDeleter, manifestDeleter)
@@ -2166,10 +2204,12 @@ func (p *fakeImageStreamDeleter) NotifyImageStreamPrune(stream *imageapi.ImageSt
 	return
 }
 
+type errorForSHA func(dgst string) error
+
 type fakeBlobDeleter struct {
 	mutex       sync.Mutex
 	invocations sets.String
-	err         error
+	getError    errorForSHA
 }
 
 var _ BlobDeleter = &fakeBlobDeleter{}
@@ -2178,7 +2218,10 @@ func (p *fakeBlobDeleter) DeleteBlob(registryClient *http.Client, registryURL *u
 	p.mutex.Lock()
 	defer p.mutex.Unlock()
 	p.invocations.Insert(fmt.Sprintf("%s|%s", registryURL.String(), blob))
-	return p.err
+	if p.getError == nil {
+		return nil
+	}
+	return p.getError(blob)
 }
 
 type fakeLayerLinkDeleter struct {

diff --git a/pkg/oc/admin/prune/imageprune/worker.go b/pkg/oc/admin/prune/imageprune/worker.go
@@ -237,7 +237,7 @@ func (w *worker) prune(job *Job) *JobResult {
 			w.layerLinkPruner,
 		))
 
-		blobDeletions, blobFailures := pruneBlobs(
+		blobDeletions, blobFailures = pruneBlobs(
 			w.registryClient,
 			w.registryURL,
 			job.Components,