Merge pull request #23996 from dmcgowan/always-store-image-digest-update

[carry 23503]  Always store the image digest when pulling and pushing an image.

daemon/images.go

@@ -167,7 +167,7 @@ func (daemon *Daemon) Images(filterArgs, filter string, all bool) ([]*types.Imag
 			} else {
 				continue
 			}
-		} else if danglingOnly {
+		} else if danglingOnly && len(newImage.RepoTags) > 0 {
 			continue
 		}
 

distribution/pull.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/docker/distribution/digest"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/distribution/metadata"
 	"github.com/docker/docker/distribution/xfer"
@@ -203,3 +204,22 @@ func ValidateRepoName(name string) error {
 	}
 	return nil
 }
+
+func addDigestReference(store reference.Store, ref reference.Named, dgst digest.Digest, imageID image.ID) error {
+	dgstRef, err := reference.WithDigest(ref, dgst)
+	if err != nil {
+		return err
+	}
+
+	if oldTagImageID, err := store.Get(dgstRef); err == nil {
+		if oldTagImageID != imageID {
+			// Updating digests not supported by reference store
+			logrus.Errorf("Image ID for digest %s changed from %s to %s, cannot update", dgst.String(), oldTagImageID, imageID)
+		}
+		return nil
+	} else if err != reference.ErrDoesNotExist {
+		return err
+	}
+
+	return store.AddDigest(dgstRef, imageID, true)
+}

distribution/pull_v2.go

@@ -393,7 +393,7 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat
 	oldTagImageID, err := p.config.ReferenceStore.Get(ref)
 	if err == nil {
 		if oldTagImageID == imageID {
-			return false, nil
+			return false, addDigestReference(p.config.ReferenceStore, ref, manifestDigest, imageID)
 		}
 	} else if err != reference.ErrDoesNotExist {
 		return false, err
@@ -403,10 +403,14 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat
 		if err = p.config.ReferenceStore.AddDigest(canonical, imageID, true); err != nil {
 			return false, err
 		}
-	} else if err = p.config.ReferenceStore.AddTag(ref, imageID, true); err != nil {
-		return false, err
+	} else {
+		if err = addDigestReference(p.config.ReferenceStore, ref, manifestDigest, imageID); err != nil {
+			return false, err
+		}
+		if err = p.config.ReferenceStore.AddTag(ref, imageID, true); err != nil {
+			return false, err
+		}
 	}
-
 	return true, nil
 }
 

distribution/push_v2.go

@@ -200,6 +200,11 @@ func (p *v2Pusher) pushV2Tag(ctx context.Context, ref reference.NamedTagged, ima
 
 	manifestDigest := digest.FromBytes(canonicalManifest)
 	progress.Messagef(p.config.ProgressOutput, "", "%s: digest: %s size: %d", ref.Tag(), manifestDigest, len(canonicalManifest))
+
+	if err := addDigestReference(p.config.ReferenceStore, ref, manifestDigest, imageID); err != nil {
+		return err
+	}
+
 	// Signal digest to the trust client so it can sign the
 	// push, if appropriate.
 	progress.Aux(p.config.ProgressOutput, PushResult{Tag: ref.Tag(), Digest: manifestDigest, Size: len(canonicalManifest)})

integration-cli/docker_cli_by_digest_test.go

@@ -314,6 +314,79 @@ func (s *DockerRegistrySuite) TestListImagesWithDigests(c *check.C) {
 	c.Assert(busyboxRe.MatchString(out), checker.True, check.Commentf("expected %q: %s", busyboxRe.String(), out))
 }
 
+func (s *DockerRegistrySuite) TestListDanglingImagesWithDigests(c *check.C) {
+	// setup image1
+	digest1, err := setupImageWithTag(c, "dangle1")
+	c.Assert(err, checker.IsNil, check.Commentf("error setting up image"))
+	imageReference1 := fmt.Sprintf("%s@%s", repoName, digest1)
+	c.Logf("imageReference1 = %s", imageReference1)
+
+	// pull image1 by digest
+	dockerCmd(c, "pull", imageReference1)
+
+	// list images
+	out, _ := dockerCmd(c, "images", "--digests")
+
+	// make sure repo shown, tag=<none>, digest = $digest1
+	re1 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest1.String() + `\s`)
+	c.Assert(re1.MatchString(out), checker.True, check.Commentf("expected %q: %s", re1.String(), out))
+	// setup image2
+	digest2, err := setupImageWithTag(c, "dangle2")
+	//error setting up image
+	c.Assert(err, checker.IsNil)
+	imageReference2 := fmt.Sprintf("%s@%s", repoName, digest2)
+	c.Logf("imageReference2 = %s", imageReference2)
+
+	// pull image1 by digest
+	dockerCmd(c, "pull", imageReference1)
+
+	// pull image2 by digest
+	dockerCmd(c, "pull", imageReference2)
+
+	// list images
+	out, _ = dockerCmd(c, "images", "--digests", "--filter=\"dangling=true\"")
+
+	// make sure repo shown, tag=<none>, digest = $digest1
+	c.Assert(re1.MatchString(out), checker.True, check.Commentf("expected %q: %s", re1.String(), out))
+
+	// make sure repo shown, tag=<none>, digest = $digest2
+	re2 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest2.String() + `\s`)
+	c.Assert(re2.MatchString(out), checker.True, check.Commentf("expected %q: %s", re2.String(), out))
+
+	// pull dangle1 tag
+	dockerCmd(c, "pull", repoName+":dangle1")
+
+	// list images
+	out, _ = dockerCmd(c, "images", "--digests", "--filter=\"dangling=true\"")
+
+	// make sure image 1 has repo, tag, <none> AND repo, <none>, digest
+	reWithDigest1 := regexp.MustCompile(`\s*` + repoName + `\s*dangle1\s*` + digest1.String() + `\s`)
+	c.Assert(reWithDigest1.MatchString(out), checker.False, check.Commentf("unexpected %q: %s", reWithDigest1.String(), out))
+	// make sure image 2 has repo, <none>, digest
+	c.Assert(re2.MatchString(out), checker.True, check.Commentf("expected %q: %s", re2.String(), out))
+
+	// pull dangle2 tag
+	dockerCmd(c, "pull", repoName+":dangle2")
+
+	// list images, show tagged images
+	out, _ = dockerCmd(c, "images", "--digests")
+
+	// make sure image 1 has repo, tag, digest
+	c.Assert(reWithDigest1.MatchString(out), checker.True, check.Commentf("expected %q: %s", reWithDigest1.String(), out))
+
+	// make sure image 2 has repo, tag, digest
+	reWithDigest2 := regexp.MustCompile(`\s*` + repoName + `\s*dangle2\s*` + digest2.String() + `\s`)
+	c.Assert(reWithDigest2.MatchString(out), checker.True, check.Commentf("expected %q: %s", reWithDigest2.String(), out))
+
+	// list images, no longer dangling, should not match
+	out, _ = dockerCmd(c, "images", "--digests", "--filter=\"dangling=true\"")
+
+	// make sure image 1 has repo, tag, digest
+	c.Assert(reWithDigest1.MatchString(out), checker.False, check.Commentf("unexpected %q: %s", reWithDigest1.String(), out))
+	// make sure image 2 has repo, tag, digest
+	c.Assert(reWithDigest2.MatchString(out), checker.False, check.Commentf("unexpected %q: %s", reWithDigest2.String(), out))
+}
+
 func (s *DockerRegistrySuite) TestInspectImageWithDigests(c *check.C) {
 	digest, err := setupImage(c)
 	c.Assert(err, check.IsNil, check.Commentf("error setting up image"))

integration-cli/docker_cli_push_test.go

@@ -287,7 +287,7 @@ func (s *DockerTrustSuite) TestTrustedPush(c *check.C) {
 	s.trustedCmd(pullCmd)
 	out, _, err = runCommandWithOutput(pullCmd)
 	c.Assert(err, check.IsNil, check.Commentf(out))
-	c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out))
+	c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out))
 
 	// Assert that we rotated the snapshot key to the server by checking our local keystore
 	contents, err := ioutil.ReadDir(filepath.Join(cliconfig.ConfigDir(), "trust/private/tuf_keys", privateRegistryURL, "dockerclitrusted/pushtest"))
@@ -312,7 +312,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithEnvPasswords(c *check.C) {
 	s.trustedCmd(pullCmd)
 	out, _, err = runCommandWithOutput(pullCmd)
 	c.Assert(err, check.IsNil, check.Commentf(out))
-	c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out))
+	c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out))
 }
 
 func (s *DockerTrustSuite) TestTrustedPushWithFailingServer(c *check.C) {
@@ -358,7 +358,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithExistingTag(c *check.C) {
 	s.trustedCmd(pullCmd)
 	out, _, err = runCommandWithOutput(pullCmd)
 	c.Assert(err, check.IsNil, check.Commentf(out))
-	c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out))
+	c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out))
 }
 
 func (s *DockerTrustSuite) TestTrustedPushWithExistingSignedTag(c *check.C) {
@@ -492,7 +492,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithReleasesDelegationOnly(c *check.C)
 	s.trustedCmd(pullCmd)
 	out, _, err = runCommandWithOutput(pullCmd)
 	c.Assert(err, check.IsNil, check.Commentf(out))
-	c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out))
+	c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out))
 }
 
 func (s *DockerTrustSuite) TestTrustedPushSignsAllFirstLevelRolesWeHaveKeysFor(c *check.C) {