From c80448c4d1836dd0e23953fa923f7a270d32df05 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Wed, 15 May 2013 13:11:39 +0000 Subject: [PATCH 01/12] improve rmi --- api_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++-- server.go | 36 ++++++++++++++++++++++++++++++++++ server_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ tags.go | 23 ++++++++++++++++++++++ 4 files changed, 156 insertions(+), 2 deletions(-) diff --git a/api_test.go b/api_test.go index 5096b05212..17075dae7f 100644 --- a/api_test.go +++ b/api_test.go @@ -1189,8 +1189,51 @@ func TestDeleteContainers(t *testing.T) { } func TestDeleteImages(t *testing.T) { - //FIXME: Implement this test - t.Log("Test not implemented") + runtime, err := newTestRuntime() + 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)) + } + + r := httptest.NewRecorder() + if err := deleteImages(srv, r, nil, map[string]string{"name": "test:test"}); err != nil { + t.Fatal(err) + } + if r.Code != http.StatusNoContent { + t.Fatalf("%d NO CONTENT expected, received %d\n", http.StatusNoContent, r.Code) + } + + 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 diff --git a/server.go b/server.go index 453574946d..d749ead73c 100644 --- a/server.go +++ b/server.go @@ -439,9 +439,45 @@ func (srv *Server) ImageDelete(name string) error { if err != nil { return fmt.Errorf("No such image: %s", name) } else { + tag := "" + if strings.Contains(name, ":") { + nameParts := strings.Split(name, ":") + name = nameParts[0] + tag = nameParts[1] + } + // if the images is referenced several times + Debugf("Image %s referenced %d times", img.Id, len(srv.runtime.repositories.ById()[img.Id])) + if len(srv.runtime.repositories.ById()[img.Id]) > 1 { + // if it's repo:tag, try to delete the tag (docker rmi base:latest) + if tag != "" { + if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + return err + } + return nil + } else { + // check if the image is referenced in another repo (base and user/base are the same, docker rmi user/base) + var other bool + for _, repoTag := range srv.runtime.repositories.ById()[img.Id] { + if !strings.Contains(repoTag, name+":") { + other = true + break + } + } + // if found in another repo, delete the repo, other delete the whole image (docker rmi base) + if other { + if err := srv.runtime.repositories.Delete(name, "", img.Id); err != nil { + return err + } + return nil + } + } + } if err := srv.runtime.graph.Delete(img.Id); err != nil { return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) } + if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + return err + } } return nil } diff --git a/server_test.go b/server_test.go index 7b90252864..fe1cbecd9b 100644 --- a/server_test.go +++ b/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"); 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"); 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 { diff --git a/tags.go b/tags.go index 1b9cd19c83..b7aa692477 100644 --- a/tags.go +++ b/tags.go @@ -109,6 +109,29 @@ func (store *TagStore) ImageName(id string) string { return TruncateId(id) } +func (store *TagStore) Delete(repoName, tag, imageName string) error { + if err := store.Reload(); err != nil { + return 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) + } + } else { + return fmt.Errorf("No such tag: %s:%s", repoName, tag) + } + } else { + delete(store.Repositories, repoName) + } + } else { + fmt.Errorf("No such repository: %s", repoName) + } + return store.Save() +} + func (store *TagStore) Set(repoName, tag, imageName string, force bool) error { img, err := store.LookupImage(imageName) if err != nil { From db1e965b657e7e184b9f4e136e9c0860769d8f68 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 16 May 2013 11:27:50 -0700 Subject: [PATCH 02/12] Merge fixes + cleanup --- server.go | 62 +++++++++++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 34 deletions(-) diff --git a/server.go b/server.go index 8b97555ba6..03030e120f 100644 --- a/server.go +++ b/server.go @@ -699,46 +699,40 @@ func (srv *Server) ImageDelete(name string) error { img, err := srv.runtime.repositories.LookupImage(name) if err != nil { return fmt.Errorf("No such image: %s", name) - } else { - tag := "" - if strings.Contains(name, ":") { - nameParts := strings.Split(name, ":") - name = nameParts[0] - tag = nameParts[1] + } + var tag string + if strings.Contains(name, ":") { + nameParts := strings.Split(name, ":") + name = nameParts[0] + tag = nameParts[1] + } + + // if the images is referenced several times + utils.Debugf("Image %s referenced %d times", img.Id, len(srv.runtime.repositories.ById()[img.Id])) + if len(srv.runtime.repositories.ById()[img.Id]) > 1 { + // if it's repo:tag, try to delete the tag (docker rmi base:latest) + if tag != "" { + if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + return err + } + return nil } - // if the images is referenced several times - Debugf("Image %s referenced %d times", img.Id, len(srv.runtime.repositories.ById()[img.Id])) - if len(srv.runtime.repositories.ById()[img.Id]) > 1 { - // if it's repo:tag, try to delete the tag (docker rmi base:latest) - if tag != "" { - if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + // check if the image is referenced in another repo (base and user/base are the same, docker rmi user/base) + for _, repoTag := range srv.runtime.repositories.ById()[img.Id] { + if !strings.Contains(repoTag, name+":") { + // if found in another repo, delete the repo, otherwie delete the whole image (docker rmi base) + if err := srv.runtime.repositories.Delete(name, "", img.Id); err != nil { return err } return nil - } else { - // check if the image is referenced in another repo (base and user/base are the same, docker rmi user/base) - var other bool - for _, repoTag := range srv.runtime.repositories.ById()[img.Id] { - if !strings.Contains(repoTag, name+":") { - other = true - break - } - } - // if found in another repo, delete the repo, other delete the whole image (docker rmi base) - if other { - if err := srv.runtime.repositories.Delete(name, "", img.Id); err != nil { - return err - } - return nil - } } } - if err := srv.runtime.graph.Delete(img.Id); err != nil { - return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) - } - if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { - return err - } + } + if err := srv.runtime.graph.Delete(img.Id); err != nil { + return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) + } + if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + return err } return nil } From f01990aad2200690618cd64d2378e7e610433a71 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Fri, 17 May 2013 17:57:44 +0000 Subject: [PATCH 03/12] fix --- server.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server.go b/server.go index 03030e120f..1ad1d80a4f 100644 --- a/server.go +++ b/server.go @@ -717,10 +717,11 @@ func (srv *Server) ImageDelete(name string) error { } return nil } - // check if the image is referenced in another repo (base and user/base are the same, docker rmi user/base) + // Let's say you have the same image referenced by base and base2 + // check if the image is referenced in another repo (docker rmi base) for _, repoTag := range srv.runtime.repositories.ById()[img.Id] { - if !strings.Contains(repoTag, name+":") { - // if found in another repo, delete the repo, otherwie delete the whole image (docker rmi base) + if !strings.HasPrefix(repoTag, name+":") { + // if found in another repo (base2) delete the repo base, otherwise delete the whole image if err := srv.runtime.repositories.Delete(name, "", img.Id); err != nil { return err } From d7673274d22140dc6dfa288351f1d4d2e2fa4b63 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Sat, 18 May 2013 14:29:32 +0000 Subject: [PATCH 04/12] check if the image to delete isn't parent of another --- server.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/server.go b/server.go index 1ad1d80a4f..d709aba7e2 100644 --- a/server.go +++ b/server.go @@ -729,6 +729,15 @@ func (srv *Server) ImageDelete(name string) error { } } } + // check is the image to delete isn't parent of another image + images, _ := srv.runtime.graph.All() + for _, image := range images { + if imgParent, err := image.GetParent(); err == nil && imgParent != nil { + if imgParent.Id == img.Id { + return fmt.Errorf("Can't delete %s, otherwise %s will be broken", name, image.ShortId()) + } + } + } if err := srv.runtime.graph.Delete(img.Id); err != nil { return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) } From 67b20f2c8cc8d85ab09de95970e4d28c80f0aeb6 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Mon, 20 May 2013 18:31:45 +0000 Subject: [PATCH 05/12] add check to see if the image isn't parent of another and add -f to force --- api.go | 11 ++++++++++- api_test.go | 7 ++++++- commands.go | 8 +++++++- docs/sources/api/docker_remote_api.rst | 3 +++ server.go | 14 ++++++++------ server_test.go | 4 ++-- tags.go | 2 +- 7 files changed, 37 insertions(+), 12 deletions(-) diff --git a/api.go b/api.go index 8984d00cd9..53a895918c 100644 --- a/api.go +++ b/api.go @@ -36,6 +36,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 { http.Error(w, err.Error(), http.StatusInternalServerError) } @@ -453,11 +455,18 @@ func deleteContainers(srv *Server, w http.ResponseWriter, r *http.Request, vars } func deleteImages(srv *Server, 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 { + force, err := getBoolParam(r.Form.Get("force")) + if err != nil { + return err + } + if err := srv.ImageDelete(name, force); err != nil { return err } w.WriteHeader(http.StatusNoContent) diff --git a/api_test.go b/api_test.go index 921aebe4ce..283ebb130d 100644 --- a/api_test.go +++ b/api_test.go @@ -1312,8 +1312,13 @@ func TestDeleteImages(t *testing.T) { 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, r, nil, map[string]string{"name": "test:test"}); err != nil { + if err := deleteImages(srv, r, req, map[string]string{"name": "test:test"}); err != nil { t.Fatal(err) } if r.Code != http.StatusNoContent { diff --git a/commands.go b/commands.go index 33ba8125de..26cea0189e 100644 --- a/commands.go +++ b/commands.go @@ -459,6 +459,7 @@ func (cli *DockerCli) CmdPort(args ...string) error { // 'docker rmi IMAGE' removes all images with the name IMAGE func (cli *DockerCli) CmdRmi(args ...string) error { cmd := Subcmd("rmi", "IMAGE [IMAGE...]", "Remove an image") + force := cmd.Bool("f", false, "Force") if err := cmd.Parse(args); err != nil { return nil } @@ -467,8 +468,13 @@ func (cli *DockerCli) CmdRmi(args ...string) error { return nil } + v := url.Values{} + if *force { + v.Set("force", "1") + } + for _, name := range cmd.Args() { - _, _, err := cli.call("DELETE", "/images/"+name, nil) + _, _, err := cli.call("DELETE", "/images/"+name+"?"+v.Encode(), nil) if err != nil { fmt.Printf("%s", err) } else { diff --git a/docs/sources/api/docker_remote_api.rst b/docs/sources/api/docker_remote_api.rst index 03f1d4b9cf..9541e27de7 100644 --- a/docs/sources/api/docker_remote_api.rst +++ b/docs/sources/api/docker_remote_api.rst @@ -728,6 +728,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 @@ -750,8 +751,10 @@ Remove an image HTTP/1.1 204 OK + :query force: 1/True/true or 0/False/false, default false :statuscode 204: no error :statuscode 404: no such image + :statuscode 409: conflict :statuscode 500: server error diff --git a/server.go b/server.go index d90e7fda08..9a2ab4edf5 100644 --- a/server.go +++ b/server.go @@ -710,7 +710,7 @@ func (srv *Server) ContainerDestroy(name string, removeVolume bool) error { return nil } -func (srv *Server) ImageDelete(name string) error { +func (srv *Server) ImageDelete(name string, force bool) error { img, err := srv.runtime.repositories.LookupImage(name) if err != nil { return fmt.Errorf("No such image: %s", name) @@ -745,11 +745,13 @@ func (srv *Server) ImageDelete(name string) error { } } // check is the image to delete isn't parent of another image - images, _ := srv.runtime.graph.All() - for _, image := range images { - if imgParent, err := image.GetParent(); err == nil && imgParent != nil { - if imgParent.Id == img.Id { - return fmt.Errorf("Can't delete %s, otherwise %s will be broken", name, image.ShortId()) + if !force { + images, _ := srv.runtime.graph.All() + for _, image := range images { + if imgParent, err := image.GetParent(); err == nil && imgParent != nil { + if imgParent.Id == img.Id { + return fmt.Errorf("Conflict: Can't delete %s otherwise %s will be broken", name, image.ShortId()) + } } } } diff --git a/server_test.go b/server_test.go index fe1cbecd9b..f223fbe53d 100644 --- a/server_test.go +++ b/server_test.go @@ -29,7 +29,7 @@ func TestContainerTagImageDelete(t *testing.T) { t.Errorf("Excepted 3 images, %d found", len(images)) } - if err := srv.ImageDelete("utest/docker:tag2"); err != nil { + if err := srv.ImageDelete("utest/docker:tag2", true); err != nil { t.Fatal(err) } @@ -42,7 +42,7 @@ func TestContainerTagImageDelete(t *testing.T) { t.Errorf("Excepted 2 images, %d found", len(images)) } - if err := srv.ImageDelete("utest:tag1"); err != nil { + if err := srv.ImageDelete("utest:tag1", true); err != nil { t.Fatal(err) } diff --git a/tags.go b/tags.go index f6b3a93a95..4a54398c5f 100644 --- a/tags.go +++ b/tags.go @@ -156,7 +156,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 } From 2eb4e2a0b86f8d244f0b6cbeb0422f8499d834f8 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Wed, 29 May 2013 16:31:47 +0000 Subject: [PATCH 06/12] removed the -f --- api.go | 6 +----- api_test.go | 2 +- commands.go | 8 +------- server.go | 18 +++++++++++------- server_test.go | 4 ++-- 5 files changed, 16 insertions(+), 22 deletions(-) diff --git a/api.go b/api.go index 8f2befb237..1759257e6e 100644 --- a/api.go +++ b/api.go @@ -450,11 +450,7 @@ func deleteImages(srv *Server, version float64, w http.ResponseWriter, r *http.R return fmt.Errorf("Missing parameter") } name := vars["name"] - force, err := getBoolParam(r.Form.Get("force")) - if err != nil { - return err - } - if err := srv.ImageDelete(name, force); err != nil { + if err := srv.ImageDelete(name); err != nil { return err } w.WriteHeader(http.StatusNoContent) diff --git a/api_test.go b/api_test.go index fb1cd211ad..844d15cc13 100644 --- a/api_test.go +++ b/api_test.go @@ -1268,7 +1268,7 @@ func TestDeleteImages(t *testing.T) { } r := httptest.NewRecorder() - if err := deleteImages(srv, r, req, map[string]string{"name": "test:test"}); err != nil { + if err := deleteImages(srv, API_VERSION, r, req, map[string]string{"name": "test:test"}); err != nil { t.Fatal(err) } if r.Code != http.StatusNoContent { diff --git a/commands.go b/commands.go index 704f5ac48d..2af7e548fc 100644 --- a/commands.go +++ b/commands.go @@ -558,7 +558,6 @@ func (cli *DockerCli) CmdPort(args ...string) error { // 'docker rmi IMAGE' removes all images with the name IMAGE func (cli *DockerCli) CmdRmi(args ...string) error { cmd := Subcmd("rmi", "IMAGE [IMAGE...]", "Remove an image") - force := cmd.Bool("f", false, "Force") if err := cmd.Parse(args); err != nil { return nil } @@ -567,13 +566,8 @@ func (cli *DockerCli) CmdRmi(args ...string) error { return nil } - v := url.Values{} - if *force { - v.Set("force", "1") - } - for _, name := range cmd.Args() { - _, _, err := cli.call("DELETE", "/images/"+name+"?"+v.Encode(), nil) + _, _, err := cli.call("DELETE", "/images/"+name, nil) if err != nil { fmt.Printf("%s", err) } else { diff --git a/server.go b/server.go index d1a089c3a9..2e819728b4 100644 --- a/server.go +++ b/server.go @@ -704,7 +704,7 @@ func (srv *Server) ContainerDestroy(name string, removeVolume bool) error { return nil } -func (srv *Server) ImageDelete(name string, force bool) error { +func (srv *Server) ImageDelete(name string) error { img, err := srv.runtime.repositories.LookupImage(name) if err != nil { return fmt.Errorf("No such image: %s", name) @@ -739,13 +739,17 @@ func (srv *Server) ImageDelete(name string, force bool) error { } } // check is the image to delete isn't parent of another image - if !force { - images, _ := srv.runtime.graph.All() - for _, image := range images { - if imgParent, err := image.GetParent(); err == nil && imgParent != nil { - if imgParent.Id == img.Id { - return fmt.Errorf("Conflict: Can't delete %s otherwise %s will be broken", name, image.ShortId()) + images, _ := srv.runtime.graph.All() + for _, image := range images { + if imgParent, err := image.GetParent(); err == nil && imgParent != nil { + if imgParent.Id == img.Id { + if strings.Contains(img.Id, name) { + return fmt.Errorf("Conflict with %s, %s was not removed", image.ShortId(), name) } + if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + return err + } + return nil } } } diff --git a/server_test.go b/server_test.go index 743b33d545..541c927b83 100644 --- a/server_test.go +++ b/server_test.go @@ -29,7 +29,7 @@ func TestContainerTagImageDelete(t *testing.T) { t.Errorf("Excepted 3 images, %d found", len(images)) } - if err := srv.ImageDelete("utest/docker:tag2", true); err != nil { + if err := srv.ImageDelete("utest/docker:tag2"); err != nil { t.Fatal(err) } @@ -42,7 +42,7 @@ func TestContainerTagImageDelete(t *testing.T) { t.Errorf("Excepted 2 images, %d found", len(images)) } - if err := srv.ImageDelete("utest:tag1", true); err != nil { + if err := srv.ImageDelete("utest:tag1"); err != nil { t.Fatal(err) } From 94f0d478de6e50e8b26e9779da22dbd74cc5952f Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Wed, 29 May 2013 17:01:54 +0000 Subject: [PATCH 07/12] refacto --- server.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/server.go b/server.go index 2e819728b4..64bde12c4d 100644 --- a/server.go +++ b/server.go @@ -739,19 +739,15 @@ func (srv *Server) ImageDelete(name string) error { } } // check is the image to delete isn't parent of another image - images, _ := srv.runtime.graph.All() - for _, image := range images { - if imgParent, err := image.GetParent(); err == nil && imgParent != nil { - if imgParent.Id == img.Id { - if strings.Contains(img.Id, name) { - return fmt.Errorf("Conflict with %s, %s was not removed", image.ShortId(), name) - } - if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { - return err - } - return nil - } + byParent, _ := srv.runtime.graph.ByParent() + if childs, exists := byParent[img.Id]; exists { + if strings.Contains(img.Id, name) { + return fmt.Errorf("Conflict with %s, %s was not removed", childs[0].ShortId(), name) } + if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + return err + } + return nil } if err := srv.runtime.graph.Delete(img.Id); err != nil { return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) From 7e92302c4fba259ac1128db548cf01dad9ad087c Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Wed, 29 May 2013 17:27:32 +0000 Subject: [PATCH 08/12] auto prune WIP --- server.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/server.go b/server.go index 64bde12c4d..69ead3aff6 100644 --- a/server.go +++ b/server.go @@ -749,11 +749,23 @@ func (srv *Server) ImageDelete(name string) error { } return nil } - if err := srv.runtime.graph.Delete(img.Id); err != nil { - return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) - } - if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { - return err + parents, _ := img.History() + for _, parent := range parents { + byParent, _ = srv.runtime.graph.ByParent() + //stop if image has children + if _, exists := byParent[parent.Id]; exists { + break + } + //stop if image is tagged and it is not the first image we delete + if _, hasTags := srv.runtime.repositories.ById()[parent.Id]; hasTags && img.Id != parent.Id { + break + } + if err := srv.runtime.graph.Delete(parent.Id); err != nil { + return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) + } + if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { + return err + } } return nil } From c05e9f856d5f12a1244924b02bad66ba5bacb550 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Wed, 29 May 2013 11:11:19 -0700 Subject: [PATCH 09/12] Error output fix for docker rmi --- commands.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/commands.go b/commands.go index 2af7e548fc..0289048b89 100644 --- a/commands.go +++ b/commands.go @@ -567,9 +567,8 @@ func (cli *DockerCli) CmdRmi(args ...string) error { } for _, name := range cmd.Args() { - _, _, err := cli.call("DELETE", "/images/"+name, nil) - if err != nil { - fmt.Printf("%s", err) + if _, _, err := cli.call("DELETE", "/images/"+name, nil); err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) } else { fmt.Println(name) } From 054451fd190f195e06d8a8aa65e83882f60b1f14 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 30 May 2013 12:30:21 -0700 Subject: [PATCH 10/12] NON-WORKING: Beginning of rmi refactor --- server.go | 108 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 83 insertions(+), 25 deletions(-) diff --git a/server.go b/server.go index 69ead3aff6..126ac7d6e9 100644 --- a/server.go +++ b/server.go @@ -1,6 +1,7 @@ package docker import ( + "errors" "fmt" "github.com/dotcloud/docker/auth" "github.com/dotcloud/docker/registry" @@ -704,11 +705,86 @@ func (srv *Server) ContainerDestroy(name string, removeVolume bool) error { return nil } -func (srv *Server) ImageDelete(name string) error { - img, err := srv.runtime.repositories.LookupImage(name) - if err != nil { - return fmt.Errorf("No such image: %s", name) +func (srv *Server) pruneImage(img *Image, repo, tag string) error { + return nil +} + +var ErrImageReferenced = errors.New("Image referenced by a repository") + +func (srv *Server) deleteImageChildren(id string, byId map[string][]string, byParents map[string][]*Image) error { + // If the image is referenced by a repo, do not delete + if len(byId[id]) != 0 { + return ErrImageReferenced } + // If the image is not referenced and has no children, remove it + if len(byParents[id]) == 0 { + return srv.runtime.graph.Delete(id) + } + + // If the image is not referenced but has children, go recursive + referenced := false + for _, img := range byParents[id] { + if err := srv.deleteImageChildren(img.Id, byId, byParents); err != nil { + if err != ErrImageReferenced { + return err + } else { + referenced = true + } + } + } + if referenced { + return ErrImageReferenced + } + return nil +} + +func (srv *Server) deleteImageParents(img *Image, byId map[string][]string, byParents map[string][]*Image) error { + if img.Parent != "" { + parent, err := srv.runtime.graph.Get(img.Parent) + if err != nil { + return err + } + // Remove all children images + if err := srv.deleteImageChildren(img.Id, byId, byParents); err != nil { + return err + } + // If no error (no referenced children), then remove the parent + if err := srv.runtime.graph.Delete(img.Parent); err != nil { + return err + } + return srv.deleteImageParents(parent, byId, byParents) + } + return nil +} + +func (srv *Server) deleteImage(repoName, tag string) error { + img, err := srv.runtime.repositories.LookupImage(repoName + ":" + tag) + if err != nil { + return fmt.Errorf("No such image: %s:%s", repoName, tag) + } + utils.Debugf("Image %s referenced %d times", img.Id, len(srv.runtime.repositories.ById()[img.Id])) + if err := srv.runtime.repositories.Delete(repoName, tag, img.Id); err != nil { + return err + } + byId := srv.runtime.repositories.ById() + if len(byId[img.Id]) == 0 { + byParents, err := srv.runtime.graph.ByParent() + if err != nil { + return err + } + if err := srv.deleteImageChildren(img.Id, byId, byParents); err != nil { + if err != ErrImageReferenced { + return err + } + } else { + srv.deleteImageParents(img) + } + + } + return nil +} + +func (srv *Server) ImageDelete(name string) error { var tag string if strings.Contains(name, ":") { nameParts := strings.Split(name, ":") @@ -716,28 +792,10 @@ func (srv *Server) ImageDelete(name string) error { tag = nameParts[1] } + srv.deleteImage(name, tag) + // if the images is referenced several times - utils.Debugf("Image %s referenced %d times", img.Id, len(srv.runtime.repositories.ById()[img.Id])) - if len(srv.runtime.repositories.ById()[img.Id]) > 1 { - // if it's repo:tag, try to delete the tag (docker rmi base:latest) - if tag != "" { - if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { - return err - } - return nil - } - // Let's say you have the same image referenced by base and base2 - // check if the image is referenced in another repo (docker rmi base) - for _, repoTag := range srv.runtime.repositories.ById()[img.Id] { - if !strings.HasPrefix(repoTag, name+":") { - // if found in another repo (base2) delete the repo base, otherwise delete the whole image - if err := srv.runtime.repositories.Delete(name, "", img.Id); err != nil { - return err - } - return nil - } - } - } + // check is the image to delete isn't parent of another image byParent, _ := srv.runtime.graph.ByParent() if childs, exists := byParent[img.Id]; exists { From 5aa95b667c5986e3cea45b93bb2370cd46deeea3 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Thu, 30 May 2013 22:53:45 +0000 Subject: [PATCH 11/12] WIP needs to fix HTTP error codes --- server.go | 106 +++++++++++++++++++----------------------------------- tags.go | 22 +++++++++++- 2 files changed, 58 insertions(+), 70 deletions(-) diff --git a/server.go b/server.go index 126ac7d6e9..2af1e3cd8f 100644 --- a/server.go +++ b/server.go @@ -705,26 +705,22 @@ func (srv *Server) ContainerDestroy(name string, removeVolume bool) error { return nil } -func (srv *Server) pruneImage(img *Image, repo, tag string) error { - return nil -} - var ErrImageReferenced = errors.New("Image referenced by a repository") -func (srv *Server) deleteImageChildren(id string, byId map[string][]string, byParents map[string][]*Image) error { +func (srv *Server) deleteImageAndChildren(id string) error { // If the image is referenced by a repo, do not delete - if len(byId[id]) != 0 { + if len(srv.runtime.repositories.ById()[id]) != 0 { return ErrImageReferenced } - // If the image is not referenced and has no children, remove it - if len(byParents[id]) == 0 { - return srv.runtime.graph.Delete(id) - } // 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.deleteImageChildren(img.Id, byId, byParents); err != nil { + if err := srv.deleteImageAndChildren(img.Id); err != nil { if err != ErrImageReferenced { return err } else { @@ -735,56 +731,61 @@ func (srv *Server) deleteImageChildren(id string, byId map[string][]string, byPa 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 + } + return srv.runtime.graph.Delete(id) + } return nil } -func (srv *Server) deleteImageParents(img *Image, byId map[string][]string, byParents map[string][]*Image) error { +func (srv *Server) deleteImageParents(img *Image) error { if img.Parent != "" { parent, err := srv.runtime.graph.Get(img.Parent) if err != nil { return err } // Remove all children images - if err := srv.deleteImageChildren(img.Id, byId, byParents); err != nil { + if err := srv.deleteImageAndChildren(img.Parent); err != nil { return err } - // If no error (no referenced children), then remove the parent - if err := srv.runtime.graph.Delete(img.Parent); err != nil { - return err - } - return srv.deleteImageParents(parent, byId, byParents) + return srv.deleteImageParents(parent) } return nil } -func (srv *Server) deleteImage(repoName, tag string) error { - img, err := srv.runtime.repositories.LookupImage(repoName + ":" + tag) - if err != nil { - return fmt.Errorf("No such image: %s:%s", repoName, tag) - } - utils.Debugf("Image %s referenced %d times", img.Id, len(srv.runtime.repositories.ById()[img.Id])) - if err := srv.runtime.repositories.Delete(repoName, tag, img.Id); err != nil { +func (srv *Server) deleteImage(img *Image, repoName, tag string) error { + //Untag the current image + if err := srv.runtime.repositories.Delete(repoName, tag); err != nil { return err } - byId := srv.runtime.repositories.ById() - if len(byId[img.Id]) == 0 { - byParents, err := srv.runtime.graph.ByParent() - if err != nil { - return err - } - if err := srv.deleteImageChildren(img.Id, byId, byParents); err != nil { + if len(srv.runtime.repositories.ById()[img.Id]) == 0 { + if err := srv.deleteImageAndChildren(img.Id); err != nil { + if err != ErrImageReferenced { + return err + } + } else if err := srv.deleteImageParents(img); err != nil { if err != ErrImageReferenced { return err } - } else { - srv.deleteImageParents(img) } - } return nil } func (srv *Server) ImageDelete(name string) error { + img, err := srv.runtime.repositories.LookupImage(name) + if err != nil { + return fmt.Errorf("No such image: %s", name) + } + var tag string if strings.Contains(name, ":") { nameParts := strings.Split(name, ":") @@ -792,40 +793,7 @@ func (srv *Server) ImageDelete(name string) error { tag = nameParts[1] } - srv.deleteImage(name, tag) - - // if the images is referenced several times - - // check is the image to delete isn't parent of another image - byParent, _ := srv.runtime.graph.ByParent() - if childs, exists := byParent[img.Id]; exists { - if strings.Contains(img.Id, name) { - return fmt.Errorf("Conflict with %s, %s was not removed", childs[0].ShortId(), name) - } - if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { - return err - } - return nil - } - parents, _ := img.History() - for _, parent := range parents { - byParent, _ = srv.runtime.graph.ByParent() - //stop if image has children - if _, exists := byParent[parent.Id]; exists { - break - } - //stop if image is tagged and it is not the first image we delete - if _, hasTags := srv.runtime.repositories.ById()[parent.Id]; hasTags && img.Id != parent.Id { - break - } - if err := srv.runtime.graph.Delete(parent.Id); err != nil { - return fmt.Errorf("Error deleting image %s: %s", name, err.Error()) - } - if err := srv.runtime.repositories.Delete(name, tag, img.Id); err != nil { - return err - } - } - return nil + return srv.deleteImage(img, name, tag) } func (srv *Server) ImageGetCached(imgId string, config *Config) (*Image, error) { diff --git a/tags.go b/tags.go index 4a54398c5f..1148203d3d 100644 --- a/tags.go +++ b/tags.go @@ -110,7 +110,27 @@ func (store *TagStore) ImageName(id string) string { return utils.TruncateId(id) } -func (store *TagStore) Delete(repoName, tag, imageName string) error { +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) error { if err := store.Reload(); err != nil { return err } From 9060b5c2f52d29dec1920eb89ee0d48341540e3b Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Fri, 31 May 2013 14:37:02 +0000 Subject: [PATCH 12/12] added proper returns type, move the auto-prune in v1.1 api --- api.go | 17 +++++++-- api_params.go | 5 +++ api_test.go | 11 ++++-- commands.go | 18 ++++++++-- docs/sources/api/docker_remote_api.rst | 16 ++++++++- server.go | 48 +++++++++++++++++--------- server_test.go | 4 +-- tags.go | 15 ++++---- 8 files changed, 102 insertions(+), 32 deletions(-) diff --git a/api.go b/api.go index 1759257e6e..586fcf7a64 100644 --- a/api.go +++ b/api.go @@ -450,10 +450,23 @@ func deleteImages(srv *Server, version float64, w http.ResponseWriter, r *http.R 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 } diff --git a/api_params.go b/api_params.go index 1a24ab2875..6f759f2d80 100644 --- a/api_params.go +++ b/api_params.go @@ -23,6 +23,11 @@ type ApiInfo struct { NGoroutines int `json:",omitempty"` } +type ApiRmi struct { + Deleted string `json:",omitempty"` + Untagged string `json:",omitempty"` +} + type ApiContainers struct { Id string Image string diff --git a/api_test.go b/api_test.go index 844d15cc13..61d0bcb5ff 100644 --- a/api_test.go +++ b/api_test.go @@ -1271,10 +1271,17 @@ func TestDeleteImages(t *testing.T) { if err := deleteImages(srv, API_VERSION, r, req, map[string]string{"name": "test:test"}); err != nil { t.Fatal(err) } - if r.Code != http.StatusNoContent { - t.Fatalf("%d NO CONTENT expected, received %d\n", http.StatusNoContent, r.Code) + 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) diff --git a/commands.go b/commands.go index 0289048b89..42dc1b06af 100644 --- a/commands.go +++ b/commands.go @@ -567,10 +567,22 @@ func (cli *DockerCli) CmdRmi(args ...string) error { } for _, name := range cmd.Args() { - if _, _, err := cli.call("DELETE", "/images/"+name, nil); err != nil { - fmt.Fprintf(os.Stderr, "%s\n", err) + body, _, err := cli.call("DELETE", "/images/"+name, nil) + if err != nil { + 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 diff --git a/docs/sources/api/docker_remote_api.rst b/docs/sources/api/docker_remote_api.rst index b941367632..60285fc792 100644 --- a/docs/sources/api/docker_remote_api.rst +++ b/docs/sources/api/docker_remote_api.rst @@ -750,13 +750,27 @@ 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 diff --git a/server.go b/server.go index 2af1e3cd8f..3d2cfaa500 100644 --- a/server.go +++ b/server.go @@ -707,7 +707,7 @@ func (srv *Server) ContainerDestroy(name string, removeVolume bool) error { var ErrImageReferenced = errors.New("Image referenced by a repository") -func (srv *Server) deleteImageAndChildren(id string) error { +func (srv *Server) deleteImageAndChildren(id string, imgs *[]ApiRmi) error { // If the image is referenced by a repo, do not delete if len(srv.runtime.repositories.ById()[id]) != 0 { return ErrImageReferenced @@ -720,7 +720,7 @@ func (srv *Server) deleteImageAndChildren(id string) error { return err } for _, img := range byParents[id] { - if err := srv.deleteImageAndChildren(img.Id); err != nil { + if err := srv.deleteImageAndChildren(img.Id, imgs); err != nil { if err != ErrImageReferenced { return err } else { @@ -741,49 +741,65 @@ func (srv *Server) deleteImageAndChildren(id string) error { if err := srv.runtime.repositories.DeleteAll(id); err != nil { return err } - return srv.runtime.graph.Delete(id) + 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) error { +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); err != nil { + if err := srv.deleteImageAndChildren(img.Parent, imgs); err != nil { return err } - return srv.deleteImageParents(parent) + return srv.deleteImageParents(parent, imgs) } return nil } -func (srv *Server) deleteImage(img *Image, repoName, tag string) error { +func (srv *Server) deleteImage(img *Image, repoName, tag string) (*[]ApiRmi, error) { //Untag the current image - if err := srv.runtime.repositories.Delete(repoName, tag); err != nil { - return err + 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); err != nil { + if err := srv.deleteImageAndChildren(img.Id, &imgs); err != nil { if err != ErrImageReferenced { - return err + return &imgs, err } - } else if err := srv.deleteImageParents(img); err != nil { + } else if err := srv.deleteImageParents(img, &imgs); err != nil { if err != ErrImageReferenced { - return err + return &imgs, err } } } - return nil + return &imgs, nil } -func (srv *Server) ImageDelete(name string) error { +func (srv *Server) ImageDelete(name string, autoPrune bool) (*[]ApiRmi, error) { img, err := srv.runtime.repositories.LookupImage(name) if err != nil { - return fmt.Errorf("No such image: %s", name) + 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 diff --git a/server_test.go b/server_test.go index 541c927b83..52b3479263 100644 --- a/server_test.go +++ b/server_test.go @@ -29,7 +29,7 @@ func TestContainerTagImageDelete(t *testing.T) { t.Errorf("Excepted 3 images, %d found", len(images)) } - if err := srv.ImageDelete("utest/docker:tag2"); err != nil { + if _, err := srv.ImageDelete("utest/docker:tag2", true); err != nil { t.Fatal(err) } @@ -42,7 +42,7 @@ func TestContainerTagImageDelete(t *testing.T) { t.Errorf("Excepted 2 images, %d found", len(images)) } - if err := srv.ImageDelete("utest:tag1"); err != nil { + if _, err := srv.ImageDelete("utest:tag1", true); err != nil { t.Fatal(err) } diff --git a/tags.go b/tags.go index 1148203d3d..630727aa60 100644 --- a/tags.go +++ b/tags.go @@ -118,11 +118,11 @@ func (store *TagStore) DeleteAll(id string) error { for _, name := range names { if strings.Contains(name, ":") { nameParts := strings.Split(name, ":") - if err := store.Delete(nameParts[0], nameParts[1]); err != nil { + if _, err := store.Delete(nameParts[0], nameParts[1]); err != nil { return err } } else { - if err := store.Delete(name, ""); err != nil { + if _, err := store.Delete(name, ""); err != nil { return err } } @@ -130,9 +130,10 @@ func (store *TagStore) DeleteAll(id string) error { return nil } -func (store *TagStore) Delete(repoName, tag string) error { +func (store *TagStore) Delete(repoName, tag string) (bool, error) { + deleted := false if err := store.Reload(); err != nil { - return err + return false, err } if r, exists := store.Repositories[repoName]; exists { if tag != "" { @@ -141,16 +142,18 @@ func (store *TagStore) Delete(repoName, tag string) error { if len(r) == 0 { delete(store.Repositories, repoName) } + deleted = true } else { - return fmt.Errorf("No such tag: %s:%s", repoName, tag) + 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 store.Save() + return deleted, store.Save() } func (store *TagStore) Set(repoName, tag, imageName string, force bool) error {