diff --git a/libpod/reset.go b/libpod/reset.go index c223aeb5df..00ae3bd42c 100644 --- a/libpod/reset.go +++ b/libpod/reset.go @@ -178,7 +178,7 @@ func (r *Runtime) Reset(ctx context.Context) error { rmiOptions := &libimage.RemoveImagesOptions{ Force: true, Ignore: true, - RemoveContainerFunc: r.RemoveContainersForImageCallback(ctx), + RemoveContainerFunc: r.RemoveContainersForImageCallback(ctx, true), Filters: []string{"readonly=false"}, } if _, rmiErrors := r.LibimageRuntime().RemoveImages(ctx, nil, rmiOptions); rmiErrors != nil { diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go index bac8ec31f2..cf32d953e8 100644 --- a/libpod/runtime_img.go +++ b/libpod/runtime_img.go @@ -23,9 +23,10 @@ import ( // RemoveContainersForImageCallback returns a callback that can be used in // `libimage`. When forcefully removing images, containers using the image -// should be removed as well. The callback allows for more graceful removal as -// we can use the libpod-internal removal logic. -func (r *Runtime) RemoveContainersForImageCallback(ctx context.Context) libimage.RemoveContainerFunc { +// should be removed as well unless the request comes from the Docker compat API. +// The callback allows for more graceful removal as we can use the libpod-internal +// removal logic. +func (r *Runtime) RemoveContainersForImageCallback(ctx context.Context, force bool) libimage.RemoveContainerFunc { return func(imageID string) error { if !r.valid { return define.ErrRuntimeStopped @@ -44,12 +45,12 @@ func (r *Runtime) RemoveContainersForImageCallback(ctx context.Context) libimage if err != nil { return fmt.Errorf("container %s is in pod %s, but pod cannot be retrieved: %w", ctr.ID(), ctr.config.Pod, err) } - if _, err := r.removePod(ctx, pod, true, true, timeout); err != nil { + if _, err := r.removePod(ctx, pod, true, force, timeout); err != nil { return fmt.Errorf("removing image %s: container %s using image could not be removed: %w", imageID, ctr.ID(), err) } } else { opts := ctrRmOpts{ - Force: true, + Force: force, Timeout: timeout, } if _, _, err := r.removeContainer(ctx, ctr, opts); err != nil { diff --git a/pkg/api/handlers/compat/images_remove.go b/pkg/api/handlers/compat/images_remove.go index 8379da99b1..9382ebb86e 100644 --- a/pkg/api/handlers/compat/images_remove.go +++ b/pkg/api/handlers/compat/images_remove.go @@ -8,6 +8,7 @@ import ( "net/http" "github.com/containers/podman/v5/libpod" + "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/pkg/api/handlers/utils" api "github.com/containers/podman/v5/pkg/api/types" "github.com/containers/podman/v5/pkg/domain/entities" @@ -41,9 +42,10 @@ func RemoveImage(w http.ResponseWriter, r *http.Request) { imageEngine := abi.ImageEngine{Libpod: runtime} options := entities.ImageRemoveOptions{ - Force: query.Force, - NoPrune: query.NoPrune, - Ignore: query.Ignore, + Force: query.Force, + NoPrune: query.NoPrune, + Ignore: query.Ignore, + DisableForceRemoveContainers: true, } report, rmerrors := imageEngine.Remove(r.Context(), []string{possiblyNormalizedName}, options) if len(rmerrors) > 0 && rmerrors[0] != nil { @@ -56,6 +58,10 @@ func RemoveImage(w http.ResponseWriter, r *http.Request) { utils.Error(w, http.StatusConflict, fmt.Errorf("image %s is in use: %w", name, err)) return } + if errors.Is(err, define.ErrCtrStateInvalid) { + utils.Error(w, http.StatusConflict, fmt.Errorf("image %s is in an invalid state: %w", name, err)) + return + } utils.Error(w, http.StatusInternalServerError, err) return } diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index 9350abbe35..ca565ffda1 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -227,7 +227,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // - in: query // name: force // type: boolean - // description: remove the image even if used by containers or has other tags + // description: Remove the image even if it is being used by stopped containers or has other tags // - in: query // name: noprune // type: boolean diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index b9100af8ff..395335d9a3 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -64,7 +64,8 @@ type ImageRemoveOptions struct { // Confirms if given name is a manifest list and removes it, otherwise returns error. LookupManifest bool // NoPrune will not remove dangling images - NoPrune bool + NoPrune bool + DisableForceRemoveContainers bool } // ImageRemoveReport is the response for removing one or more image(s) from storage @@ -73,8 +74,10 @@ type ImageRemoveReport = entitiesTypes.ImageRemoveReport type ImageHistoryOptions struct{} -type ImageHistoryLayer = entitiesTypes.ImageHistoryLayer -type ImageHistoryReport = entitiesTypes.ImageHistoryReport +type ( + ImageHistoryLayer = entitiesTypes.ImageHistoryLayer + ImageHistoryReport = entitiesTypes.ImageHistoryReport +) // ImagePullOptions are the arguments for pulling images. type ImagePullOptions struct { @@ -255,8 +258,10 @@ type ImagePruneOptions struct { Filter []string `json:"filter" schema:"filter"` } -type ImageTagOptions struct{} -type ImageUntagOptions struct{} +type ( + ImageTagOptions struct{} + ImageUntagOptions struct{} +) // ImageInspectReport is the data when inspecting an image. type ImageInspectReport = entitiesTypes.ImageInspectReport diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 53b0cee197..0f38f63c6d 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -58,7 +58,7 @@ func (ir *ImageEngine) Exists(_ context.Context, nameOrID string) (*entities.Boo func (ir *ImageEngine) Prune(ctx context.Context, opts entities.ImagePruneOptions) ([]*reports.PruneReport, error) { pruneOptions := &libimage.RemoveImagesOptions{ - RemoveContainerFunc: ir.Libpod.RemoveContainersForImageCallback(ctx), + RemoveContainerFunc: ir.Libpod.RemoveContainersForImageCallback(ctx, true), IsExternalContainerFunc: ir.Libpod.IsExternalContainerCallback(ctx), ExternalContainers: opts.External, Filters: append(opts.Filter, "readonly=false"), @@ -666,7 +666,7 @@ func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entitie if !opts.All { libimageOptions.Filters = append(libimageOptions.Filters, "intermediate=false") } - libimageOptions.RemoveContainerFunc = ir.Libpod.RemoveContainersForImageCallback(ctx) + libimageOptions.RemoveContainerFunc = ir.Libpod.RemoveContainersForImageCallback(ctx, !opts.DisableForceRemoveContainers) libimageReport, libimageErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, images, libimageOptions) diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at index eb434ba4fc..93492d1f14 100644 --- a/test/apiv2/10-images.at +++ b/test/apiv2/10-images.at @@ -231,6 +231,47 @@ t DELETE images/test1:latest 200 t GET "images/get?names=alpine" 200 '[POSIX tar archive]' +# START: Testing variance between Docker API and Podman API +# regarding force deleting images. +# Podman: Force deleting an image will force remove any +# container using the image. +# Docker: Force deleting an image will only remove non +# running containers using the image. + +# Create new image +podman image build -t docker.io/library/test1:latest - <file4 +EOF + +# Create running container +podman run --rm -d --name test_container docker.io/library/test1:latest top + +# When using the Docker Compat API, force deleting an image +# shouldn't force delete any container using the image, only +# containers in a non running state should be removed. +# https://github.com/containers/podman/issues/25871 +t DELETE images/test1:latest?force=true 409 + +# When using the Podman Libpod API, deleting an image +# with a running container will fail. +t DELETE libpod/images/test1:latest 409 + +# When using the Podman Libpod API, force deleting an +# image will also force delete all containers using the image. + +# Verify container exists. +t GET libpod/containers/test_container/exists 204 + +# Delete image with force. +t DELETE libpod/images/test1:latest?force=true 200 + +# Verify container also removed. +t GET libpod/containers/test_container/exists 404 + +# END: Testing variance between Docker API and Podman API +# regarding force deleting images. + podman pull busybox t GET "images/get?names=alpine&names=busybox" 200 '[POSIX tar archive]' img_cnt=$(tar xf "$WORKDIR/curl.result.out" manifest.json -O | jq "length") diff --git a/test/apiv2/python/rest_api/test_v2_0_0_image.py b/test/apiv2/python/rest_api/test_v2_0_0_image.py index 58d03b1493..7882be2059 100644 --- a/test/apiv2/python/rest_api/test_v2_0_0_image.py +++ b/test/apiv2/python/rest_api/test_v2_0_0_image.py @@ -66,9 +66,8 @@ class ImageTestCase(APITestCase): self.assertIn("sha256:",image['Id']) def test_delete(self): - r = requests.delete(self.podman_url + "/v1.40/images/alpine?force=true") - self.assertEqual(r.status_code, 200, r.text) - self.assertIsInstance(r.json(), list) + r = requests.delete(self.compat_uri("images/alpine?force=true")) + self.assertEqual(r.status_code, 409, r.text) def test_pull(self): r = requests.post(self.uri("/images/pull?reference=alpine"), timeout=15)