Merge pull request #3230 from crosbymichael/allow-untag

Allow untag operations with no container validation

integration/server_test.go

@@ -409,3 +409,50 @@ func TestImageInsert(t *testing.T) {
 		t.Fatalf("expected no error, but got %v", err)
 	}
 }
+
+// Regression test for being able to untag an image with an existing
+// container
+func TestDeleteTagWithExistingContainers(t *testing.T) {
+	eng := NewTestEngine(t)
+	defer nuke(mkRuntimeFromEngine(eng, t))
+
+	srv := mkServerFromEngine(eng, t)
+
+	// Tag the image
+	if err := eng.Job("tag", unitTestImageID, "utest", "tag1").Run(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Create a container from the image
+	config, _, _, err := docker.ParseRun([]string{unitTestImageID, "echo test"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	id := createNamedTestContainer(eng, config, t, "testingtags")
+	if id == "" {
+		t.Fatal("No id returned")
+	}
+
+	containers := srv.Containers(true, false, -1, "", "")
+
+	if len(containers) != 1 {
+		t.Fatalf("Expected 1 container got %d", len(containers))
+	}
+
+	// Try to remove the tag
+	imgs, err := srv.ImageDelete("utest:tag1", true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(imgs) != 1 {
+		t.Fatalf("Should only have deleted one untag %d", len(imgs))
+	}
+
+	untag := imgs[0]
+
+	if untag.Untagged != unitTestImageID {
+		t.Fatalf("Expected %s got %s", unitTestImageID, untag.Untagged)
+	}
+}

server.go

@@ -1550,8 +1550,10 @@ func (srv *Server) deleteImageParents(img *Image, imgs *[]APIRmi) error {
 }
 
 func (srv *Server) deleteImage(img *Image, repoName, tag string) ([]APIRmi, error) {
-	imgs := []APIRmi{}
-	tags := []string{}
+	var (
+		imgs = []APIRmi{}
+		tags = []string{}
+	)
 
 	//If delete by id, see if the id belong only to one repository
 	if repoName == "" {
@@ -1571,6 +1573,7 @@ func (srv *Server) deleteImage(img *Image, repoName, tag string) ([]APIRmi, erro
 	} else {
 		tags = append(tags, tag)
 	}
+
 	//Untag the current image
 	for _, tag := range tags {
 		tagDeleted, err := srv.runtime.repositories.Delete(repoName, tag)
@@ -1582,6 +1585,7 @@ func (srv *Server) deleteImage(img *Image, repoName, tag string) ([]APIRmi, erro
 			srv.LogEvent("untag", img.ID, "")
 		}
 	}
+
 	if len(srv.runtime.repositories.ByID()[img.ID]) == 0 {
 		if err := srv.deleteImageAndChildren(img.ID, &imgs, nil); err != nil {
 			if err != ErrImageReferenced {
@@ -1597,10 +1601,16 @@ 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)
 	if err != nil {
 		return nil, fmt.Errorf("No such image: %s", name)
 	}
+
+	// FIXME: What does autoPrune mean ?
 	if !autoPrune {
 		if err := srv.runtime.graph.Delete(img.ID); err != nil {
 			return nil, fmt.Errorf("Cannot delete image %s: %s", name, err)
@@ -1608,29 +1618,38 @@ func (srv *Server) ImageDelete(name string, autoPrune bool) ([]APIRmi, error) {
 		return nil, nil
 	}
 
-	// Prevent deletion if image is used by a container
-	for _, container := range srv.runtime.List() {
-		parent, err := srv.runtime.repositories.LookupImage(container.Image)
-		if err != nil {
-			return nil, err
-		}
+	if !strings.Contains(img.ID, name) {
+		repository, tag = utils.ParseRepositoryTag(name)
+	}
 
-		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
-		}
+	// If we have a repo and the image is not referenced anywhere else
+	// then just perform an untag and do not validate.
+	//
+	// i.e. only validate if we are performing an actual delete and not
+	// an untag op
+	if repository != "" {
+		validate = len(srv.runtime.repositories.ByID()[img.ID]) == 1
 	}
 
-	if strings.Contains(img.ID, name) {
-		//delete via ID
-		return srv.deleteImage(img, "", "")
+	if validate {
+		// Prevent deletion if image is used by a container
+		for _, container := range srv.runtime.List() {
+			parent, err := srv.runtime.repositories.LookupImage(container.Image)
+			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
+			}
+		}
 	}
-	name, tag := utils.ParseRepositoryTag(name)
-	return srv.deleteImage(img, name, tag)
+	return srv.deleteImage(img, repository, tag)
 }
 
 func (srv *Server) ImageGetCached(imgID string, config *Config) (*Image, error) {