Merge pull request #3304 from vieux/prevent_orphan_deletion

Prevent orphan in docker rmi

integration/commands_test.go

@@ -968,3 +968,66 @@ func TestRunCidFile(t *testing.T) {
 	})
 
 }
+
+func TestContainerOrphaning(t *testing.T) {
+
+	// setup a temporary directory
+	tmpDir, err := ioutil.TempDir("", "project")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// setup a CLI and server
+	cli := docker.NewDockerCli(nil, ioutil.Discard, ioutil.Discard, testDaemonProto, testDaemonAddr)
+	defer cleanup(globalEngine, t)
+	srv := mkServerFromEngine(globalEngine, t)
+
+	// closure to build something
+	buildSomething := func(template string, image string) string {
+		dockerfile := path.Join(tmpDir, "Dockerfile")
+		replacer := strings.NewReplacer("{IMAGE}", unitTestImageID)
+		contents := replacer.Replace(template)
+		ioutil.WriteFile(dockerfile, []byte(contents), 0x777)
+		if err := cli.CmdBuild("-t", image, tmpDir); err != nil {
+			t.Fatal(err)
+		}
+		img, err := srv.ImageInspect(image)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return img.ID
+	}
+
+	// build an image
+	imageName := "orphan-test"
+	template1 := `
+	from {IMAGE}
+	cmd ["/bin/echo", "holla"]
+	`
+	img1 := buildSomething(template1, imageName)
+
+	// create a container using the fist image
+	if err := cli.CmdRun(imageName); err != nil {
+		t.Fatal(err)
+	}
+
+	// build a new image that splits lineage
+	template2 := `
+	from {IMAGE}
+	cmd ["/bin/echo", "holla"]
+	expose 22
+	`
+	buildSomething(template2, imageName)
+
+	// remove the second image by name
+	resp, err := srv.ImageDelete(imageName, true)
+
+	// see if we deleted the first image (and orphaned the container)
+	for _, i := range resp {
+		if img1 == i.Deleted {
+			t.Fatal("Orphaned image with container")
+		}
+	}
+
+}

server.go

@@ -1543,7 +1543,7 @@ func (srv *Server) deleteImageAndChildren(id string, imgs *[]APIRmi, byParents m
 	if err != nil {
 		return err
 	}
-	if len(byParents[id]) == 0 {
+	if len(byParents[id]) == 0 && srv.canDeleteImage(id) == nil {
 		if err := srv.runtime.repositories.DeleteAll(id); err != nil {
 			return err
 		}
@@ -1631,9 +1631,8 @@ func (srv *Server) deleteImage(img *Image, repoName, tag string) ([]APIRmi, erro
 func (srv *Server) ImageDelete(name string, autoPrune bool) ([]APIRmi, error) {
 	var (
 		repository, tag string
-		validate        = true
+		img, err        = srv.runtime.repositories.LookupImage(name)
 	)
-	img, err := srv.runtime.repositories.LookupImage(name)
 	if err != nil {
 		return nil, fmt.Errorf("No such image: %s", name)
 	}
@@ -1655,31 +1654,34 @@ func (srv *Server) ImageDelete(name string, autoPrune bool) ([]APIRmi, error) {
 	//
 	// i.e. only validate if we are performing an actual delete and not
 	// an untag op
-	if repository != "" {
+	if repository != "" && len(srv.runtime.repositories.ByID()[img.ID]) == 1 {
-		validate = len(srv.runtime.repositories.ByID()[img.ID]) == 1
-	}
-
-	if validate {
 		// Prevent deletion if image is used by a container
-		for _, container := range srv.runtime.List() {
+		if err := srv.canDeleteImage(img.ID); err != nil {
-			parent, err := srv.runtime.repositories.LookupImage(container.Image)
+			return nil, err
-			if err != nil {
-				return nil, err
-			}
-
-			if err := parent.WalkHistory(func(p *Image) error {
-				if img.ID == p.ID {
-					return fmt.Errorf("Conflict, cannot delete %s because the container %s is using it", name, container.ID)
-				}
-				return nil
-			}); err != nil {
-				return nil, err
-			}
 		}
 	}
 	return srv.deleteImage(img, repository, tag)
 }
 
+func (srv *Server) canDeleteImage(imgID string) error {
+	for _, container := range srv.runtime.List() {
+		parent, err := srv.runtime.repositories.LookupImage(container.Image)
+		if err != nil {
+			return err
+		}
+
+		if err := parent.WalkHistory(func(p *Image) error {
+			if imgID == p.ID {
+				return fmt.Errorf("Conflict, cannot delete %s because the container %s is using it", utils.TruncateID(imgID), utils.TruncateID(container.ID))
+			}
+			return nil
+		}); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (srv *Server) ImageGetCached(imgID string, config *Config) (*Image, error) {
 
 	// Retrieve all images