From a0d9ffd6f70fdbbc392b31fe1a230bec3ce58df0 Mon Sep 17 00:00:00 2001
From: Derek McGowan <derek@mcgstyle.net>
Date: Wed, 12 Aug 2015 16:02:12 -0700
Subject: [PATCH] Fix layer exclusion bug on multiple tag push

Ensure that layers are not excluded from manifests based on previous pushes.
Continue skipping pushes on layers which were pushed by a previous tag.

Update push multiple tag tests.
Ensure that each tag pushed exists on the registry and is pullable.
Add output comparison on multiple tag push check.

fixes #15536

Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
---
 graph/push.go    | 14 ++++++++------
 graph/push_v2.go | 17 +++++++++++++++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/graph/push.go b/graph/push.go
index 6a6bdfec2b..ae5802dcb9 100644
--- a/graph/push.go
+++ b/graph/push.go
@@ -5,6 +5,7 @@ import (
 	"io"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/docker/distribution/digest"
 	"github.com/docker/docker/cliconfig"
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/docker/docker/registry"
@@ -44,12 +45,13 @@ func (s *TagStore) NewPusher(endpoint registry.APIEndpoint, localRepo Repository
 	switch endpoint.Version {
 	case registry.APIVersion2:
 		return &v2Pusher{
-			TagStore:  s,
-			endpoint:  endpoint,
-			localRepo: localRepo,
-			repoInfo:  repoInfo,
-			config:    imagePushConfig,
-			sf:        sf,
+			TagStore:     s,
+			endpoint:     endpoint,
+			localRepo:    localRepo,
+			repoInfo:     repoInfo,
+			config:       imagePushConfig,
+			sf:           sf,
+			layersPushed: make(map[digest.Digest]bool),
 		}, nil
 	case registry.APIVersion1:
 		return &v1Pusher{
diff --git a/graph/push_v2.go b/graph/push_v2.go
index 7d5ca44d96..53f6d223fa 100644
--- a/graph/push_v2.go
+++ b/graph/push_v2.go
@@ -27,6 +27,11 @@ type v2Pusher struct {
 	config    *ImagePushConfig
 	sf        *streamformatter.StreamFormatter
 	repo      distribution.Repository
+
+	// layersPushed is the set of layers known to exist on the remote side.
+	// This avoids redundant queries when pushing multiple tags that
+	// involve the same layers.
+	layersPushed map[digest.Digest]bool
 }
 
 func (p *v2Pusher) Push() (fallback bool, err error) {
@@ -117,6 +122,10 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
 			return err
 		}
 
+		// break early if layer has already been seen in this image,
+		// this prevents infinite loops on layers which loopback, this
+		// cannot be prevented since layer IDs are not merkle hashes
+		// TODO(dmcgowan): throw error if no valid use case is found
 		if layersSeen[layer.ID] {
 			break
 		}
@@ -138,6 +147,13 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
 		dgst, err := p.graph.GetDigest(layer.ID)
 		switch err {
 		case nil:
+			if p.layersPushed[dgst] {
+				exists = true
+				// break out of switch, it is already known that
+				// the push is not needed and therefore doing a
+				// stat is unnecessary
+				break
+			}
 			_, err := p.repo.Blobs(context.Background()).Stat(context.Background(), dgst)
 			switch err {
 			case nil:
@@ -173,6 +189,7 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
 		m.History = append(m.History, manifest.History{V1Compatibility: string(jsonData)})
 
 		layersSeen[layer.ID] = true
+		p.layersPushed[dgst] = true
 	}
 
 	logrus.Infof("Signed manifest for %s:%s using daemon's key: %s", p.repo.Name(), tag, p.trustKey.KeyID())