diff --git a/graph/push.go b/graph/push.go index 92dcb5efc4..6e844aada3 100644 --- a/graph/push.go +++ b/graph/push.go @@ -29,12 +29,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, + layersSeen: make(map[string]bool), }, nil case registry.APIVersion1: return &v1Pusher{ diff --git a/graph/push_v2.go b/graph/push_v2.go index c90cf340d9..9f9a9ffef3 100644 --- a/graph/push_v2.go +++ b/graph/push_v2.go @@ -26,6 +26,11 @@ type v2Pusher struct { config *ImagePushConfig sf *streamformatter.StreamFormatter repo distribution.Repository + + // layersSeen 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. + layersSeen map[string]bool } func (p *v2Pusher) Push() (fallback bool, err error) { @@ -86,8 +91,6 @@ func (p *v2Pusher) pushV2Tag(tag string) error { return fmt.Errorf("tag does not exist: %s", tag) } - layersSeen := make(map[string]bool) - layer, err := p.graph.Get(layerId) if err != nil { return err @@ -116,7 +119,7 @@ func (p *v2Pusher) pushV2Tag(tag string) error { return err } - if layersSeen[layer.ID] { + if p.layersSeen[layer.ID] { break } @@ -171,7 +174,7 @@ func (p *v2Pusher) pushV2Tag(tag string) error { m.FSLayers = append(m.FSLayers, manifest.FSLayer{BlobSum: dgst}) m.History = append(m.History, manifest.History{V1Compatibility: string(jsonData)}) - layersSeen[layer.ID] = true + p.layersSeen[layer.ID] = true } logrus.Infof("Signed manifest for %s:%s using daemon's key: %s", p.repo.Name(), tag, p.trustKey.KeyID()) diff --git a/integration-cli/docker_cli_push_test.go b/integration-cli/docker_cli_push_test.go index d1d9804282..0cc9b0a6fa 100644 --- a/integration-cli/docker_cli_push_test.go +++ b/integration-cli/docker_cli_push_test.go @@ -60,7 +60,32 @@ func (s *DockerRegistrySuite) TestPushMultipleTags(c *check.C) { dockerCmd(c, "tag", "busybox", repoTag2) - dockerCmd(c, "push", repoName) + out, _ := dockerCmd(c, "push", repoName) + + // There should be no duplicate hashes in the output + imageSuccessfullyPushed := ": Image successfully pushed" + imageAlreadyExists := ": Image already exists" + imagePushHashes := make(map[string]struct{}) + outputLines := strings.Split(out, "\n") + for _, outputLine := range outputLines { + if strings.Contains(outputLine, imageSuccessfullyPushed) { + hash := strings.TrimSuffix(outputLine, imageSuccessfullyPushed) + if _, present := imagePushHashes[hash]; present { + c.Fatalf("Duplicate image push: %s", outputLine) + } + imagePushHashes[hash] = struct{}{} + } else if strings.Contains(outputLine, imageAlreadyExists) { + hash := strings.TrimSuffix(outputLine, imageAlreadyExists) + if _, present := imagePushHashes[hash]; present { + c.Fatalf("Duplicate image push: %s", outputLine) + } + imagePushHashes[hash] = struct{}{} + } + } + + if len(imagePushHashes) == 0 { + c.Fatal(`Expected at least one line containing "Image successfully pushed"`) + } } func (s *DockerRegistrySuite) TestPushInterrupt(c *check.C) { @@ -79,8 +104,7 @@ func (s *DockerRegistrySuite) TestPushInterrupt(c *check.C) { c.Fatalf("Failed to kill push process: %v", err) } if out, _, err := dockerCmdWithError(c, "push", repoName); err == nil { - str := string(out) - if !strings.Contains(str, "already in progress") { + if !strings.Contains(out, "already in progress") { c.Fatalf("Push should be continued on daemon side, but seems ok: %v, %s", err, out) } }