From 62213ee314d0669a278fae1e3052329f4ca35f75 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Mon, 16 Dec 2013 13:29:43 -0800 Subject: [PATCH] Allow untag operations with no container validation --- integration/server_test.go | 47 +++++++++++++++++++++++++++++ server.go | 61 +++++++++++++++++++++++++------------- 2 files changed, 87 insertions(+), 21 deletions(-) diff --git a/integration/server_test.go b/integration/server_test.go index 90752e23ef..2650311c36 100644 --- a/integration/server_test.go +++ b/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) + } +} diff --git a/server.go b/server.go index a0e6191090..9b492a2927 100644 --- a/server.go +++ b/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) + // 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 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 } - return nil - }); err != nil { - return nil, err } } - - if strings.Contains(img.ID, name) { - //delete via ID - return srv.deleteImage(img, "", "") - } - 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) {