Skip to content

Commit

Permalink
Merge pull request #7963 from jfrazelle/7845-remove-image-fail-dont-u…
Browse files Browse the repository at this point in the history
…ntag

Failing to remove an image, will not remove the image name/tag.
  • Loading branch information
crosbymichael committed Sep 12, 2014
2 parents 990da30 + b2efdc5 commit 32b5d14
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 53 deletions.
30 changes: 15 additions & 15 deletions daemon/image_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
var (
repoName, tag string
tags = []string{}
tagDeleted bool
)

// FIXME: please respect DRY and centralize repo+tag parsing in a single central place! -- shykes
Expand All @@ -60,9 +59,11 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
return err
}

repos := daemon.Repositories().ByID()[img.ID]

//If delete by id, see if the id belong only to one repository
if repoName == "" {
for _, repoAndTag := range daemon.Repositories().ByID()[img.ID] {
for _, repoAndTag := range repos {
parsedRepo, parsedTag := parsers.ParseRepositoryTag(repoAndTag)
if repoName == "" || repoName == parsedRepo {
repoName = parsedRepo
Expand All @@ -83,9 +84,15 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
return nil
}

//Untag the current image
if len(repos) <= 1 {
if err := daemon.canDeleteImage(img.ID, force); err != nil {
return err
}
}

// Untag the current image
for _, tag := range tags {
tagDeleted, err = daemon.Repositories().Delete(repoName, tag)
tagDeleted, err := daemon.Repositories().Delete(repoName, tag)
if err != nil {
return err
}
Expand All @@ -99,9 +106,6 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
tags = daemon.Repositories().ByID()[img.ID]
if (len(tags) <= 1 && repoName == "") || len(tags) == 0 {
if len(byParents[img.ID]) == 0 {
if err := daemon.canDeleteImage(img.ID, force, tagDeleted); err != nil {
return err
}
if err := daemon.Repositories().DeleteAll(img.ID); err != nil {
return err
}
Expand All @@ -125,11 +129,7 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
return nil
}

func (daemon *Daemon) canDeleteImage(imgID string, force, untagged bool) error {
var message string
if untagged {
message = " (docker untagged the image)"
}
func (daemon *Daemon) canDeleteImage(imgID string, force bool) error {
for _, container := range daemon.List() {
parent, err := daemon.Repositories().LookupImage(container.Image)
if err != nil {
Expand All @@ -140,11 +140,11 @@ func (daemon *Daemon) canDeleteImage(imgID string, force, untagged bool) error {
if imgID == p.ID {
if container.IsRunning() {
if force {
return fmt.Errorf("Conflict, cannot force delete %s because the running container %s is using it%s, stop it and retry", utils.TruncateID(imgID), utils.TruncateID(container.ID), message)
return fmt.Errorf("Conflict, cannot force delete %s because the running container %s is using it, stop it and retry", utils.TruncateID(imgID), utils.TruncateID(container.ID))
}
return fmt.Errorf("Conflict, cannot delete %s because the running container %s is using it%s, stop it and use -f to force", utils.TruncateID(imgID), utils.TruncateID(container.ID), message)
return fmt.Errorf("Conflict, cannot delete %s because the running container %s is using it, stop it and use -f to force", utils.TruncateID(imgID), utils.TruncateID(container.ID))
} else if !force {
return fmt.Errorf("Conflict, cannot delete %s because the container %s is using it%s, use -f to force", utils.TruncateID(imgID), utils.TruncateID(container.ID), message)
return fmt.Errorf("Conflict, cannot delete %s because the container %s is using it, use -f to force", utils.TruncateID(imgID), utils.TruncateID(container.ID))
}
}
return nil
Expand Down
38 changes: 0 additions & 38 deletions integration-cli/docker_cli_images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,44 +20,6 @@ func TestImagesEnsureImageIsListed(t *testing.T) {
logDone("images - busybox should be listed")
}

func TestCLIImageTagRemove(t *testing.T) {
imagesBefore, _, _ := cmd(t, "images", "-a")
cmd(t, "tag", "busybox", "utest:tag1")
cmd(t, "tag", "busybox", "utest/docker:tag2")
cmd(t, "tag", "busybox", "utest:5000/docker:tag3")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+3 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}
}
cmd(t, "rmi", "utest/docker:tag2")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+2 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}

}
cmd(t, "rmi", "utest:5000/docker:tag3")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+1 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}

}
cmd(t, "rmi", "utest:tag1")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+0 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}

}
logDone("tag,rmi- tagging the same images multiple times then removing tags")
}

func TestImagesOrderedByCreationDate(t *testing.T) {
defer deleteImages("order:test_a")
defer deleteImages("order:test_c")
Expand Down
77 changes: 77 additions & 0 deletions integration-cli/docker_cli_rmi_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package main

import (
"fmt"
"os/exec"
"strings"
"testing"
)

func TestImageRemoveWithContainerFails(t *testing.T) {
errSubstr := "is using it"

// create a container
runCmd := exec.Command(dockerBinary, "run", "-d", "busybox", "true")
out, _, err := runCommandWithOutput(runCmd)
errorOut(err, t, fmt.Sprintf("failed to create a container: %v %v", out, err))

cleanedContainerID := stripTrailingCharacters(out)

// try to delete the image
runCmd = exec.Command(dockerBinary, "rmi", "busybox")
out, _, err = runCommandWithOutput(runCmd)
if err == nil {
t.Fatalf("Container %q is using image, should not be able to rmi: %q", cleanedContainerID, out)
}
if !strings.Contains(out, errSubstr) {
t.Fatalf("Container %q is using image, error message should contain %q: %v", cleanedContainerID, errSubstr, out)
}

// make sure it didn't delete the busybox name
images, _, _ := cmd(t, "images")
if !strings.Contains(images, "busybox") {
t.Fatalf("The name 'busybox' should not have been removed from images: %q", images)
}

deleteContainer(cleanedContainerID)

logDone("rmi- container using image while rmi, should not remove image name")
}

func TestImageTagRemove(t *testing.T) {
imagesBefore, _, _ := cmd(t, "images", "-a")
cmd(t, "tag", "busybox", "utest:tag1")
cmd(t, "tag", "busybox", "utest/docker:tag2")
cmd(t, "tag", "busybox", "utest:5000/docker:tag3")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+3 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}
}
cmd(t, "rmi", "utest/docker:tag2")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+2 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}

}
cmd(t, "rmi", "utest:5000/docker:tag3")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+1 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}

}
cmd(t, "rmi", "utest:tag1")
{
imagesAfter, _, _ := cmd(t, "images", "-a")
if nLines(imagesAfter) != nLines(imagesBefore)+0 {
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
}

}
logDone("tag,rmi- tagging the same images multiple times then removing tags")
}

0 comments on commit 32b5d14

Please sign in to comment.