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 <rothberg@redhat.com>

3 files changed, 80 insertions, 37 deletions

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: