Merge branch '610-improve_rmi-feature'

* Runtime: improved image removal to garbage-collect unreferenced parents
- Runtime: fixed image removal to cleanly remove tags and repositories

api.go

@@ -45,6 +45,8 @@ func httpError(w http.ResponseWriter, err error) {
 		http.Error(w, err.Error(), http.StatusNotFound)
 	} else if strings.HasPrefix(err.Error(), "Bad parameter") {
 		http.Error(w, err.Error(), http.StatusBadRequest)
+	} else if strings.HasPrefix(err.Error(), "Conflict") {
+		http.Error(w, err.Error(), http.StatusConflict)
 	} else if strings.HasPrefix(err.Error(), "Impossible") {
 		http.Error(w, err.Error(), http.StatusNotAcceptable)
 	} else {
@@ -481,14 +483,30 @@ func deleteContainers(srv *Server, version float64, w http.ResponseWriter, r *ht
 }
 
 func deleteImages(srv *Server, version float64, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
+	if err := parseForm(r); err != nil {
+		return err
+	}
 	if vars == nil {
 		return fmt.Errorf("Missing parameter")
 	}
 	name := vars["name"]
-	if err := srv.ImageDelete(name); err != nil {
+	imgs, err := srv.ImageDelete(name, version > 1.0)
+	if err != nil {
 		return err
 	}
-	w.WriteHeader(http.StatusNoContent)
+	if imgs != nil {
+		if len(*imgs) != 0 {
+			b, err := json.Marshal(imgs)
+			if err != nil {
+				return err
+			}
+			writeJSON(w, b)
+		} else {
+			return fmt.Errorf("Conflict, %s wasn't deleted", name)
+		}
+	} else {
+		w.WriteHeader(http.StatusNoContent)
+	}
 	return nil
 }
 

api_params.go

@@ -23,6 +23,11 @@ type APIInfo struct {
 	SwapLimit   bool `json:",omitempty"`
 }
 
+type APIRmi struct {
+	Deleted  string `json:",omitempty"`
+	Untagged string `json:",omitempty"`
+}
+
 type APIContainers struct {
 	ID      string `json:"Id"`
 	Image   string

api_test.go

@@ -1307,8 +1307,63 @@ func TestGetEnabledCors(t *testing.T) {
 }
 
 func TestDeleteImages(t *testing.T) {
-	//FIXME: Implement this test
+	runtime, err := newTestRuntime()
-	t.Log("Test not implemented")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(runtime)
+
+	srv := &Server{runtime: runtime}
+
+	if err := srv.runtime.repositories.Set("test", "test", unitTestImageName, true); err != nil {
+		t.Fatal(err)
+	}
+
+	images, err := srv.Images(false, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(images) != 2 {
+		t.Errorf("Excepted 2 images, %d found", len(images))
+	}
+
+	req, err := http.NewRequest("DELETE", "/images/test:test", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	r := httptest.NewRecorder()
+	if err := deleteImages(srv, APIVERSION, r, req, map[string]string{"name": "test:test"}); err != nil {
+		t.Fatal(err)
+	}
+	if r.Code != http.StatusOK {
+		t.Fatalf("%d OK expected, received %d\n", http.StatusOK, r.Code)
+	}
+
+	var outs []APIRmi
+	if err := json.Unmarshal(r.Body.Bytes(), &outs); err != nil {
+		t.Fatal(err)
+	}
+	if len(outs) != 1 {
+		t.Fatalf("Expected %d event (untagged), got %d", 1, len(outs))
+	}
+	images, err = srv.Images(false, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(images) != 1 {
+		t.Errorf("Excepted 1 image, %d found", len(images))
+	}
+
+	/*	if c := runtime.Get(container.Id); c != nil {
+			t.Fatalf("The container as not been deleted")
+		}
+
+		if _, err := os.Stat(path.Join(container.rwPath(), "test")); err == nil {
+			t.Fatalf("The test file has not been deleted")
+		} */
 }
 
 // Mocked types for tests

commands.go

@@ -590,11 +590,22 @@ func (cli *DockerCli) CmdRmi(args ...string) error {
 	}
 
 	for _, name := range cmd.Args() {
-		_, _, err := cli.call("DELETE", "/images/"+name, nil)
+		body, _, err := cli.call("DELETE", "/images/"+name, nil)
 		if err != nil {
-			fmt.Printf("%s", err)
+			fmt.Fprintf(os.Stderr, "%s", err)
 		} else {
-			fmt.Println(name)
+			var outs []APIRmi
+			err = json.Unmarshal(body, &outs)
+			if err != nil {
+				return err
+			}
+			for _, out := range outs {
+				if out.Deleted != "" {
+					fmt.Println("Deleted:", out.Deleted)
+				} else {
+					fmt.Println("Untagged:", out.Untagged)
+				}
+			}
 		}
 	}
 	return nil

docs/sources/api/docker_remote_api.rst

@@ -777,6 +777,7 @@ Tag an image into a repository
 	:statuscode 200: no error
 	:statuscode 400: bad parameter
 	:statuscode 404: no such image
+	:statuscode 409: conflict
         :statuscode 500: server error
 
 
@@ -793,14 +794,30 @@ Remove an image
 
 	   DELETE /images/test HTTP/1.1
 
-	**Example response**:
+	**Example response v1.0**:
 
         .. sourcecode:: http
 
            HTTP/1.1 204 OK
 
+	**Example response v1.1**:
+
+        .. sourcecode:: http
+
+           HTTP/1.1 200 OK
+	   Content-type: application/json
+
+	   [
+	    {"Untagged":"3e2f21a89f"},
+	    {"Deleted":"3e2f21a89f"},
+	    {"Deleted":"53b4f83ac9"}
+	   ]
+
+	:query force: 1/True/true or 0/False/false, default false
+	:statuscode 200: no error
 	:statuscode 204: no error
         :statuscode 404: no such image
+	:statuscode 409: conflict
         :statuscode 500: server error
 
 

server.go

@@ -1,6 +1,7 @@
 package docker
 
 import (
+	"errors"
 	"fmt"
 	"github.com/dotcloud/docker/auth"
 	"github.com/dotcloud/docker/registry"
@@ -717,17 +718,112 @@ func (srv *Server) ContainerDestroy(name string, removeVolume bool) error {
 	return nil
 }
 
-func (srv *Server) ImageDelete(name string) error {
+var ErrImageReferenced = errors.New("Image referenced by a repository")
-	img, err := srv.runtime.repositories.LookupImage(name)
+
-	if err != nil {
+func (srv *Server) deleteImageAndChildren(id string, imgs *[]APIRmi) error {
-		return fmt.Errorf("No such image: %s", name)
+	// If the image is referenced by a repo, do not delete
+	if len(srv.runtime.repositories.ByID()[id]) != 0 {
+		return ErrImageReferenced
 	}
-	if err := srv.runtime.graph.Delete(img.ID); err != nil {
+
-		return fmt.Errorf("Error deleting image %s: %s", name, err.Error())
+	// If the image is not referenced but has children, go recursive
+	referenced := false
+	byParents, err := srv.runtime.graph.ByParent()
+	if err != nil {
+		return err
+	}
+	for _, img := range byParents[id] {
+		if err := srv.deleteImageAndChildren(img.ID, imgs); err != nil {
+			if err != ErrImageReferenced {
+				return err
+			}
+			referenced = true
+		}
+	}
+	if referenced {
+		return ErrImageReferenced
+	}
+
+	// If the image is not referenced and has no children, remove it
+	byParents, err = srv.runtime.graph.ByParent()
+	if err != nil {
+		return err
+	}
+	if len(byParents[id]) == 0 {
+		if err := srv.runtime.repositories.DeleteAll(id); err != nil {
+			return err
+		}
+		err := srv.runtime.graph.Delete(id)
+		if err != nil {
+			return err
+		}
+		*imgs = append(*imgs, APIRmi{Deleted: utils.TruncateID(id)})
+		return nil
 	}
 	return nil
 }
 
+func (srv *Server) deleteImageParents(img *Image, imgs *[]APIRmi) error {
+	if img.Parent != "" {
+		parent, err := srv.runtime.graph.Get(img.Parent)
+		if err != nil {
+			return err
+		}
+		// Remove all children images
+		if err := srv.deleteImageAndChildren(img.Parent, imgs); err != nil {
+			return err
+		}
+		return srv.deleteImageParents(parent, imgs)
+	}
+	return nil
+}
+
+func (srv *Server) deleteImage(img *Image, repoName, tag string) (*[]APIRmi, error) {
+	//Untag the current image
+	var imgs []APIRmi
+	tagDeleted, err := srv.runtime.repositories.Delete(repoName, tag)
+	if err != nil {
+		return nil, err
+	}
+	if tagDeleted {
+		imgs = append(imgs, APIRmi{Untagged: img.ShortID()})
+	}
+	if len(srv.runtime.repositories.ByID()[img.ID]) == 0 {
+		if err := srv.deleteImageAndChildren(img.ID, &imgs); err != nil {
+			if err != ErrImageReferenced {
+				return &imgs, err
+			}
+		} else if err := srv.deleteImageParents(img, &imgs); err != nil {
+			if err != ErrImageReferenced {
+				return &imgs, err
+			}
+		}
+	}
+	return &imgs, nil
+}
+
+func (srv *Server) ImageDelete(name string, autoPrune bool) (*[]APIRmi, error) {
+	img, err := srv.runtime.repositories.LookupImage(name)
+	if err != nil {
+		return nil, fmt.Errorf("No such image: %s", name)
+	}
+	if !autoPrune {
+		if err := srv.runtime.graph.Delete(img.ID); err != nil {
+			return nil, fmt.Errorf("Error deleting image %s: %s", name, err.Error())
+		}
+		return nil, nil
+	}
+
+	var tag string
+	if strings.Contains(name, ":") {
+		nameParts := strings.Split(name, ":")
+		name = nameParts[0]
+		tag = nameParts[1]
+	}
+
+	return srv.deleteImage(img, name, tag)
+}
+
 func (srv *Server) ImageGetCached(imgId string, config *Config) (*Image, error) {
 
 	// Retrieve all images

server_test.go

@@ -4,6 +4,58 @@ import (
 	"testing"
 )
 
+func TestContainerTagImageDelete(t *testing.T) {
+	runtime, err := newTestRuntime()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(runtime)
+
+	srv := &Server{runtime: runtime}
+
+	if err := srv.runtime.repositories.Set("utest", "tag1", unitTestImageName, false); err != nil {
+		t.Fatal(err)
+	}
+	if err := srv.runtime.repositories.Set("utest/docker", "tag2", unitTestImageName, false); err != nil {
+		t.Fatal(err)
+	}
+
+	images, err := srv.Images(false, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(images) != 3 {
+		t.Errorf("Excepted 3 images, %d found", len(images))
+	}
+
+	if _, err := srv.ImageDelete("utest/docker:tag2", true); err != nil {
+		t.Fatal(err)
+	}
+
+	images, err = srv.Images(false, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(images) != 2 {
+		t.Errorf("Excepted 2 images, %d found", len(images))
+	}
+
+	if _, err := srv.ImageDelete("utest:tag1", true); err != nil {
+		t.Fatal(err)
+	}
+
+	images, err = srv.Images(false, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(images) != 1 {
+		t.Errorf("Excepted 1 image, %d found", len(images))
+	}
+}
+
 func TestCreateRm(t *testing.T) {
 	runtime, err := newTestRuntime()
 	if err != nil {

tags.go

@@ -110,6 +110,52 @@ func (store *TagStore) ImageName(id string) string {
 	return utils.TruncateID(id)
 }
 
+func (store *TagStore) DeleteAll(id string) error {
+	names, exists := store.ByID()[id]
+	if !exists || len(names) == 0 {
+		return nil
+	}
+	for _, name := range names {
+		if strings.Contains(name, ":") {
+			nameParts := strings.Split(name, ":")
+			if _, err := store.Delete(nameParts[0], nameParts[1]); err != nil {
+				return err
+			}
+		} else {
+			if _, err := store.Delete(name, ""); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func (store *TagStore) Delete(repoName, tag string) (bool, error) {
+	deleted := false
+	if err := store.Reload(); err != nil {
+		return false, err
+	}
+	if r, exists := store.Repositories[repoName]; exists {
+		if tag != "" {
+			if _, exists2 := r[tag]; exists2 {
+				delete(r, tag)
+				if len(r) == 0 {
+					delete(store.Repositories, repoName)
+				}
+				deleted = true
+			} else {
+				return false, fmt.Errorf("No such tag: %s:%s", repoName, tag)
+			}
+		} else {
+			delete(store.Repositories, repoName)
+			deleted = true
+		}
+	} else {
+		fmt.Errorf("No such repository: %s", repoName)
+	}
+	return deleted, store.Save()
+}
+
 func (store *TagStore) Set(repoName, tag, imageName string, force bool) error {
 	img, err := store.LookupImage(imageName)
 	if err != nil {
@@ -133,7 +179,7 @@ func (store *TagStore) Set(repoName, tag, imageName string, force bool) error {
 	} else {
 		repo = make(map[string]string)
 		if old, exists := store.Repositories[repoName]; exists && !force {
-			return fmt.Errorf("Tag %s:%s is already set to %s", repoName, tag, old)
+			return fmt.Errorf("Conflict: Tag %s:%s is already set to %s", repoName, tag, old)
 		}
 		store.Repositories[repoName] = repo
 	}