From 7f97896c59d23d9dda704b19203a9ceb49b57237 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 4 May 2020 14:01:39 +0200 Subject: image removal: refactor part 2 Continue the refactoring of image removal. I didn't manage to break all the following changes into smaller and easier to digest commits due to time constraints: * Return an error slice instead of a single error. Use multierror only in the client/frontend. Reflect that in the types. * Use the batch image removal in the client while preserving the more rest-idiomatic single-image removal endpoint. * Add a new handler for the single-image removal endpoint to make it share the same code as the batch endpoint. * Expose bindings for the single and batch endpoints, so we can properly test them. * Add several convenience functions for error handling to pkg/errorhandling. * Set the correct error type in libpod to set the exit code to 2 when one or more containers are using an image. * Massage the bindings tests a bit and tackle compilation errors. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/libpod/images.go | 51 ++++++++++++++++++++++++++++--- pkg/api/handlers/types.go | 2 +- pkg/api/server/register_images.go | 64 +++++++++++++++++++-------------------- 3 files changed, 80 insertions(+), 37 deletions(-) (limited to 'pkg/api') diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index f7be5ce9a..93b4564a1 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -23,6 +23,7 @@ import ( "github.com/containers/libpod/pkg/api/handlers/utils" "github.com/containers/libpod/pkg/domain/entities" "github.com/containers/libpod/pkg/domain/infra/abi" + "github.com/containers/libpod/pkg/errorhandling" "github.com/containers/libpod/pkg/util" utils2 "github.com/containers/libpod/utils" "github.com/gorilla/schema" @@ -700,8 +701,8 @@ func SearchImages(w http.ResponseWriter, r *http.Request) { utils.WriteResponse(w, http.StatusOK, reports) } -// ImagesRemove is the endpoint for image removal. -func ImagesRemove(w http.ResponseWriter, r *http.Request) { +// ImagesBatchRemove is the endpoint for batch image removal. +func ImagesBatchRemove(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) query := struct { @@ -722,7 +723,49 @@ func ImagesRemove(w http.ResponseWriter, r *http.Request) { opts := entities.ImageRemoveOptions{All: query.All, Force: query.Force} imageEngine := abi.ImageEngine{Libpod: runtime} - rmReport, rmError := imageEngine.Remove(r.Context(), query.Images, opts) - report := handlers.LibpodImagesRemoveReport{ImageRemoveReport: *rmReport, Error: rmError.Error()} + rmReport, rmErrors := imageEngine.Remove(r.Context(), query.Images, opts) + + strErrs := errorhandling.ErrorsToStrings(rmErrors) + report := handlers.LibpodImagesRemoveReport{ImageRemoveReport: *rmReport, Errors: strErrs} utils.WriteResponse(w, http.StatusOK, report) } + +// ImagesRemove is the endpoint for removing one image. +func ImagesRemove(w http.ResponseWriter, r *http.Request) { + runtime := r.Context().Value("runtime").(*libpod.Runtime) + decoder := r.Context().Value("decoder").(*schema.Decoder) + query := struct { + Force bool `schema:"force"` + }{ + Force: false, + } + + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) + return + } + + opts := entities.ImageRemoveOptions{Force: query.Force} + imageEngine := abi.ImageEngine{Libpod: runtime} + rmReport, rmErrors := imageEngine.Remove(r.Context(), []string{utils.GetName(r)}, opts) + + // In contrast to batch-removal, where we're only setting the exit + // code, we need to have another closer look at the errors here and set + // the appropriate http status code. + + switch rmReport.ExitCode { + case 0: + report := handlers.LibpodImagesRemoveReport{ImageRemoveReport: *rmReport, Errors: []string{}} + utils.WriteResponse(w, http.StatusOK, report) + case 1: + // 404 - no such image + utils.Error(w, "error removing image", http.StatusNotFound, errorhandling.JoinErrors(rmErrors)) + case 2: + // 409 - conflict error (in use by containers) + utils.Error(w, "error removing image", http.StatusConflict, errorhandling.JoinErrors(rmErrors)) + default: + // 500 - internal error + utils.Error(w, "failed to remove image", http.StatusInternalServerError, errorhandling.JoinErrors(rmErrors)) + } +} diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index 58a12ea6a..a7abf59c0 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -41,7 +41,7 @@ type LibpodImagesPullReport struct { type LibpodImagesRemoveReport struct { entities.ImageRemoveReport // Image removal requires is to return data and an error. - Error string + Errors []string } type ContainersPruneReport struct { diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index f59dca6f5..0e8d68b7e 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -822,7 +822,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // 500: // $ref: '#/responses/InternalError' r.Handle(VersionedPath("/libpod/images/import"), s.APIHandler(libpod.ImagesImport)).Methods(http.MethodPost) - // swagger:operation GET /libpod/images/remove libpod libpodImagesRemove + // swagger:operation DELETE /libpod/images/remove libpod libpodImagesRemove // --- // tags: // - images @@ -853,7 +853,37 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // $ref: "#/responses/BadParamError" // 500: // $ref: '#/responses/InternalError' - r.Handle(VersionedPath("/libpod/images/remove"), s.APIHandler(libpod.ImagesRemove)).Methods(http.MethodGet) + r.Handle(VersionedPath("/libpod/images/remove"), s.APIHandler(libpod.ImagesBatchRemove)).Methods(http.MethodDelete) + // swagger:operation DELETE /libpod/images/{name:.*}/remove libpod libpodRemoveImage + // --- + // tags: + // - images + // summary: Remove an image from the local storage. + // description: Remove an image from the local storage. + // parameters: + // - in: path + // name: name:.* + // type: string + // required: true + // description: name or ID of image to remove + // - in: query + // name: force + // type: boolean + // description: remove the image even if used by containers or has other tags + // produces: + // - application/json + // responses: + // 200: + // $ref: "#/responses/DocsImageDeleteResponse" + // 400: + // $ref: "#/responses/BadParamError" + // 404: + // $ref: '#/responses/NoSuchImage' + // 409: + // $ref: '#/responses/ConflictError' + // 500: + // $ref: '#/responses/InternalError' + r.Handle(VersionedPath("/libpod/images/{name:.*}/remove"), s.APIHandler(libpod.ImagesRemove)).Methods(http.MethodDelete) // swagger:operation POST /libpod/images/pull libpod libpodImagesPull // --- // tags: @@ -952,36 +982,6 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // 500: // $ref: '#/responses/InternalError' r.Handle(VersionedPath("/libpod/images/search"), s.APIHandler(libpod.SearchImages)).Methods(http.MethodGet) - // swagger:operation DELETE /libpod/images/{name:.*} libpod libpodRemoveImage - // --- - // tags: - // - images - // summary: Remove Image - // description: Delete an image from local store - // parameters: - // - in: path - // name: name:.* - // type: string - // required: true - // description: name or ID of image to delete - // - in: query - // name: force - // type: boolean - // description: remove the image even if used by containers or has other tags - // produces: - // - application/json - // responses: - // 200: - // $ref: "#/responses/DocsImageDeleteResponse" - // 400: - // $ref: "#/responses/BadParamError" - // 404: - // $ref: '#/responses/NoSuchImage' - // 409: - // $ref: '#/responses/ConflictError' - // 500: - // $ref: '#/responses/InternalError' - r.Handle(VersionedPath("/libpod/images/{name:.*}"), s.APIHandler(compat.RemoveImage)).Methods(http.MethodDelete) // swagger:operation GET /libpod/images/{name:.*}/get libpod libpodExportImage // --- // tags: -- cgit v1.2.3-54-g00ecf