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 <umohnani@redhat.com>

Closes: #1055
Approved by: mheon
This commit is contained in:
umohnani8
2018-07-06 11:55:02 -04:00
committed by Atomic Bot
parent a1f3d44497
commit 4f188aa191
4 changed files with 225 additions and 36 deletions

View File

@ -7,7 +7,8 @@ import (
"strings"
"time"
"github.com/containers/storage"
"github.com/sirupsen/logrus"
"github.com/docker/go-units"
digest "github.com/opencontainers/go-digest"
"github.com/pkg/errors"
@ -259,12 +260,16 @@ func sortImagesOutput(sortBy string, imagesOutput imagesSorted) imagesSorted {
}
// getImagesTemplateOutput returns the images information to be printed in human readable format
func getImagesTemplateOutput(ctx context.Context, runtime *libpod.Runtime, images []*image.Image, opts imagesOptions, storageLayers []storage.Layer) (imagesOutput imagesSorted) {
func getImagesTemplateOutput(ctx context.Context, runtime *libpod.Runtime, images []*image.Image, opts imagesOptions) (imagesOutput imagesSorted) {
for _, img := range images {
// If all is false and the image doesn't have a name, check to see if the top layer of the image is a parent
// to another image's top layer. If it is, then it is an intermediate image so don't print out if the --all flag
// is not set.
if !opts.all && len(img.Names()) == 0 && !layerIsLeaf(storageLayers, img.TopLayer()) {
isParent, err := img.IsParent()
if err != nil {
logrus.Errorf("error checking if image is a parent %q: %v", img.ID(), err)
}
if !opts.all && len(img.Names()) == 0 && isParent {
continue
}
createdTime := img.Created()
@ -325,17 +330,13 @@ func generateImagesOutput(ctx context.Context, runtime *libpod.Runtime, images [
return nil
}
var out formats.Writer
storageLayers, err := runtime.GetLayers()
if err != nil {
return errors.Wrap(err, "error getting layers from store")
}
switch opts.format {
case formats.JSONString:
imagesOutput := getImagesJSONOutput(ctx, runtime, images)
out = formats.JSONStructArray{Output: imagesToGeneric([]imagesTemplateParams{}, imagesOutput)}
default:
imagesOutput := getImagesTemplateOutput(ctx, runtime, images, opts, storageLayers)
imagesOutput := getImagesTemplateOutput(ctx, runtime, images, opts)
out = formats.StdoutTemplateArray{Output: imagesToGeneric(imagesOutput, []imagesJSONParams{}), Template: opts.outputformat, Fields: imagesOutput[0].HeaderMap()}
}
return formats.Writer(out).Out()
@ -391,14 +392,3 @@ func CreateFilterFuncs(ctx context.Context, r *libpod.Runtime, c *cli.Context, i
}
return filterFuncs, nil
}
// layerIsLeaf goes through the layers in the store and checks if "layer" is
// the parent of any other layer in store.
func layerIsLeaf(storageLayers []storage.Layer, layer string) bool {
for _, storeLayer := range storageLayers {
if storeLayer.Parent == layer {
return false
}
}
return true
}

View File

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

View File

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

View File

@ -103,4 +103,100 @@ var _ = Describe("Podman rmi", func() {
resultForce.WaitWithDefaultTimeout()
Expect(resultForce.ExitCode()).To(Equal(0))
})
It("podman rmi image that is a parent of another image", func() {
session := podmanTest.Podman([]string{"rmi", "-fa"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"run", "--name", "c_test", ALPINE, "true"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"commit", "-q", "c_test", "test"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"rm", "c_test"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"rmi", ALPINE})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"images", "-q"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(len(session.OutputToStringArray())).To(Equal(1))
session = podmanTest.Podman([]string{"images", "-q", "-a"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(len(session.OutputToStringArray())).To(Equal(2))
untaggedImg := session.OutputToStringArray()[1]
session = podmanTest.Podman([]string{"rmi", "-f", untaggedImg})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Not(Equal(0)))
})
It("podman rmi with cached images", func() {
session := podmanTest.Podman([]string{"rmi", "-fa"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
dockerfile := `FROM docker.io/library/alpine:latest
RUN mkdir hello
RUN touch test.txt
ENV foo=bar
`
podmanTest.BuildImage(dockerfile, "test", "true")
dockerfile = `FROM docker.io/library/alpine:latest
RUN mkdir hello
RUN touch test.txt
RUN mkdir blah
ENV foo=bar
`
podmanTest.BuildImage(dockerfile, "test2", "true")
session = podmanTest.Podman([]string{"images", "-q", "-a"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
numOfImages := len(session.OutputToStringArray())
session = podmanTest.Podman([]string{"rmi", "test2"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"images", "-q", "-a"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(numOfImages - len(session.OutputToStringArray())).To(Equal(2))
session = podmanTest.Podman([]string{"rmi", "test"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"images", "-q", "-a"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(len(session.OutputToStringArray())).To(Equal(1))
podmanTest.BuildImage(dockerfile, "test3", "true")
session = podmanTest.Podman([]string{"rmi", ALPINE})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"rmi", "test3"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"images", "-q", "-a"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(len(session.OutputToString())).To(Equal(0))
})
})