Skip to content

Commit b2efdc5

Browse files
jessfrazjessfraz
committed
Removing an image that fails, also removes the image name/tag.
Fixes moby#7845 and moby#7801, and a real pain point I had :) Docker-DCO-1.1-Signed-off-by: Jessica Frazelle <jess@docker.com> (github: jfrazelle)
1 parent b5a4c70 commit b2efdc5

File tree

3 files changed

+92
-53
lines changed

3 files changed

+92
-53
lines changed

‎daemon/image_delete.go‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
3333
var (
3434
repoName, tag string
3535
tags = []string{}
36-
tagDeleted bool
3736
)
3837

3938
// 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.
6059
return err
6160
}
6261

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

86-
//Untag the current image
87+
if len(repos) <= 1 {
88+
if err := daemon.canDeleteImage(img.ID, force); err != nil {
89+
return err
90+
}
91+
}
92+
93+
// Untag the current image
8794
for _, tag := range tags {
88-
tagDeleted, err = daemon.Repositories().Delete(repoName, tag)
95+
tagDeleted, err := daemon.Repositories().Delete(repoName, tag)
8996
if err != nil {
9097
return err
9198
}
@@ -99,9 +106,6 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
99106
tags = daemon.Repositories().ByID()[img.ID]
100107
if (len(tags) <= 1 && repoName == "") || len(tags) == 0 {
101108
if len(byParents[img.ID]) == 0 {
102-
if err := daemon.canDeleteImage(img.ID, force, tagDeleted); err != nil {
103-
return err
104-
}
105109
if err := daemon.Repositories().DeleteAll(img.ID); err != nil {
106110
return err
107111
}
@@ -125,11 +129,7 @@ func (daemon *Daemon) DeleteImage(eng *engine.Engine, name string, imgs *engine.
125129
return nil
126130
}
127131

128-
func (daemon *Daemon) canDeleteImage(imgID string, force, untagged bool) error {
129-
var message string
130-
if untagged {
131-
message = " (docker untagged the image)"
132-
}
132+
func (daemon *Daemon) canDeleteImage(imgID string, force bool) error {
133133
for _, container := range daemon.List() {
134134
parent, err := daemon.Repositories().LookupImage(container.Image)
135135
if err != nil {
@@ -140,11 +140,11 @@ func (daemon *Daemon) canDeleteImage(imgID string, force, untagged bool) error {
140140
if imgID == p.ID {
141141
if container.IsRunning() {
142142
if force {
143-
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)
143+
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))
144144
}
145-
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)
145+
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))
146146
} else if !force {
147-
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)
147+
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))
148148
}
149149
}
150150
return nil

‎integration-cli/docker_cli_images_test.go‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,44 +20,6 @@ func TestImagesEnsureImageIsListed(t *testing.T) {
2020
logDone("images - busybox should be listed")
2121
}
2222

23-
func TestCLIImageTagRemove(t *testing.T) {
24-
imagesBefore, _, _ := cmd(t, "images", "-a")
25-
cmd(t, "tag", "busybox", "utest:tag1")
26-
cmd(t, "tag", "busybox", "utest/docker:tag2")
27-
cmd(t, "tag", "busybox", "utest:5000/docker:tag3")
28-
{
29-
imagesAfter, _, _ := cmd(t, "images", "-a")
30-
if nLines(imagesAfter) != nLines(imagesBefore)+3 {
31-
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
32-
}
33-
}
34-
cmd(t, "rmi", "utest/docker:tag2")
35-
{
36-
imagesAfter, _, _ := cmd(t, "images", "-a")
37-
if nLines(imagesAfter) != nLines(imagesBefore)+2 {
38-
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
39-
}
40-
41-
}
42-
cmd(t, "rmi", "utest:5000/docker:tag3")
43-
{
44-
imagesAfter, _, _ := cmd(t, "images", "-a")
45-
if nLines(imagesAfter) != nLines(imagesBefore)+1 {
46-
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
47-
}
48-
49-
}
50-
cmd(t, "rmi", "utest:tag1")
51-
{
52-
imagesAfter, _, _ := cmd(t, "images", "-a")
53-
if nLines(imagesAfter) != nLines(imagesBefore)+0 {
54-
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
55-
}
56-
57-
}
58-
logDone("tag,rmi- tagging the same images multiple times then removing tags")
59-
}
60-
6123
func TestImagesOrderedByCreationDate(t *testing.T) {
6224
defer deleteImages("order:test_a")
6325
defer deleteImages("order:test_c")

‎integration-cli/docker_cli_rmi_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"os/exec"
6+
"strings"
7+
"testing"
8+
)
9+
10+
func TestImageRemoveWithContainerFails(t *testing.T) {
11+
errSubstr := "is using it"
12+
13+
// create a container
14+
runCmd := exec.Command(dockerBinary, "run", "-d", "busybox", "true")
15+
out, _, err := runCommandWithOutput(runCmd)
16+
errorOut(err, t, fmt.Sprintf("failed to create a container: %v %v", out, err))
17+
18+
cleanedContainerID := stripTrailingCharacters(out)
19+
20+
// try to delete the image
21+
runCmd = exec.Command(dockerBinary, "rmi", "busybox")
22+
out, _, err = runCommandWithOutput(runCmd)
23+
if err == nil {
24+
t.Fatalf("Container %q is using image, should not be able to rmi: %q", cleanedContainerID, out)
25+
}
26+
if !strings.Contains(out, errSubstr) {
27+
t.Fatalf("Container %q is using image, error message should contain %q: %v", cleanedContainerID, errSubstr, out)
28+
}
29+
30+
// make sure it didn't delete the busybox name
31+
images, _, _ := cmd(t, "images")
32+
if !strings.Contains(images, "busybox") {
33+
t.Fatalf("The name 'busybox' should not have been removed from images: %q", images)
34+
}
35+
36+
deleteContainer(cleanedContainerID)
37+
38+
logDone("rmi- container using image while rmi, should not remove image name")
39+
}
40+
41+
func TestImageTagRemove(t *testing.T) {
42+
imagesBefore, _, _ := cmd(t, "images", "-a")
43+
cmd(t, "tag", "busybox", "utest:tag1")
44+
cmd(t, "tag", "busybox", "utest/docker:tag2")
45+
cmd(t, "tag", "busybox", "utest:5000/docker:tag3")
46+
{
47+
imagesAfter, _, _ := cmd(t, "images", "-a")
48+
if nLines(imagesAfter) != nLines(imagesBefore)+3 {
49+
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
50+
}
51+
}
52+
cmd(t, "rmi", "utest/docker:tag2")
53+
{
54+
imagesAfter, _, _ := cmd(t, "images", "-a")
55+
if nLines(imagesAfter) != nLines(imagesBefore)+2 {
56+
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
57+
}
58+
59+
}
60+
cmd(t, "rmi", "utest:5000/docker:tag3")
61+
{
62+
imagesAfter, _, _ := cmd(t, "images", "-a")
63+
if nLines(imagesAfter) != nLines(imagesBefore)+1 {
64+
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
65+
}
66+
67+
}
68+
cmd(t, "rmi", "utest:tag1")
69+
{
70+
imagesAfter, _, _ := cmd(t, "images", "-a")
71+
if nLines(imagesAfter) != nLines(imagesBefore)+0 {
72+
t.Fatalf("before: %q\n\nafter: %q\n", imagesBefore, imagesAfter)
73+
}
74+
75+
}
76+
logDone("tag,rmi- tagging the same images multiple times then removing tags")
77+
}

0 commit comments

Comments
 (0)