diff --git a/components/engine/daemon/image_delete.go b/components/engine/daemon/image_delete.go index 7c6329a669..c1af38bf7f 100644 --- a/components/engine/daemon/image_delete.go +++ b/components/engine/daemon/image_delete.go @@ -104,27 +104,34 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I repoRefs = daemon.referenceStore.References(imgID) - // If this is a tag reference and all the remaining references - // to this image are digest references, delete the remaining - // references so that they don't prevent removal of the image. + // If a tag reference was removed and the only remaining + // references to the same repository are digest references, + // then clean up those digest references. if _, isCanonical := parsedRef.(reference.Canonical); !isCanonical { - foundTagRef := false + foundRepoTagRef := false for _, repoRef := range repoRefs { - if _, repoRefIsCanonical := repoRef.(reference.Canonical); !repoRefIsCanonical { - foundTagRef = true + if _, repoRefIsCanonical := repoRef.(reference.Canonical); !repoRefIsCanonical && parsedRef.Name() == repoRef.Name() { + foundRepoTagRef = true break } } - if !foundTagRef { + if !foundRepoTagRef { + // Remove canonical references from same repository + remainingRefs := []reference.Named{} for _, repoRef := range repoRefs { - if _, err := daemon.removeImageRef(repoRef); err != nil { - return records, err - } + if _, repoRefIsCanonical := repoRef.(reference.Canonical); repoRefIsCanonical && parsedRef.Name() == repoRef.Name() { + if _, err := daemon.removeImageRef(repoRef); err != nil { + return records, err + } - untaggedRecord := types.ImageDelete{Untagged: repoRef.String()} - records = append(records, untaggedRecord) + untaggedRecord := types.ImageDelete{Untagged: repoRef.String()} + records = append(records, untaggedRecord) + } else { + remainingRefs = append(remainingRefs, repoRef) + + } } - repoRefs = []reference.Named{} + repoRefs = remainingRefs } } @@ -135,11 +142,10 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I removedRepositoryRef = true } else { - // If an ID reference was given AND there is exactly one - // repository reference to the image then we will want to - // remove that reference. - // FIXME: Is this the behavior we want? - if len(repoRefs) == 1 { + // If an ID reference was given AND there is at most one tag + // reference to the image AND all references are within one + // repository, then remove all references. + if isSingleReference(repoRefs) { c := conflictHard if !force { c |= conflictSoft &^ conflictActiveReference @@ -148,21 +154,48 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I return nil, conflict } - parsedRef, err := daemon.removeImageRef(repoRefs[0]) - if err != nil { - return nil, err + for _, repoRef := range repoRefs { + parsedRef, err := daemon.removeImageRef(repoRef) + if err != nil { + return nil, err + } + + untaggedRecord := types.ImageDelete{Untagged: parsedRef.String()} + + daemon.LogImageEvent(imgID.String(), imgID.String(), "untag") + records = append(records, untaggedRecord) } - - untaggedRecord := types.ImageDelete{Untagged: parsedRef.String()} - - daemon.LogImageEvent(imgID.String(), imgID.String(), "untag") - records = append(records, untaggedRecord) } } return records, daemon.imageDeleteHelper(imgID, &records, force, prune, removedRepositoryRef) } +// isSingleReference returns true when all references are from one repository +// and there is at most one tag. Returns false for empty input. +func isSingleReference(repoRefs []reference.Named) bool { + if len(repoRefs) <= 1 { + return len(repoRefs) == 1 + } + var singleRef reference.Named + canonicalRefs := map[string]struct{}{} + for _, repoRef := range repoRefs { + if _, isCanonical := repoRef.(reference.Canonical); isCanonical { + canonicalRefs[repoRef.Name()] = struct{}{} + } else if singleRef == nil { + singleRef = repoRef + } else { + return false + } + } + if singleRef == nil { + // Just use first canonical ref + singleRef = repoRefs[0] + } + _, ok := canonicalRefs[singleRef.Name()] + return len(canonicalRefs) == 1 && ok +} + // isImageIDPrefix returns whether the given possiblePrefix is a prefix of the // given imageID. func isImageIDPrefix(imageID, possiblePrefix string) bool { diff --git a/components/engine/integration-cli/docker_cli_by_digest_test.go b/components/engine/integration-cli/docker_cli_by_digest_test.go index 2f71d0f103..b89c76cce3 100644 --- a/components/engine/integration-cli/docker_cli_by_digest_test.go +++ b/components/engine/integration-cli/docker_cli_by_digest_test.go @@ -380,9 +380,14 @@ func (s *DockerRegistrySuite) TestDeleteImageByIDOnlyPulledByDigest(c *check.C) dockerCmd(c, "pull", imageReference) // just in case... + dockerCmd(c, "tag", imageReference, repoName+":sometag") + imageID := inspectField(c, imageReference, "Id") dockerCmd(c, "rmi", imageID) + + _, err = inspectFieldWithError(imageID, "Id") + c.Assert(err, checker.NotNil, check.Commentf("image should have been deleted")) } func (s *DockerRegistrySuite) TestDeleteImageWithDigestAndTag(c *check.C) { @@ -412,6 +417,40 @@ func (s *DockerRegistrySuite) TestDeleteImageWithDigestAndTag(c *check.C) { c.Assert(err, checker.NotNil, check.Commentf("image should have been deleted")) } +func (s *DockerRegistrySuite) TestDeleteImageWithDigestAndMultiRepoTag(c *check.C) { + pushDigest, err := setupImage(c) + c.Assert(err, checker.IsNil, check.Commentf("error setting up image")) + + repo2 := fmt.Sprintf("%s/%s", repoName, "repo2") + + // pull from the registry using the @ reference + imageReference := fmt.Sprintf("%s@%s", repoName, pushDigest) + dockerCmd(c, "pull", imageReference) + + imageID := inspectField(c, imageReference, "Id") + + repoTag := repoName + ":sometag" + repoTag2 := repo2 + ":othertag" + dockerCmd(c, "tag", imageReference, repoTag) + dockerCmd(c, "tag", imageReference, repoTag2) + + dockerCmd(c, "rmi", repoTag) + + // rmi should have deleted repoTag and image reference, but left repoTag2 + inspectField(c, repoTag2, "Id") + _, err = inspectFieldWithError(imageReference, "Id") + c.Assert(err, checker.NotNil, check.Commentf("image digest reference should have been removed")) + + _, err = inspectFieldWithError(repoTag, "Id") + c.Assert(err, checker.NotNil, check.Commentf("image tag reference should have been removed")) + + dockerCmd(c, "rmi", repoTag2) + + // rmi should have deleted the tag, the digest reference, and the image itself + _, err = inspectFieldWithError(imageID, "Id") + c.Assert(err, checker.NotNil, check.Commentf("image should have been deleted")) +} + // TestPullFailsWithAlteredManifest tests that a `docker pull` fails when // we have modified a manifest blob and its digest cannot be verified. // This is the schema2 version of the test.