From 4f188aa191f1d28be19af64317bd8512f5c0cac0 Mon Sep 17 00:00:00 2001 From: umohnani8 Date: Fri, 6 Jul 2018 11:55:02 -0400 Subject: podman rmi should only untag image if parent of another podman rmi was deleting an image even if it was a parent of another image. This fix just untags the image instead. This also fixes podman rmi to remove intermediate images of an image when the image is removed. Signed-off-by: umohnani8 Closes: #1055 Approved by: mheon --- libpod/image/image.go | 101 ++++++++++++++++++++++++++++++++++++++++++++++++-- libpod/runtime_img.go | 36 +++++++++++------- 2 files changed, 120 insertions(+), 17 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index 5dd2c57f3..082c4906f 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -59,6 +59,9 @@ type Runtime struct { SignaturePolicyPath string } +// ErrRepoTagNotFound is the error returned when the image id given doesn't match a rep tag in store +var ErrRepoTagNotFound = errors.New("unable to match user input to any specific repotag") + // NewImageRuntimeFromStore creates an ImageRuntime based on a provided store func NewImageRuntimeFromStore(store storage.Store) *Runtime { return &Runtime{ @@ -333,9 +336,39 @@ func (i *Image) TopLayer() string { // Remove an image; container removal for the image must be done // outside the context of images +// TODO: the force param does nothing as of now. Need to move container +// handling logic here eventually. func (i *Image) Remove(force bool) error { - _, err := i.imageruntime.store.DeleteImage(i.ID(), true) - return err + parent, err := i.GetParent() + if err != nil { + return err + } + if _, err := i.imageruntime.store.DeleteImage(i.ID(), true); err != nil { + return err + } + for parent != nil { + nextParent, err := parent.GetParent() + if err != nil { + return err + } + children, err := parent.GetChildren() + if err != nil { + return err + } + // Do not remove if image is a base image and is not untagged, or if + // the image has more children. + if (nextParent == nil && len(parent.Names()) > 0) || len(children) > 0 { + return nil + } + id := parent.ID() + if _, err := i.imageruntime.store.DeleteImage(id, true); err != nil { + logrus.Debugf("unable to remove intermediate image %q: %v", id, err) + } else { + fmt.Println(id) + } + parent = nextParent + } + return nil } // Decompose an Image @@ -902,7 +935,7 @@ func (i *Image) MatchRepoTag(input string) (string, error) { } } if maxCount == 0 { - return "", errors.Errorf("unable to match user input to any specific repotag") + return "", ErrRepoTagNotFound } if len(results[maxCount]) > 1 { return "", errors.Errorf("user input matched multiple repotags for the image") @@ -916,6 +949,68 @@ func splitString(input string) string { return split[len(split)-1] } +// IsParent goes through the layers in the store and checks if i.TopLayer is +// the parent of any other layer in store. Double check that image with that +// layer exists as well. +func (i *Image) IsParent() (bool, error) { + children, err := i.GetChildren() + if err != nil { + return false, err + } + return len(children) > 0, nil +} + +// GetParent returns the image ID of the parent. Return nil if a parent is not found. +func (i *Image) GetParent() (*Image, error) { + images, err := i.imageruntime.GetImages() + if err != nil { + return nil, err + } + layer, err := i.imageruntime.store.Layer(i.TopLayer()) + if err != nil { + return nil, err + } + for _, img := range images { + if img.TopLayer() == layer.Parent { + return img, nil + } + } + return nil, nil +} + +// GetChildren returns a list of the imageIDs that depend on the image +func (i *Image) GetChildren() ([]string, error) { + var children []string + images, err := i.imageruntime.GetImages() + if err != nil { + return nil, err + } + layers, err := i.imageruntime.store.Layers() + if err != nil { + return nil, err + } + + for _, layer := range layers { + if layer.Parent == i.TopLayer() { + if imageID := getImageOfTopLayer(images, layer.ID); len(imageID) > 0 { + children = append(children, imageID...) + } + } + } + return children, nil +} + +// getImageOfTopLayer returns the image ID where layer is the top layer of the image +func getImageOfTopLayer(images []*Image, layer string) []string { + var matches []string + for _, img := range images { + if img.TopLayer() == layer { + matches = append(matches, img.ID()) + } + } + return matches +} + // InputIsID returns a bool if the user input for an image // is the image's partial or full id func (i *Image) InputIsID() bool { diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go index 8b0c08cd3..d127d753f 100644 --- a/libpod/runtime_img.go +++ b/libpod/runtime_img.go @@ -75,7 +75,7 @@ type CopyOptions struct { // RemoveImage deletes an image from local storage // Images being used by running containers can only be removed if force=true -func (r *Runtime) RemoveImage(ctx context.Context, image *image.Image, force bool) (string, error) { +func (r *Runtime) RemoveImage(ctx context.Context, img *image.Image, force bool) (string, error) { r.lock.Lock() defer r.lock.Unlock() @@ -90,50 +90,58 @@ func (r *Runtime) RemoveImage(ctx context.Context, image *image.Image, force boo } imageCtrs := []*Container{} for _, ctr := range ctrs { - if ctr.config.RootfsImageID == image.ID() { + if ctr.config.RootfsImageID == img.ID() { imageCtrs = append(imageCtrs, ctr) } } - if len(imageCtrs) > 0 && len(image.Names()) <= 1 { + if len(imageCtrs) > 0 && len(img.Names()) <= 1 { if force { for _, ctr := range imageCtrs { if err := r.removeContainer(ctx, ctr, true); err != nil { - return "", errors.Wrapf(err, "error removing image %s: container %s using image could not be removed", image.ID(), ctr.ID()) + return "", errors.Wrapf(err, "error removing image %s: container %s using image could not be removed", img.ID(), ctr.ID()) } } } else { - return "", fmt.Errorf("could not remove image %s as it is being used by %d containers", image.ID(), len(imageCtrs)) + return "", fmt.Errorf("could not remove image %s as it is being used by %d containers", img.ID(), len(imageCtrs)) } } - if len(image.Names()) > 1 && !image.InputIsID() { + hasChildren, err := img.IsParent() + if err != nil { + return "", err + } + + if (len(img.Names()) > 1 && !img.InputIsID()) || hasChildren { // If the image has multiple reponames, we do not technically delete // the image. we figure out which repotag the user is trying to refer // to and untag it. - repoName, err := image.MatchRepoTag(image.InputName) + repoName, err := img.MatchRepoTag(img.InputName) + if hasChildren && err == image.ErrRepoTagNotFound { + return "", errors.Errorf("unable to delete %q (cannot be forced) - image has dependent child images", img.ID()) + } if err != nil { return "", err } - if err := image.UntagImage(repoName); err != nil { + if err := img.UntagImage(repoName); err != nil { return "", err } return fmt.Sprintf("Untagged: %s", repoName), nil - } else if len(image.Names()) > 1 && image.InputIsID() && !force { + } else if len(img.Names()) > 1 && img.InputIsID() && !force { // If the user requests to delete an image by ID and the image has multiple // reponames and no force is applied, we error out. - return "", fmt.Errorf("unable to delete %s (must force) - image is referred to in multiple tags", image.ID()) + return "", fmt.Errorf("unable to delete %s (must force) - image is referred to in multiple tags", img.ID()) } - err = image.Remove(force) + err = img.Remove(force) if err != nil && errors.Cause(err) == storage.ErrImageUsedByContainer { - if errStorage := r.rmStorageContainers(force, image); errStorage == nil { + if errStorage := r.rmStorageContainers(force, img); errStorage == nil { // Containers associated with the image should be deleted now, // let's try removing the image again. - err = image.Remove(force) + err = img.Remove(force) } else { err = errStorage } } - return image.ID(), err + return img.ID(), err } // Remove containers that are in storage rather than Podman. -- cgit v1.2.3-54-g00ecf