summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>2020-05-05 17:25:04 +0200
committerGitHub <noreply@github.com>2020-05-05 17:25:04 +0200
commite6235ef8f1a3111f8f1afbb1bf64f0e6da704a5b (patch)
tree0e9adb10b78b589120b81b49c46b41cc75da7966
parent4a1331d0afd9a21ff9465916d4006ff7297ae07c (diff)
parent7f97896c59d23d9dda704b19203a9ceb49b57237 (diff)
downloadpodman-e6235ef8f1a3111f8f1afbb1bf64f0e6da704a5b.tar.gz
podman-e6235ef8f1a3111f8f1afbb1bf64f0e6da704a5b.tar.bz2
podman-e6235ef8f1a3111f8f1afbb1bf64f0e6da704a5b.zip
Merge pull request #6076 from vrothberg/rmi-v2.2
image removal: refactor part 2
-rw-r--r--cmd/podman/containers/run.go6
-rw-r--r--cmd/podman/images/rm.go7
-rw-r--r--libpod/runtime_img.go2
-rw-r--r--pkg/api/handlers/libpod/images.go51
-rw-r--r--pkg/api/handlers/types.go2
-rw-r--r--pkg/api/server/register_images.go64
-rw-r--r--pkg/bindings/images/images.go30
-rw-r--r--pkg/bindings/images/rm.go65
-rw-r--r--pkg/bindings/test/images_test.go40
-rw-r--r--pkg/bindings/test/manifests_test.go2
-rw-r--r--pkg/domain/entities/engine_image.go2
-rw-r--r--pkg/domain/infra/abi/images.go109
-rw-r--r--pkg/domain/infra/tunnel/images.go4
-rw-r--r--pkg/errorhandling/errorhandling.go36
14 files changed, 262 insertions, 158 deletions
diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go
index b13983e37..f72446cb6 100644
--- a/cmd/podman/containers/run.go
+++ b/cmd/podman/containers/run.go
@@ -170,9 +170,9 @@ func run(cmd *cobra.Command, args []string) error {
return nil
}
if runRmi {
- _, err := registry.ImageEngine().Remove(registry.GetContext(), []string{args[0]}, entities.ImageRemoveOptions{})
- if err != nil {
- logrus.Errorf("%s", errors.Wrapf(err, "failed removing image"))
+ _, rmErrors := registry.ImageEngine().Remove(registry.GetContext(), []string{args[0]}, entities.ImageRemoveOptions{})
+ if len(rmErrors) > 0 {
+ logrus.Errorf("%s", errors.Wrapf(errorhandling.JoinErrors(rmErrors), "failed removing image"))
}
}
return nil
diff --git a/cmd/podman/images/rm.go b/cmd/podman/images/rm.go
index 1cf5fa365..4b9920532 100644
--- a/cmd/podman/images/rm.go
+++ b/cmd/podman/images/rm.go
@@ -5,6 +5,7 @@ import (
"github.com/containers/libpod/cmd/podman/registry"
"github.com/containers/libpod/pkg/domain/entities"
+ "github.com/containers/libpod/pkg/errorhandling"
"github.com/pkg/errors"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
@@ -48,7 +49,9 @@ func rm(cmd *cobra.Command, args []string) error {
return errors.Errorf("when using the --all switch, you may not pass any images names or IDs")
}
- report, err := registry.ImageEngine().Remove(registry.GetContext(), args, imageOpts)
+ // Note: certain image-removal errors are non-fatal. Hence, the report
+ // might be set even if err != nil.
+ report, rmErrors := registry.ImageEngine().Remove(registry.GetContext(), args, imageOpts)
if report != nil {
for _, u := range report.Untagged {
fmt.Println("Untagged: " + u)
@@ -62,5 +65,5 @@ func rm(cmd *cobra.Command, args []string) error {
registry.SetExitCode(report.ExitCode)
}
- return err
+ return errorhandling.JoinErrors(rmErrors)
}
diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go
index 919080c42..cd7f54799 100644
--- a/libpod/runtime_img.go
+++ b/libpod/runtime_img.go
@@ -56,7 +56,7 @@ func (r *Runtime) RemoveImage(ctx context.Context, img *image.Image, force bool)
}
}
} else {
- return nil, fmt.Errorf("could not remove image %s as it is being used by %d containers", img.ID(), len(imageCtrs))
+ return nil, errors.Wrapf(define.ErrImageInUse, "could not remove image %s as it is being used by %d containers", img.ID(), len(imageCtrs))
}
}
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:
diff --git a/pkg/bindings/images/images.go b/pkg/bindings/images/images.go
index 4d8ae6a6e..034ade618 100644
--- a/pkg/bindings/images/images.go
+++ b/pkg/bindings/images/images.go
@@ -109,36 +109,6 @@ func Load(ctx context.Context, r io.Reader, name *string) (*entities.ImageLoadRe
return &report, response.Process(&report)
}
-// Remove deletes an image from local storage. The optional force parameter
-// will forcibly remove the image by removing all all containers, including
-// those that are Running, first.
-func Remove(ctx context.Context, images []string, opts entities.ImageRemoveOptions) (*entities.ImageRemoveReport, error) {
- var report handlers.LibpodImagesRemoveReport
- conn, err := bindings.GetClient(ctx)
- if err != nil {
- return nil, err
- }
- params := url.Values{}
- params.Set("all", strconv.FormatBool(opts.All))
- params.Set("force", strconv.FormatBool(opts.Force))
- for _, i := range images {
- params.Add("images", i)
- }
-
- response, err := conn.DoRequest(nil, http.MethodGet, "/images/remove", params)
- if err != nil {
- return nil, err
- }
- if err := response.Process(&report); err != nil {
- return nil, err
- }
- var rmError error
- if report.Error != "" {
- rmError = errors.New(report.Error)
- }
- return &report.ImageRemoveReport, rmError
-}
-
// Export saves an image from local storage as a tarball or image archive. The optional format
// parameter is used to change the format of the output.
func Export(ctx context.Context, nameOrID string, w io.Writer, format *string, compress *bool) error {
diff --git a/pkg/bindings/images/rm.go b/pkg/bindings/images/rm.go
new file mode 100644
index 000000000..e3b5590df
--- /dev/null
+++ b/pkg/bindings/images/rm.go
@@ -0,0 +1,65 @@
+package images
+
+import (
+ "context"
+ "net/http"
+ "net/url"
+ "strconv"
+
+ "github.com/containers/libpod/pkg/api/handlers"
+ "github.com/containers/libpod/pkg/bindings"
+ "github.com/containers/libpod/pkg/domain/entities"
+ "github.com/containers/libpod/pkg/errorhandling"
+)
+
+// BachtRemove removes a batch of images from the local storage.
+func BatchRemove(ctx context.Context, images []string, opts entities.ImageRemoveOptions) (*entities.ImageRemoveReport, []error) {
+ // FIXME - bindings tests are missing for this endpoint. Once the CI is
+ // re-enabled for bindings, we need to add them. At the time of writing,
+ // the tests don't compile.
+ var report handlers.LibpodImagesRemoveReport
+ conn, err := bindings.GetClient(ctx)
+ if err != nil {
+ return nil, []error{err}
+ }
+
+ params := url.Values{}
+ params.Set("all", strconv.FormatBool(opts.All))
+ params.Set("force", strconv.FormatBool(opts.Force))
+ for _, i := range images {
+ params.Add("images", i)
+ }
+
+ response, err := conn.DoRequest(nil, http.MethodDelete, "/images/remove", params)
+ if err != nil {
+ return nil, []error{err}
+ }
+ if err := response.Process(&report); err != nil {
+ return nil, []error{err}
+ }
+
+ return &report.ImageRemoveReport, errorhandling.StringsToErrors(report.Errors)
+}
+
+// Remove removes an image from the local storage. Use force to remove an
+// image, even if it's used by containers.
+func Remove(ctx context.Context, nameOrID string, force bool) (*entities.ImageRemoveReport, error) {
+ var report handlers.LibpodImagesRemoveReport
+ conn, err := bindings.GetClient(ctx)
+ if err != nil {
+ return nil, err
+ }
+
+ params := url.Values{}
+ params.Set("force", strconv.FormatBool(force))
+ response, err := conn.DoRequest(nil, http.MethodDelete, "/images/%s/remove", params, nameOrID)
+ if err != nil {
+ return nil, err
+ }
+ if err := response.Process(&report); err != nil {
+ return nil, err
+ }
+
+ errs := errorhandling.StringsToErrors(report.Errors)
+ return &report.ImageRemoveReport, errorhandling.JoinErrors(errs)
+}
diff --git a/pkg/bindings/test/images_test.go b/pkg/bindings/test/images_test.go
index 58210efd0..9c8e82149 100644
--- a/pkg/bindings/test/images_test.go
+++ b/pkg/bindings/test/images_test.go
@@ -84,17 +84,20 @@ var _ = Describe("Podman images", func() {
// Test to validate the remove image api
It("remove image", func() {
// Remove invalid image should be a 404
- _, err = images.Remove(bt.conn, "foobar5000", &bindings.PFalse)
+ response, err := images.Remove(bt.conn, "foobar5000", false)
Expect(err).ToNot(BeNil())
+ Expect(response).To(BeNil())
code, _ := bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusNotFound))
// Remove an image by name, validate image is removed and error is nil
inspectData, err := images.GetImage(bt.conn, busybox.shortName, nil)
Expect(err).To(BeNil())
- response, err := images.Remove(bt.conn, busybox.shortName, nil)
+ response, err = images.Remove(bt.conn, busybox.shortName, false)
Expect(err).To(BeNil())
- Expect(inspectData.ID).To(Equal(response[0]["Deleted"]))
+ code, _ = bindings.CheckResponseCode(err)
+
+ Expect(inspectData.ID).To(Equal(response.Deleted[0]))
inspectData, err = images.GetImage(bt.conn, busybox.shortName, nil)
code, _ = bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusNotFound))
@@ -104,30 +107,31 @@ var _ = Describe("Podman images", func() {
_, err = bt.RunTopContainer(&top, &bindings.PFalse, nil)
Expect(err).To(BeNil())
// we should now have a container called "top" running
- containerResponse, err := containers.Inspect(bt.conn, "top", &bindings.PFalse)
+ containerResponse, err := containers.Inspect(bt.conn, "top", nil)
Expect(err).To(BeNil())
Expect(containerResponse.Name).To(Equal("top"))
// try to remove the image "alpine". This should fail since we are not force
// deleting hence image cannot be deleted until the container is deleted.
- response, err = images.Remove(bt.conn, alpine.shortName, &bindings.PFalse)
+ response, err = images.Remove(bt.conn, alpine.shortName, false)
code, _ = bindings.CheckResponseCode(err)
- Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
+ Expect(code).To(BeNumerically("==", http.StatusConflict))
// Removing the image "alpine" where force = true
- response, err = images.Remove(bt.conn, alpine.shortName, &bindings.PTrue)
+ response, err = images.Remove(bt.conn, alpine.shortName, true)
Expect(err).To(BeNil())
-
- // Checking if both the images are gone as well as the container is deleted
- inspectData, err = images.GetImage(bt.conn, busybox.shortName, nil)
+ // To be extra sure, check if the previously created container
+ // is gone as well.
+ _, err = containers.Inspect(bt.conn, "top", &bindings.PFalse)
code, _ = bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusNotFound))
- inspectData, err = images.GetImage(bt.conn, alpine.shortName, nil)
+ // Now make sure both images are gone.
+ inspectData, err = images.GetImage(bt.conn, busybox.shortName, nil)
code, _ = bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusNotFound))
- _, err = containers.Inspect(bt.conn, "top", &bindings.PFalse)
+ inspectData, err = images.GetImage(bt.conn, alpine.shortName, nil)
code, _ = bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusNotFound))
})
@@ -209,7 +213,7 @@ var _ = Describe("Podman images", func() {
It("Load|Import Image", func() {
// load an image
- _, err := images.Remove(bt.conn, alpine.name, nil)
+ _, err := images.Remove(bt.conn, alpine.name, false)
Expect(err).To(BeNil())
exists, err := images.Exists(bt.conn, alpine.name)
Expect(err).To(BeNil())
@@ -219,7 +223,7 @@ var _ = Describe("Podman images", func() {
Expect(err).To(BeNil())
names, err := images.Load(bt.conn, f, nil)
Expect(err).To(BeNil())
- Expect(names.Name).To(Equal(alpine.name))
+ Expect(names.Names[0]).To(Equal(alpine.name))
exists, err = images.Exists(bt.conn, alpine.name)
Expect(err).To(BeNil())
Expect(exists).To(BeTrue())
@@ -227,7 +231,7 @@ var _ = Describe("Podman images", func() {
// load with a repo name
f, err = os.Open(filepath.Join(ImageCacheDir, alpine.tarballName))
Expect(err).To(BeNil())
- _, err = images.Remove(bt.conn, alpine.name, nil)
+ _, err = images.Remove(bt.conn, alpine.name, false)
Expect(err).To(BeNil())
exists, err = images.Exists(bt.conn, alpine.name)
Expect(err).To(BeNil())
@@ -235,7 +239,7 @@ var _ = Describe("Podman images", func() {
newName := "quay.io/newname:fizzle"
names, err = images.Load(bt.conn, f, &newName)
Expect(err).To(BeNil())
- Expect(names.Name).To(Equal(alpine.name))
+ Expect(names.Names[0]).To(Equal(alpine.name))
exists, err = images.Exists(bt.conn, newName)
Expect(err).To(BeNil())
Expect(exists).To(BeTrue())
@@ -243,7 +247,7 @@ var _ = Describe("Podman images", func() {
// load with a bad repo name should trigger a 500
f, err = os.Open(filepath.Join(ImageCacheDir, alpine.tarballName))
Expect(err).To(BeNil())
- _, err = images.Remove(bt.conn, alpine.name, nil)
+ _, err = images.Remove(bt.conn, alpine.name, false)
Expect(err).To(BeNil())
exists, err = images.Exists(bt.conn, alpine.name)
Expect(err).To(BeNil())
@@ -271,7 +275,7 @@ var _ = Describe("Podman images", func() {
It("Import Image", func() {
// load an image
- _, err = images.Remove(bt.conn, alpine.name, nil)
+ _, err = images.Remove(bt.conn, alpine.name, false)
Expect(err).To(BeNil())
exists, err := images.Exists(bt.conn, alpine.name)
Expect(err).To(BeNil())
diff --git a/pkg/bindings/test/manifests_test.go b/pkg/bindings/test/manifests_test.go
index 23c3d8194..4987dfe5b 100644
--- a/pkg/bindings/test/manifests_test.go
+++ b/pkg/bindings/test/manifests_test.go
@@ -47,7 +47,7 @@ var _ = Describe("Podman containers ", func() {
code, _ := bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
- _, err = images.Remove(bt.conn, id, nil)
+ _, err = images.Remove(bt.conn, id, false)
Expect(err).To(BeNil())
// create manifest list with images
diff --git a/pkg/domain/entities/engine_image.go b/pkg/domain/entities/engine_image.go
index 46a96ca20..45686ec79 100644
--- a/pkg/domain/entities/engine_image.go
+++ b/pkg/domain/entities/engine_image.go
@@ -19,7 +19,7 @@ type ImageEngine interface {
Prune(ctx context.Context, opts ImagePruneOptions) (*ImagePruneReport, error)
Pull(ctx context.Context, rawImage string, opts ImagePullOptions) (*ImagePullReport, error)
Push(ctx context.Context, source string, destination string, opts ImagePushOptions) error
- Remove(ctx context.Context, images []string, opts ImageRemoveOptions) (*ImageRemoveReport, error)
+ Remove(ctx context.Context, images []string, opts ImageRemoveOptions) (*ImageRemoveReport, []error)
Save(ctx context.Context, nameOrId string, tags []string, options ImageSaveOptions) error
Search(ctx context.Context, term string, opts ImageSearchOptions) ([]ImageSearchReport, error)
Shutdown(ctx context.Context)
diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go
index 0af06fb89..7ab5131f0 100644
--- a/pkg/domain/infra/abi/images.go
+++ b/pkg/domain/infra/abi/images.go
@@ -21,7 +21,6 @@ import (
domainUtils "github.com/containers/libpod/pkg/domain/utils"
"github.com/containers/libpod/pkg/util"
"github.com/containers/storage"
- "github.com/hashicorp/go-multierror"
imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
@@ -423,8 +422,10 @@ func (ir *ImageEngine) Tree(ctx context.Context, nameOrId string, opts entities.
return &entities.ImageTreeReport{Tree: results}, nil
}
-// Remove removes one or more images from local storage.
-func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entities.ImageRemoveOptions) (report *entities.ImageRemoveReport, finalError error) {
+// removeErrorsToExitCode returns an exit code for the specified slice of
+// image-removal errors. The error codes are set according to the documented
+// behaviour in the Podman man pages.
+func removeErrorsToExitCode(rmErrors []error) int {
var (
// noSuchImageErrors indicates that at least one image was not found.
noSuchImageErrors bool
@@ -434,59 +435,53 @@ func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entitie
// otherErrors indicates that at least one error other than the two
// above occured.
otherErrors bool
- // deleteError is a multierror to conveniently collect errors during
- // removal. We really want to delete as many images as possible and not
- // error out immediately.
- deleteError *multierror.Error
)
- report = &entities.ImageRemoveReport{}
+ if len(rmErrors) == 0 {
+ return 0
+ }
- // Set the removalCode and the error after all work is done.
- defer func() {
- switch {
- // 2
- case inUseErrors:
- // One of the specified images has child images or is
- // being used by a container.
- report.ExitCode = 2
- // 1
- case noSuchImageErrors && !(otherErrors || inUseErrors):
- // One of the specified images did not exist, and no other
- // failures.
- report.ExitCode = 1
- // 0
+ for _, e := range rmErrors {
+ switch errors.Cause(e) {
+ case define.ErrNoSuchImage:
+ noSuchImageErrors = true
+ case define.ErrImageInUse, storage.ErrImageUsedByContainer:
+ inUseErrors = true
default:
- // Nothing to do.
- }
- if deleteError != nil {
- // go-multierror has a trailing new line which we need to remove to normalize the string.
- finalError = deleteError.ErrorOrNil()
- finalError = errors.New(strings.TrimSpace(finalError.Error()))
+ otherErrors = true
}
+ }
+
+ switch {
+ case inUseErrors:
+ // One of the specified images has child images or is
+ // being used by a container.
+ return 2
+ case noSuchImageErrors && !(otherErrors || inUseErrors):
+ // One of the specified images did not exist, and no other
+ // failures.
+ return 1
+ default:
+ return 125
+ }
+}
+
+// Remove removes one or more images from local storage.
+func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entities.ImageRemoveOptions) (report *entities.ImageRemoveReport, rmErrors []error) {
+ report = &entities.ImageRemoveReport{}
+
+ // Set the exit code at very end.
+ defer func() {
+ report.ExitCode = removeErrorsToExitCode(rmErrors)
}()
// deleteImage is an anonymous function to conveniently delete an image
// without having to pass all local data around.
deleteImage := func(img *image.Image) error {
results, err := ir.Libpod.RemoveImage(ctx, img, opts.Force)
- switch errors.Cause(err) {
- case nil:
- break
- case define.ErrNoSuchImage:
- inUseErrors = true // ExitCode is expected
- case storage.ErrImageUsedByContainer:
- inUseErrors = true // Important for exit codes in Podman.
- return errors.New(
- fmt.Sprintf("A container associated with containers/storage, i.e. via Buildah, CRI-O, etc., may be associated with this image: %-12.12s\n", img.ID()))
- case define.ErrImageInUse:
- inUseErrors = true
- return err
- default:
- otherErrors = true // Important for exit codes in Podman.
+ if err != nil {
return err
}
-
report.Deleted = append(report.Deleted, results.Deleted)
report.Untagged = append(report.Untagged, results.Untagged...)
return nil
@@ -499,9 +494,7 @@ func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entitie
for {
storageImages, err := ir.Libpod.ImageRuntime().GetRWImages()
if err != nil {
- deleteError = multierror.Append(deleteError,
- errors.Wrapf(err, "unable to query local images"))
- otherErrors = true // Important for exit codes in Podman.
+ rmErrors = append(rmErrors, err)
return
}
// No images (left) to remove, so we're done.
@@ -510,9 +503,7 @@ func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entitie
}
// Prevent infinity loops by making a delete-progress check.
if previousImages == len(storageImages) {
- otherErrors = true // Important for exit codes in Podman.
- deleteError = multierror.Append(deleteError,
- errors.New("unable to delete all images, check errors and re-run image removal if needed"))
+ rmErrors = append(rmErrors, errors.New("unable to delete all images, check errors and re-run image removal if needed"))
break
}
previousImages = len(storageImages)
@@ -520,15 +511,15 @@ func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entitie
for _, img := range storageImages {
isParent, err := img.IsParent(ctx)
if err != nil {
- otherErrors = true // Important for exit codes in Podman.
- deleteError = multierror.Append(deleteError, err)
+ rmErrors = append(rmErrors, err)
+ continue
}
// Skip parent images.
if isParent {
continue
}
if err := deleteImage(img); err != nil {
- deleteError = multierror.Append(deleteError, err)
+ rmErrors = append(rmErrors, err)
}
}
}
@@ -539,21 +530,13 @@ func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entitie
// Delete only the specified images.
for _, id := range images {
img, err := ir.Libpod.ImageRuntime().NewFromLocal(id)
- switch errors.Cause(err) {
- case nil:
- break
- case image.ErrNoSuchImage:
- noSuchImageErrors = true // Important for exit codes in Podman.
- fallthrough
- default:
- deleteError = multierror.Append(deleteError, errors.Wrapf(err, "failed to remove image '%s'", id))
+ if err != nil {
+ rmErrors = append(rmErrors, err)
continue
}
-
err = deleteImage(img)
if err != nil {
- otherErrors = true // Important for exit codes in Podman.
- deleteError = multierror.Append(deleteError, err)
+ rmErrors = append(rmErrors, err)
}
}
diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go
index dcc5fc3e7..00893194c 100644
--- a/pkg/domain/infra/tunnel/images.go
+++ b/pkg/domain/infra/tunnel/images.go
@@ -20,8 +20,8 @@ func (ir *ImageEngine) Exists(_ context.Context, nameOrId string) (*entities.Boo
return &entities.BoolReport{Value: found}, err
}
-func (ir *ImageEngine) Remove(ctx context.Context, imagesArg []string, opts entities.ImageRemoveOptions) (*entities.ImageRemoveReport, error) {
- return images.Remove(ir.ClientCxt, imagesArg, opts)
+func (ir *ImageEngine) Remove(ctx context.Context, imagesArg []string, opts entities.ImageRemoveOptions) (*entities.ImageRemoveReport, []error) {
+ return images.BatchRemove(ir.ClientCxt, imagesArg, opts)
}
func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions) ([]*entities.ImageSummary, error) {
diff --git a/pkg/errorhandling/errorhandling.go b/pkg/errorhandling/errorhandling.go
index 970d47636..3117b0ca4 100644
--- a/pkg/errorhandling/errorhandling.go
+++ b/pkg/errorhandling/errorhandling.go
@@ -2,10 +2,46 @@ package errorhandling
import (
"os"
+ "strings"
+ "github.com/hashicorp/go-multierror"
+ "github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
+// JoinErrors converts the error slice into a single human-readable error.
+func JoinErrors(errs []error) error {
+ if len(errs) == 0 {
+ return nil
+ }
+
+ // `multierror` appends new lines which we need to remove to prevent
+ // blank lines when printing the error.
+ var multiE *multierror.Error
+ multiE = multierror.Append(multiE, errs...)
+ return errors.New(strings.TrimSpace(multiE.ErrorOrNil().Error()))
+}
+
+// ErrorsToString converts the slice of errors into a slice of corresponding
+// error messages.
+func ErrorsToStrings(errs []error) []string {
+ strErrs := make([]string, len(errs))
+ for i := range errs {
+ strErrs[i] = errs[i].Error()
+ }
+ return strErrs
+}
+
+// StringsToErrors converts a slice of error messages into a slice of
+// corresponding errors.
+func StringsToErrors(strErrs []string) []error {
+ errs := make([]error, len(strErrs))
+ for i := range strErrs {
+ errs[i] = errors.New(strErrs[i])
+ }
+ return errs
+}
+
// SyncQuiet syncs a file and logs any error. Should only be used within
// a defer.
func SyncQuiet(f *os.File) {