Image Pruning

Update layer deletion responses to include per-layer errors.
openshift · May 6, 2015 · e7b0136 · e7b0136
1 parent 28939bf
commit e7b0136
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 46 deletions.
diff --git a/pkg/cmd/dockerregistry/dockerregistry.go b/pkg/cmd/dockerregistry/dockerregistry.go
@@ -59,7 +59,7 @@ func (r DeleteLayersRequest) AddStream(layer, stream string) {
 
 type DeleteLayersResponse struct {
 	Result string
-	Errors []string
+	Errors map[string][]string
 }
 
 // deleteLayerFunc returns an http.HandlerFunc that is able to fully delete a
@@ -89,11 +89,10 @@ func deleteLayerFunc(app *handlers.App) http.HandlerFunc {
 		}
 
 		adminService := app.Registry().AdminService()
-		errs := []error{}
+		errs := map[string][]error{}
 		for layer, repos := range deletions {
 			log.Infof("Deleting layer=%q, repos=%v", layer, repos)
-			layerErrs := adminService.DeleteLayer(layer, repos)
-			errs = append(errs, layerErrs...)
+			errs[layer] = adminService.DeleteLayer(layer, repos)
 		}
 
 		log.Infof("errs=%v", errs)
@@ -108,10 +107,14 @@ func deleteLayerFunc(app *handlers.App) http.HandlerFunc {
 
 		response := DeleteLayersResponse{
 			Result: result,
+			Errors: map[string][]string{},
 		}
 
-		for _, err := range errs {
-			response.Errors = append(response.Errors, err.Error())
+		for layer, layerErrors := range errs {
+			response.Errors[layer] = []string{}
+			for _, err := range layerErrors {
+				response.Errors[layer] = append(response.Errors[layer], err.Error())
+			}
 		}
 
 		buf, err := json.Marshal(&response)

diff --git a/pkg/cmd/experimental/imageprune/imageprune.go b/pkg/cmd/experimental/imageprune/imageprune.go
@@ -64,7 +64,7 @@ func NewCmdPruneImages(f *clientcmd.Factory, parentName, name string, out io.Wri
 					prune.DescribingImagePruneFunc(out)(image, referencedStreams)
 					return prune.DeletingImagePruneFunc(osClient.Images(), osClient)(image, referencedStreams)
 				}
-				layerPruneFunc = func(registryURL string, req dockerregistry.DeleteLayersRequest) []error {
+				layerPruneFunc = func(registryURL string, req dockerregistry.DeleteLayersRequest) (error, map[string][]error) {
 					prune.DescribingLayerPruneFunc(out)(registryURL, req)
 					return prune.DeletingLayerPruneFunc(http.DefaultClient)(registryURL, req)
 				}

diff --git a/pkg/image/prune/imagepruner.go b/pkg/image/prune/imagepruner.go
@@ -40,7 +40,7 @@ type ImagePruneFunc func(image *imageapi.Image, streams []*imageapi.ImageStream)
 // LayerPruneFunc is a function that is invoked for each registry, along with
 // a DeleteLayersRequest that contains the layers that can be pruned and the
 // image stream names that reference each layer.
-type LayerPruneFunc func(registryURL string, req dockerregistry.DeleteLayersRequest) []error
+type LayerPruneFunc func(registryURL string, req dockerregistry.DeleteLayersRequest) (requestError error, layerErrors map[string][]error)
 
 // ImagePruner knows how to prune images and layers.
 type ImagePruner interface {
@@ -103,7 +103,7 @@ func NewImagePruner(minimumAgeInMinutesToPrune int, tagRevisionsToKeep int, imag
 		return nil, fmt.Errorf("Error listing image streams: %v", err)
 	}
 
-	allPods, err := pods.Pods(kapi.NamespaceAll).List(labels.Everything())
+	allPods, err := pods.Pods(kapi.NamespaceAll).List(labels.Everything(), fields.Everything())
 	if err != nil {
 		return nil, fmt.Errorf("Error listing pods: %v", err)
 	}
@@ -585,7 +585,8 @@ func pruneLayers(g graph.Graph, layerNodes []*graph.ImageLayerNode, layerPruneFu
 
 	for registryURL, req := range registryDeletionRequests {
 		glog.V(4).Infof("Invoking layerPruneFunc with registry=%q, req=%#v", registryURL, req)
-		layerPruneFunc(registryURL, req)
+		requestError, layerErrors := layerPruneFunc(registryURL, req)
+		glog.V(4).Infof("layerPruneFunc requestError=%v, layerErrors=%#v", requestError, layerErrors)
 	}
 }
 
@@ -647,9 +648,12 @@ func DeletingImagePruneFunc(images client.ImageInterface, streams client.ImageSt
 // DescribingLayerPruneFunc returns a LayerPruneFunc that writes information
 // about the layers that are eligible for pruning to out.
 func DescribingLayerPruneFunc(out io.Writer) LayerPruneFunc {
-	return func(registryURL string, deletions dockerregistry.DeleteLayersRequest) []error {
+	return func(registryURL string, deletions dockerregistry.DeleteLayersRequest) (error, map[string][]error) {
+		result := map[string][]error{}
+
 		fmt.Fprintf(out, "Pruning from registry %q\n", registryURL)
 		for layer, repos := range deletions {
+			result[layer] = []error{}
 			fmt.Fprintf(out, "\tLayer %q\n", layer)
 			if len(repos) > 0 {
 				fmt.Fprint(out, "\tReferenced streams:\n")
@@ -658,7 +662,7 @@ func DescribingLayerPruneFunc(out io.Writer) LayerPruneFunc {
 				fmt.Fprintf(out, "\t\t%q\n", repo)
 			}
 		}
-		return []error{}
+		return nil, result
 	}
 }
 
@@ -672,53 +676,56 @@ func DescribingLayerPruneFunc(out io.Writer) LayerPruneFunc {
 // key being a layer, and each value being a list of Docker image repository
 // names referenced by the layer.
 func DeletingLayerPruneFunc(registryClient *http.Client) LayerPruneFunc {
-	return func(registryURL string, deletions dockerregistry.DeleteLayersRequest) []error {
-		errs := []error{}
-
+	return func(registryURL string, deletions dockerregistry.DeleteLayersRequest) (requestError error, layerErrors map[string][]error) {
 		glog.V(4).Infof("Starting pruning of layers from %q, req %#v", registryURL, deletions)
 		body, err := json.Marshal(&deletions)
 		if err != nil {
 			glog.Errorf("Error marshaling request body: %v", err)
-			return []error{fmt.Errorf("Error creating request body: %v", err)}
+			return fmt.Errorf("Error creating request body: %v", err), nil
 		}
 
 		//TODO https
 		req, err := http.NewRequest("DELETE", fmt.Sprintf("http://%s/admin/layers", registryURL), bytes.NewReader(body))
 		if err != nil {
 			glog.Errorf("Error creating request: %v", err)
-			return []error{fmt.Errorf("Error creating request: %v", err)}
+			return fmt.Errorf("Error creating request: %v", err), nil
 		}
 
 		glog.V(4).Infof("Sending request to registry")
 		resp, err := registryClient.Do(req)
 		if err != nil {
 			glog.Errorf("Error sending request: %v", err)
-			return []error{fmt.Errorf("Error sending request: %v", err)}
+			return fmt.Errorf("Error sending request: %v", err), nil
 		}
 		defer resp.Body.Close()
 
 		buf, err := ioutil.ReadAll(resp.Body)
 		if err != nil {
 			glog.Errorf("Error reading response body: %v", err)
-			return []error{fmt.Errorf("Error reading response body: %v", err)}
+			return fmt.Errorf("Error reading response body: %v", err), nil
 		}
 
 		if resp.StatusCode != http.StatusOK {
 			glog.Errorf("Unexpected status code in response: %d", resp.StatusCode)
-			return []error{fmt.Errorf("Unexpected status code %d in response %s", resp.StatusCode, buf)}
+			return fmt.Errorf("Unexpected status code %d in response %s", resp.StatusCode, buf), nil
 		}
 
 		var deleteResponse dockerregistry.DeleteLayersResponse
 		if err := json.Unmarshal(buf, &deleteResponse); err != nil {
 			glog.Errorf("Error unmarshaling response: %v", err)
-			return []error{fmt.Errorf("Error unmarshaling response: %v", err)}
+			return fmt.Errorf("Error unmarshaling response: %v", err), nil
 		}
 
-		for _, e := range deleteResponse.Errors {
-			errs = append(errs, errors.New(e))
+		errs := map[string][]error{}
+
+		for layer, layerErrors := range deleteResponse.Errors {
+			errs[layer] = []error{}
+			for _, err := range layerErrors {
+				errs[layer] = append(errs[layer], errors.New(err))
+			}
 		}
 
-		return errs
+		return nil, errs
 	}
 }
 

diff --git a/pkg/image/prune/imagepruner_test.go b/pkg/image/prune/imagepruner_test.go
@@ -592,8 +592,8 @@ func TestImagePruning(t *testing.T) {
 			return []error{}
 		}
 
-		layerPruneFunc := func(registryURL string, req dockerregistry.DeleteLayersRequest) []error {
-			return []error{}
+		layerPruneFunc := func(registryURL string, req dockerregistry.DeleteLayersRequest) (error, map[string][]error) {
+			return nil, map[string][]error{}
 		}
 
 		p.Run(imagePruneFunc, layerPruneFunc)
@@ -789,7 +789,7 @@ func TestLayerPruning(t *testing.T) {
 			return []error{}
 		}
 
-		layerPruneFunc := func(registryURL string, req dockerregistry.DeleteLayersRequest) []error {
+		layerPruneFunc := func(registryURL string, req dockerregistry.DeleteLayersRequest) (error, map[string][]error) {
 			registryDeletions, ok := actualDeletions[registryURL]
 			if !ok {
 				registryDeletions = util.NewStringSet()
@@ -807,7 +807,7 @@ func TestLayerPruning(t *testing.T) {
 			actualDeletions[registryURL] = registryDeletions
 			actualUpdatedStreams[registryURL] = streamUpdates
 
-			return []error{}
+			return nil, map[string][]error{}
 		}
 
 		p := newImagePruner(60, 1, &test.images, &test.streams, &kapi.PodList{}, &kapi.ReplicationControllerList{}, &buildapi.BuildConfigList{}, &buildapi.BuildList{}, &deployapi.DeploymentConfigList{})
@@ -864,21 +864,22 @@ func TestDeletingLayerPruneFunc(t *testing.T) {
 		simulateClientError        bool
 		registryResponseStatusCode int
 		registryResponse           string
+		expectedRequestError       string
 		expectedErrors             []string
 	}{
 		"client error": {
-			simulateClientError: true,
-			expectedErrors:      []string{"Error sending request:"},
+			simulateClientError:  true,
+			expectedRequestError: "Error sending request:",
 		},
 		"non-200 response": {
 			registryResponseStatusCode: http.StatusInternalServerError,
-			expectedErrors:             []string{fmt.Sprintf("Unexpected status code %d in response", http.StatusInternalServerError)},
+			expectedRequestError:       fmt.Sprintf("Unexpected status code %d in response", http.StatusInternalServerError),
 			registryResponse:           "{}",
 		},
 		"error unmarshaling response body": {
 			registryResponseStatusCode: http.StatusOK,
 			registryResponse:           "foo",
-			expectedErrors:             []string{"Error unmarshaling response:"},
+			expectedRequestError:       "Error unmarshaling response:",
 		},
 		"happy path - no response errors": {
 			registryResponseStatusCode: http.StatusOK,
@@ -887,7 +888,7 @@ func TestDeletingLayerPruneFunc(t *testing.T) {
 		},
 		"happy path - with response errors": {
 			registryResponseStatusCode: http.StatusOK,
-			registryResponse:           `{"result":"failure","errors":["error1","error2","error3"]}`,
+			registryResponse:           `{"result":"failure","errors":{"layer1":["error1","error2","error3"]}}`,
 			expectedErrors:             []string{"error1", "error2", "error3"},
 		},
 	}
@@ -914,8 +915,21 @@ func TestDeletingLayerPruneFunc(t *testing.T) {
 			"layer1": {"aaa/stream1", "bbb/stream2"},
 		}
 
-		errs := pruneFunc(registry, deletions)
+		requestError, layerErrors := pruneFunc(registry, deletions)
+
+		gotError := requestError != nil
+		expectError := len(test.expectedRequestError) != 0
+		if e, a := expectError, gotError; e != a {
+			t.Errorf("%s: requestError: expected %t, got %t: %v", name, e, a, requestError)
+			continue
+		}
+		if gotError {
+			if e, a := test.expectedRequestError, requestError; !strings.HasPrefix(a.Error(), e) {
+				t.Errorf("%s: expected request error %q, got %q", name, e, a)
+			}
+		}
 
+		errs := layerErrors["layer1"]
 		if e, a := len(test.expectedErrors), len(errs); e != a {
 			t.Errorf("%s: expected %d errors (%v), got %d (%v)", name, e, test.expectedErrors, a, errs)
 			continue

diff --git a/pkg/image/prune/summary.go b/pkg/image/prune/summary.go
@@ -16,17 +16,33 @@ type summarizingPruner struct {
 	imageFailures  []string
 	imageErrors    []error
 
-	layerSuccesses []string
-	layerFailures  []string
-	layerErrors    []error
+	/*
+		{
+			registry1: {
+				layer1: {
+					requestError: nil,
+					layerErrors: [err1, err2],
+				},
+				...,
+			},
+			registry2: ...
+		}
+	*/
+	registryResults map[string]registryResult
+}
+
+type registryResult struct {
+	requestError error
+	layerErrors  map[string][]error
 }
 
 var _ ImagePruner = &summarizingPruner{}
 
 func NewSummarizingImagePruner(pruner ImagePruner, out io.Writer) ImagePruner {
 	return &summarizingPruner{
-		delegate: pruner,
-		out:      out,
+		delegate:        pruner,
+		out:             out,
+		registryResults: map[string]registryResult{},
 	}
 }
 
@@ -36,12 +52,30 @@ func (p *summarizingPruner) Run(baseImagePruneFunc ImagePruneFunc, baseLayerPrun
 }
 
 func (p *summarizingPruner) summarize() {
-	fmt.Fprintln(p.out, "IMAGE PRUNING SUMMARY:")
+	fmt.Fprintln(p.out, "\nIMAGE PRUNING SUMMARY:")
 	fmt.Fprintf(p.out, "# Image prune successes: %d\n", len(p.imageSuccesses))
 	fmt.Fprintf(p.out, "# Image prune errors: %d\n", len(p.imageFailures))
-	fmt.Fprintln(p.out, "LAYER PRUNING SUMMARY:")
-	fmt.Fprintf(p.out, "# Layer prune successes: %d\n", len(p.layerSuccesses))
-	fmt.Fprintf(p.out, "# Layer prune errors: %d\n", len(p.layerFailures))
+
+	fmt.Fprintln(p.out, "\nLAYER PRUNING SUMMARY:")
+	for registry, result := range p.registryResults {
+		p.summarizeRegistry(registry, result)
+	}
+}
+
+func (p *summarizingPruner) summarizeRegistry(registry string, result registryResult) {
+	fmt.Fprintf(p.out, "\tRegistry: %s\n", registry)
+	fmt.Fprintf(p.out, "\t\tRequest sent successfully: %t\n", result.requestError == nil)
+	successes, failures := 0, 0
+	for _, errs := range result.layerErrors {
+		switch len(errs) {
+		case 0:
+			successes++
+		default:
+			failures++
+		}
+	}
+	fmt.Fprintf(p.out, "\t\t# Layer prune successes: %d\n", successes)
+	fmt.Fprintf(p.out, "\t\t# Layer prune errors: %d\n", failures)
 }
 
 func (p *summarizingPruner) imagePruneFunc(base ImagePruneFunc) ImagePruneFunc {
@@ -59,8 +93,12 @@ func (p *summarizingPruner) imagePruneFunc(base ImagePruneFunc) ImagePruneFunc {
 }
 
 func (p *summarizingPruner) layerPruneFunc(base LayerPruneFunc) LayerPruneFunc {
-	return func(registryURL string, req dockerregistry.DeleteLayersRequest) []error {
-		errs := base(registryURL, req)
-		return errs
+	return func(registryURL string, req dockerregistry.DeleteLayersRequest) (error, map[string][]error) {
+		requestError, layerErrors := base(registryURL, req)
+		p.registryResults[registryURL] = registryResult{
+			requestError: requestError,
+			layerErrors:  layerErrors,
+		}
+		return requestError, layerErrors
 	}
 }