diff --git a/daemon/daemonbuilder/builder.go b/daemon/daemonbuilder/builder.go index b8ff702119..6786401631 100644 --- a/daemon/daemonbuilder/builder.go +++ b/daemon/daemonbuilder/builder.go @@ -193,7 +193,7 @@ func (d Docker) Copy(c *daemon.Container, destPath string, src builder.FileInfo, // GetCachedImage returns a reference to a cached image whose parent equals `parent` // and runconfig equals `cfg`. A cache miss is expected to return an empty ID and a nil error. func (d Docker) GetCachedImage(imgID string, cfg *runconfig.Config) (string, error) { - cache, err := d.Daemon.ImageGetCached(string(imgID), cfg) + cache, err := d.Daemon.ImageGetCached(imgID, cfg) if cache == nil || err != nil { return "", err } diff --git a/daemon/image_delete.go b/daemon/image_delete.go index efe6362812..070ec0b1d4 100644 --- a/daemon/image_delete.go +++ b/daemon/image_delete.go @@ -279,7 +279,7 @@ func (daemon *Daemon) checkImageDeleteHardConflict(img *image.Image) *imageDelet } // Check if the image has any descendent images. - if daemon.Graph().HasChildren(img) { + if daemon.Graph().HasChildren(img.ID) { return &imageDeleteConflict{ hard: true, imgID: img.ID, @@ -337,5 +337,5 @@ func (daemon *Daemon) checkImageDeleteSoftConflict(img *image.Image) *imageDelet // that there are no repository references to the given image and it has no // child images. func (daemon *Daemon) imageIsDangling(img *image.Image) bool { - return !(daemon.repositories.HasReferences(img) || daemon.Graph().HasChildren(img)) + return !(daemon.Repositories().HasReferences(img) || daemon.Graph().HasChildren(img.ID)) } diff --git a/graph/graph.go b/graph/graph.go index cbdfeee1a1..7ae1807608 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -104,6 +104,7 @@ type Graph struct { tarSplitDisabled bool uidMaps []idtools.IDMap gidMaps []idtools.IDMap + parentRefs map[string]int } // file names for ./graph// @@ -141,12 +142,13 @@ func NewGraph(root string, driver graphdriver.Driver, uidMaps, gidMaps []idtools } graph := &Graph{ - root: abspath, - idIndex: truncindex.NewTruncIndex([]string{}), - driver: driver, - retained: &retainedLayers{layerHolders: make(map[string]map[string]struct{})}, - uidMaps: uidMaps, - gidMaps: gidMaps, + root: abspath, + idIndex: truncindex.NewTruncIndex([]string{}), + driver: driver, + retained: &retainedLayers{layerHolders: make(map[string]map[string]struct{})}, + uidMaps: uidMaps, + gidMaps: gidMaps, + parentRefs: make(map[string]int), } // Windows does not currently support tarsplit functionality. @@ -174,6 +176,20 @@ func (graph *Graph) restore() error { for _, v := range dir { id := v.Name() if graph.driver.Exists(id) { + pth := filepath.Join(graph.root, id, "json") + jsonSource, err := os.Open(pth) + if err != nil { + return err + } + defer jsonSource.Close() + decoder := json.NewDecoder(jsonSource) + var img *image.Image + if err := decoder.Decode(img); err != nil { + return err + } + graph.imageMutex.Lock(img.Parent) + graph.parentRefs[img.Parent]++ + graph.imageMutex.Unlock(img.Parent) ids = append(ids, id) } } @@ -326,7 +342,13 @@ func (graph *Graph) register(im image.Descriptor, layerData io.Reader) (err erro if err := os.Rename(tmp, graph.imageRoot(imgID)); err != nil { return err } - graph.idIndex.Add(imgID) + + graph.idIndex.Add(img.ID) + + graph.imageMutex.Lock(img.Parent) + graph.parentRefs[img.Parent]++ + graph.imageMutex.Unlock(img.Parent) + return nil } @@ -393,6 +415,10 @@ func (graph *Graph) Delete(name string) error { if err != nil { return err } + img, err := graph.Get(id) + if err != nil { + return err + } tmp, err := graph.mktemp() graph.idIndex.Delete(id) if err == nil { @@ -407,6 +433,14 @@ func (graph *Graph) Delete(name string) error { } // Remove rootfs data from the driver graph.driver.Remove(id) + + graph.imageMutex.Lock(img.Parent) + graph.parentRefs[img.Parent]-- + if graph.parentRefs[img.Parent] == 0 { + delete(graph.parentRefs, img.Parent) + } + graph.imageMutex.Unlock(img.Parent) + // Remove the trashed image directory return os.RemoveAll(tmp) } @@ -424,9 +458,11 @@ func (graph *Graph) Map() map[string]*image.Image { // The walking order is undetermined. func (graph *Graph) walkAll(handler func(*image.Image)) { graph.idIndex.Iterate(func(id string) { - if img, err := graph.Get(id); err != nil { + img, err := graph.Get(id) + if err != nil { return - } else if handler != nil { + } + if handler != nil { handler(img) } }) @@ -453,8 +489,11 @@ func (graph *Graph) ByParent() map[string][]*image.Image { } // HasChildren returns whether the given image has any child images. -func (graph *Graph) HasChildren(img *image.Image) bool { - return len(graph.ByParent()[img.ID]) > 0 +func (graph *Graph) HasChildren(imgID string) bool { + graph.imageMutex.Lock(imgID) + count := graph.parentRefs[imgID] + graph.imageMutex.Unlock(imgID) + return count > 0 } // Retain keeps the images and layers that are in the pulling chain so that @@ -472,11 +511,10 @@ func (graph *Graph) Release(sessionID string, layerIDs ...string) { // A head is an image which is not the parent of another image in the graph. func (graph *Graph) Heads() map[string]*image.Image { heads := make(map[string]*image.Image) - byParent := graph.ByParent() graph.walkAll(func(image *image.Image) { // If it's not in the byParent lookup table, then // it's not a parent -> so it's a head! - if _, exists := byParent[image.ID]; !exists { + if !graph.HasChildren(image.ID) { heads[image.ID] = image } }) diff --git a/integration-cli/docker_cli_images_test.go b/integration-cli/docker_cli_images_test.go index 3f639a5a16..f85c84ee83 100644 --- a/integration-cli/docker_cli_images_test.go +++ b/integration-cli/docker_cli_images_test.go @@ -51,7 +51,6 @@ func (s *DockerSuite) TestImagesEnsureImageWithBadTagIsNotListed(c *check.C) { if strings.Contains(out, "busybox") { c.Fatal("images should not have listed busybox") } - } func (s *DockerSuite) TestImagesOrderedByCreationDate(c *check.C) { @@ -204,3 +203,42 @@ func (s *DockerSuite) TestImagesWithIncorrectFilter(c *check.C) { c.Assert(err, check.NotNil) c.Assert(out, checker.Contains, "Invalid filter") } + +func (s *DockerSuite) TestImagesEnsureOnlyHeadsImagesShown(c *check.C) { + testRequires(c, DaemonIsLinux) + + dockerfile := ` + FROM scratch + MAINTAINER docker + ENV foo bar` + + head, out, err := buildImageWithOut("scratch-image", dockerfile, false) + c.Assert(err, check.IsNil) + + split := strings.Split(out, "\n") + intermediate := strings.TrimSpace(split[5][7:]) + + out, _ = dockerCmd(c, "images") + if strings.Contains(out, intermediate) { + c.Fatalf("images shouldn't show non-heads images, got %s in %s", intermediate, out) + } + if !strings.Contains(out, head[:12]) { + c.Fatalf("images should contain final built images, want %s in out, got %s", head[:12], out) + } +} + +func (s *DockerSuite) TestImagesEnsureImagesFromScratchShown(c *check.C) { + testRequires(c, DaemonIsLinux) + + dockerfile := ` + FROM scratch + MAINTAINER docker` + + id, _, err := buildImageWithOut("scratch-image", dockerfile, false) + c.Assert(err, check.IsNil) + + out, _ := dockerCmd(c, "images") + if !strings.Contains(out, id[:12]) { + c.Fatalf("images should contain images built from scratch (e.g. %s), got %s", id[:12], out) + } +} diff --git a/integration-cli/docker_cli_rmi_test.go b/integration-cli/docker_cli_rmi_test.go index 8a60ec7fe1..19914315ee 100644 --- a/integration-cli/docker_cli_rmi_test.go +++ b/integration-cli/docker_cli_rmi_test.go @@ -284,3 +284,15 @@ RUN echo 2 #layer2 // should be allowed to untag with the -f flag c.Assert(out, checker.Contains, fmt.Sprintf("Untagged: %s:latest", newTag)) } + +func (*DockerSuite) TestRmiParentImageFail(c *check.C) { + testRequires(c, DaemonIsLinux) + + parent, err := inspectField("busybox", "Parent") + c.Assert(err, check.IsNil) + out, _, err := dockerCmdWithError("rmi", parent) + c.Assert(err, check.NotNil) + if !strings.Contains(out, "image has dependent child images") { + c.Fatalf("rmi should have failed because it's a parent image, got %s", out) + } +}