diff --git a/pkg/storage/image.go b/pkg/storage/image.go index a5832e0f..ddff82c1 100644 --- a/pkg/storage/image.go +++ b/pkg/storage/image.go @@ -4,7 +4,7 @@ import ( "errors" "fmt" "net" - "path/filepath" + "path" "regexp" "strings" @@ -51,6 +51,9 @@ type ImageServer interface { ImageStatus(systemContext *types.SystemContext, filter string) (*ImageResult, error) // PullImage imports an image from the specified location. PullImage(systemContext *types.SystemContext, imageName string, options *copy.Options) (types.ImageReference, error) + // UntagImage removes a name from the specified image, and if it was + // the only name the image had, removes the image. + UntagImage(systemContext *types.SystemContext, imageName string) error // RemoveImage deletes the specified image. RemoveImage(systemContext *types.SystemContext, imageName string) error // GetStore returns the reference to the storage library Store which @@ -263,6 +266,57 @@ func (svc *imageService) PullImage(systemContext *types.SystemContext, imageName return destRef, nil } +func (svc *imageService) UntagImage(systemContext *types.SystemContext, nameOrID string) error { + ref, err := alltransports.ParseImageName(nameOrID) + if err != nil { + ref2, err2 := istorage.Transport.ParseStoreReference(svc.store, "@"+nameOrID) + if err2 != nil { + ref3, err3 := istorage.Transport.ParseStoreReference(svc.store, nameOrID) + if err3 != nil { + return err + } + ref2 = ref3 + } + ref = ref2 + } + + img, err := istorage.Transport.GetStoreImage(svc.store, ref) + if err != nil { + return err + } + + if nameOrID != img.ID { + namedRef, err := svc.prepareImage(nameOrID, ©.Options{}) + if err != nil { + return err + } + + name := nameOrID + if namedRef.DockerReference() != nil { + name = namedRef.DockerReference().Name() + if tagged, ok := namedRef.DockerReference().(reference.NamedTagged); ok { + name = name + ":" + tagged.Tag() + } + if canonical, ok := namedRef.DockerReference().(reference.Canonical); ok { + name = name + "@" + canonical.Digest().String() + } + } + + prunedNames := make([]string, 0, len(img.Names)) + for _, imgName := range img.Names { + if imgName != name && imgName != nameOrID { + prunedNames = append(prunedNames, imgName) + } + } + + if len(prunedNames) > 0 { + return svc.store.SetNames(img.ID, prunedNames) + } + } + + return ref.DeleteImage(systemContext) +} + func (svc *imageService) RemoveImage(systemContext *types.SystemContext, nameOrID string) error { ref, err := alltransports.ParseImageName(nameOrID) if err != nil { @@ -440,7 +494,7 @@ func (svc *imageService) ResolveNames(imageName string) ([]string, error) { _, rest := splitDomain(r.Name()) images := []string{} for _, r := range svc.registries { - images = append(images, filepath.Join(r, rest)) + images = append(images, path.Join(r, rest)) } return images, nil } diff --git a/server/image_remove.go b/server/image_remove.go index 32ca4066..d15296cc 100644 --- a/server/image_remove.go +++ b/server/image_remove.go @@ -35,7 +35,7 @@ func (s *Server) RemoveImage(ctx context.Context, req *pb.RemoveImageRequest) (* } } for _, img := range images { - err = s.StorageImageServer().RemoveImage(s.ImageContext(), img) + err = s.StorageImageServer().UntagImage(s.ImageContext(), img) if err != nil { logrus.Debugf("error deleting image %s: %v", img, err) continue diff --git a/test/helpers.bash b/test/helpers.bash index a8d61f52..22955d33 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -234,16 +234,16 @@ function start_crio() { if ! [ "$3" = "--no-pause-image" ] ; then "$BIN2IMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --source-binary "$PAUSE_BINARY" fi - "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=redis:alpine --import-from=dir:"$ARTIFACTS_PATH"/redis-image --add-name=docker.io/library/redis:alpine --signature-policy="$INTEGRATION_ROOT"/policy.json + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=docker.io/library/redis:alpine --import-from=dir:"$ARTIFACTS_PATH"/redis-image --signature-policy="$INTEGRATION_ROOT"/policy.json # TODO: remove the code below for copying redis:alpine in using a canonical reference once # https://github.com/kubernetes-incubator/cri-o/issues/531 is complete and we can # copy the image using a tagged reference and then subsequently find the image without # having to explicitly record the canonical reference as one of the image's names - "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b --import-from=dir:"$ARTIFACTS_PATH"/redis-image-digest --signature-policy="$INTEGRATION_ROOT"/policy.json - "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/oom --import-from=dir:"$ARTIFACTS_PATH"/oom-image --add-name=docker.io/library/mrunalp/oom --signature-policy="$INTEGRATION_ROOT"/policy.json - "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/image-volume-test --import-from=dir:"$ARTIFACTS_PATH"/image-volume-test-image --add-name=docker.io/library/mrunalp/image-volume-test --signature-policy="$INTEGRATION_ROOT"/policy.json - "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --add-name=docker.io/library/busybox:latest --signature-policy="$INTEGRATION_ROOT"/policy.json - "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=runcom/stderr-test:latest --import-from=dir:"$ARTIFACTS_PATH"/stderr-test --add-name=docker.io/runcom/stderr-test:latest --signature-policy="$INTEGRATION_ROOT"/policy.json + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=docker.io/library/redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b --import-from=dir:"$ARTIFACTS_PATH"/redis-image-digest --signature-policy="$INTEGRATION_ROOT"/policy.json + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/oom --import-from=dir:"$ARTIFACTS_PATH"/oom-image --signature-policy="$INTEGRATION_ROOT"/policy.json + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=docker.io/library/mrunalp/image-volume-test --import-from=dir:"$ARTIFACTS_PATH"/image-volume-test-image --signature-policy="$INTEGRATION_ROOT"/policy.json + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=docker.io/library/busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --signature-policy="$INTEGRATION_ROOT"/policy.json + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=docker.io/library/runcom/stderr-test:latest --import-from=dir:"$ARTIFACTS_PATH"/stderr-test --signature-policy="$INTEGRATION_ROOT"/policy.json "$CRIO_BINARY" ${DEFAULT_MOUNTS_OPTS} ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTIONS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --cni-plugin-dir "$CRIO_CNI_PLUGIN" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --log-size-max "$LOG_SIZE_MAX_LIMIT" --config /dev/null config >$CRIO_CONFIG # Prepare the CNI configuration files, we're running with non host networking by default diff --git a/test/image_remove.bats b/test/image_remove.bats new file mode 100644 index 00000000..ca2017d0 --- /dev/null +++ b/test/image_remove.bats @@ -0,0 +1,74 @@ +#!/usr/bin/env bats + +load helpers + +IMAGE=docker.io/kubernetes/pause + +function teardown() { + cleanup_test +} + +@test "image remove with multiple names, by name" { + start_crio "" "" --no-pause-image + # Pull the image, giving it one name. + run crioctl image pull "$IMAGE" + echo "$output" + [ "$status" -eq 0 ] + # Add a second name to the image. + run "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name="$IMAGE":latest --add-name="$IMAGE":othertag --signature-policy="$INTEGRATION_ROOT"/policy.json + echo "$output" + [ "$status" -eq 0 ] + # Get the list of image names and IDs. + run crioctl image list + echo "$output" + [ "$status" -eq 0 ] + [ "$output" != "" ] + # Cycle through each name, removing it by name. The image that we assigned a second + # name to should still be around when we get to removing its second name. + grep ^Tag: <<< "$output" | while read -r header tag ; do + run crioctl image remove --id "$tag" + echo "$output" + [ "$status" -eq 0 ] + done + # List all images and their names. There should be none now. + run crioctl image list --quiet + echo "$output" + [ "$status" -eq 0 ] + [ "$output" = "" ] + printf '%s\n' "$output" | while IFS= read -r id; do + echo "$id" + done + # All done. + cleanup_images + stop_crio +} + +@test "image remove with multiple names, by ID" { + start_crio "" "" --no-pause-image + # Pull the image, giving it one name. + run crioctl image pull "$IMAGE" + echo "$output" + [ "$status" -eq 0 ] + # Add a second name to the image. + run "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name="$IMAGE":latest --add-name="$IMAGE":othertag --signature-policy="$INTEGRATION_ROOT"/policy.json + echo "$output" + [ "$status" -eq 0 ] + # Get the image ID of the image we just saved. + run crioctl image status --id="$IMAGE" + echo "$output" + [ "$status" -eq 0 ] + [ "$output" != "" ] + # Try to remove the image using its ID. That should succeed because removing by ID always works. + grep ^ID: <<< "$output" | while read -r header id ; do + run crioctl image remove --id "$id" + echo "$output" + [ "$status" -eq 0 ] + done + # The image should be gone. + run crioctl image status --id="$IMAGE" + echo "$output" + [ "$status" -ne 0 ] + # All done. + cleanup_images + stop_crio +}