diff --git a/daemon/image_delete.go b/daemon/image_delete.go index 00e98edfbb..332db7b4c0 100644 --- a/daemon/image_delete.go +++ b/daemon/image_delete.go @@ -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 @@ -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 @@ -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 } @@ -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 } @@ -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 { @@ -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 diff --git a/integration-cli/docker_cli_images_test.go b/integration-cli/docker_cli_images_test.go index 2cf2bbfcf5..5a7207cec5 100644 --- a/integration-cli/docker_cli_images_test.go +++ b/integration-cli/docker_cli_images_test.go @@ -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") diff --git a/integration-cli/docker_cli_rmi_test.go b/integration-cli/docker_cli_rmi_test.go new file mode 100644 index 0000000000..4b36151efe --- /dev/null +++ b/integration-cli/docker_cli_rmi_test.go @@ -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") +}