From e7b013665523ca7d1505dc813c7b099350192f92 Mon Sep 17 00:00:00 2001 From: Andy Goldstein Date: Wed, 6 May 2015 15:29:51 -0400 Subject: [PATCH] Image Pruning Update layer deletion responses to include per-layer errors. --- pkg/cmd/dockerregistry/dockerregistry.go | 15 +++-- pkg/cmd/experimental/imageprune/imageprune.go | 2 +- pkg/image/prune/imagepruner.go | 41 +++++++----- pkg/image/prune/imagepruner_test.go | 34 +++++++--- pkg/image/prune/summary.go | 62 +++++++++++++++---- 5 files changed, 108 insertions(+), 46 deletions(-) diff --git a/pkg/cmd/dockerregistry/dockerregistry.go b/pkg/cmd/dockerregistry/dockerregistry.go index 80d0027601ec..54ceeda01d3e 100644 --- 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 index 91a8ab750c2d..921a792579a3 100644 --- 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 index f1d9dadff349..f7fecd0fb1bc 100644 --- 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 index 46453a69bd2f..a8d1e910663d 100644 --- 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 index 2902d3cda761..d9db74578c0a 100644 --- 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 } }