mirror of https://github.com/containers/podman.git
bug: Correct Docker compat REST API image delete endpoint
The Docker `-XDELETE image/$name?force=true` endpoint only removes containers using an image if they are in a non running state. In Podman, when forcefully removing images we also forcefully delete containers using the image including running containers. This patch changes the Docker image force delete compat API to act like the Docker API while maintaining commands like `podman rmi -f $imagename` It also corrects the API return code returned when an image is requested to be deleted with running containers using it. Fixes: https://github.com/containers/podman/issues/25871 Signed-off-by: Lewis Roy <lewis@redhat.com>
This commit is contained in:
parent
5c5ecdea88
commit
6e7de438cc
|
@ -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 {
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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"
|
||||
|
@ -44,6 +45,7 @@ func RemoveImage(w http.ResponseWriter, r *http.Request) {
|
|||
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
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -65,6 +65,7 @@ type ImageRemoveOptions struct {
|
|||
LookupManifest bool
|
||||
// NoPrune will not remove dangling images
|
||||
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
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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 - <<EOF
|
||||
from alpine
|
||||
RUN >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")
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue