Merge pull request #25946 from ninja-quokka/docker_compat_force_image_remove

bug: Correct Docker compat REST API image delete endpoint
This commit is contained in:
openshift-merge-bot[bot]
2025-04-28 11:03:41 +00:00
committed by GitHub
8 changed files with 72 additions and 20 deletions

View File

@ -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 {

View File

@ -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 {

View File

@ -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
}

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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")

View File

@ -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)