Merge pull request #20580 from BrianBland/crossRepoPushRetry

Improve auth fallback behavior for cross-repository push

distribution/push_v2.go

@@ -274,27 +274,29 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
 	// then push the blob.
 	bs := pd.repo.Blobs(ctx)
 
-	var mountFrom metadata.V2Metadata
+	var layerUpload distribution.BlobWriter
+	mountAttemptsRemaining := 3
 
-	// Attempt to find another repository in the same registry to mount the layer from to avoid an unnecessary upload
-	for _, metadata := range v2Metadata {
-		sourceRepo, err := reference.ParseNamed(metadata.SourceRepository)
+	// Attempt to find another repository in the same registry to mount the layer
+	// from to avoid an unnecessary upload.
+	// Note: metadata is stored from oldest to newest, so we iterate through this
+	// slice in reverse to maximize our chances of the blob still existing in the
+	// remote repository.
+	for i := len(v2Metadata) - 1; i >= 0 && mountAttemptsRemaining > 0; i-- {
+		mountFrom := v2Metadata[i]
+
+		sourceRepo, err := reference.ParseNamed(mountFrom.SourceRepository)
 		if err != nil {
 			continue
 		}
-		if pd.repoInfo.Hostname() == sourceRepo.Hostname() {
-			logrus.Debugf("attempting to mount layer %s (%s) from %s", diffID, metadata.Digest, sourceRepo.FullName())
-			mountFrom = metadata
-			break
+		if pd.repoInfo.Hostname() != sourceRepo.Hostname() {
+			// don't mount blobs from another registry
+			continue
 		}
-	}
 
-	var createOpts []distribution.BlobCreateOption
-
-	if mountFrom.SourceRepository != "" {
 		namedRef, err := reference.WithName(mountFrom.SourceRepository)
 		if err != nil {
-			return err
+			continue
 		}
 
 		// TODO (brianbland): We need to construct a reference where the Name is
@@ -302,45 +304,49 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.
 		// richer reference package
 		remoteRef, err := distreference.WithName(namedRef.RemoteName())
 		if err != nil {
-			return err
+			continue
 		}
 
 		canonicalRef, err := distreference.WithDigest(remoteRef, mountFrom.Digest)
 		if err != nil {
-			return err
+			continue
 		}
 
-		createOpts = append(createOpts, client.WithMountFrom(canonicalRef))
-	}
+		logrus.Debugf("attempting to mount layer %s (%s) from %s", diffID, mountFrom.Digest, sourceRepo.FullName())
 
-	// Send the layer
-	layerUpload, err := bs.Create(ctx, createOpts...)
-	switch err := err.(type) {
-	case distribution.ErrBlobMounted:
-		progress.Updatef(progressOutput, pd.ID(), "Mounted from %s", err.From.Name())
+		layerUpload, err = bs.Create(ctx, client.WithMountFrom(canonicalRef))
+		switch err := err.(type) {
+		case distribution.ErrBlobMounted:
+			progress.Updatef(progressOutput, pd.ID(), "Mounted from %s", err.From.Name())
 
-		err.Descriptor.MediaType = schema2.MediaTypeLayer
+			err.Descriptor.MediaType = schema2.MediaTypeLayer
 
-		pd.pushState.Lock()
-		pd.pushState.confirmedV2 = true
-		pd.pushState.remoteLayers[diffID] = err.Descriptor
-		pd.pushState.Unlock()
+			pd.pushState.Lock()
+			pd.pushState.confirmedV2 = true
+			pd.pushState.remoteLayers[diffID] = err.Descriptor
+			pd.pushState.Unlock()
 
-		// Cache mapping from this layer's DiffID to the blobsum
-		if err := pd.v2MetadataService.Add(diffID, metadata.V2Metadata{Digest: mountFrom.Digest, SourceRepository: pd.repoInfo.FullName()}); err != nil {
-			return xfer.DoNotRetry{Err: err}
+			// Cache mapping from this layer's DiffID to the blobsum
+			if err := pd.v2MetadataService.Add(diffID, metadata.V2Metadata{Digest: mountFrom.Digest, SourceRepository: pd.repoInfo.FullName()}); err != nil {
+				return xfer.DoNotRetry{Err: err}
+			}
+			return nil
+		case nil:
+			// blob upload session created successfully, so begin the upload
+			mountAttemptsRemaining = 0
+		default:
+			// unable to mount layer from this repository, so this source mapping is no longer valid
+			logrus.Debugf("unassociating layer %s (%s) with %s", diffID, mountFrom.Digest, mountFrom.SourceRepository)
+			pd.v2MetadataService.Remove(mountFrom)
+			mountAttemptsRemaining--
 		}
-
-		return nil
-	}
-	if mountFrom.SourceRepository != "" {
-		// unable to mount layer from this repository, so this source mapping is no longer valid
-		logrus.Debugf("unassociating layer %s (%s) with %s", diffID, mountFrom.Digest, mountFrom.SourceRepository)
-		pd.v2MetadataService.Remove(mountFrom)
 	}
 
-	if err != nil {
-		return retryOnError(err)
+	if layerUpload == nil {
+		layerUpload, err = bs.Create(ctx)
+		if err != nil {
+			return retryOnError(err)
+		}
 	}
 	defer layerUpload.Close()
 

integration-cli/docker_cli_push_test.go

@@ -201,7 +201,7 @@ func (s *DockerSchema1RegistrySuite) TestCrossRepositoryLayerPushNotSupported(c
 	out2, _, err := dockerCmdWithError("push", destRepoName)
 	c.Assert(err, check.IsNil, check.Commentf("pushing the image to the private registry has failed: %s", out2))
 	// schema1 registry should not support cross-repo layer mounts, so ensure that this does not happen
-	c.Assert(strings.Contains(out2, "Mounted from dockercli/busybox"), check.Equals, false)
+	c.Assert(strings.Contains(out2, "Mounted from"), check.Equals, false)
 
 	digest2 := digest.DigestRegexp.FindString(out2)
 	c.Assert(len(digest2), checker.GreaterThan, 0, check.Commentf("no digest found for pushed manifest"))