From c995c9bb91a2bf5b2038330d063a073d2e0c611c Mon Sep 17 00:00:00 2001 From: Gabriel Monroy Date: Fri, 20 Dec 2013 16:52:34 -0700 Subject: [PATCH 1/3] add TestContainerOrphaning integration test --- integration/commands_test.go | 63 ++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/integration/commands_test.go b/integration/commands_test.go index af720f67e6..bcf79c1534 100644 --- a/integration/commands_test.go +++ b/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, os.Stdout, 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") + } + } + +} From 8dcca2125a2176924ea9d816214d88b8b6450bf2 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Fri, 20 Dec 2013 16:26:02 -0800 Subject: [PATCH 2/3] prevent orphan --- server.go | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/server.go b/server.go index 6a3ff2cd2d..a6ded6a996 100644 --- a/server.go +++ b/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 != "" { - validate = len(srv.runtime.repositories.ByID()[img.ID]) == 1 - } - - if validate { + if repository != "" && len(srv.runtime.repositories.ByID()[img.ID]) == 1 { // 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 - } + if err := srv.canDeleteImage(img.ID); 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 From 369cde4ad7b2618e24294839761b743c5c25911b Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Fri, 20 Dec 2013 16:50:31 -0800 Subject: [PATCH 3/3] discard test output --- integration/commands_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/commands_test.go b/integration/commands_test.go index bcf79c1534..84a9008224 100644 --- a/integration/commands_test.go +++ b/integration/commands_test.go @@ -979,7 +979,7 @@ func TestContainerOrphaning(t *testing.T) { defer os.RemoveAll(tmpDir) // setup a CLI and server - cli := docker.NewDockerCli(nil, os.Stdout, ioutil.Discard, testDaemonProto, testDaemonAddr) + cli := docker.NewDockerCli(nil, ioutil.Discard, ioutil.Discard, testDaemonProto, testDaemonAddr) defer cleanup(globalEngine, t) srv := mkServerFromEngine(globalEngine, t)