Merge pull request #5747 from fuweid/cp-15-5275

estesp · web-flow · commit 47d0f52cbd14 · 2021-07-20T10:30:48.000-04:00
[release/1.5] Try next mirror in case of non-404 errors, too
diff --git a/remotes/docker/resolver.go b/remotes/docker/resolver.go
@@ -229,10 +229,10 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
 	}
 
 	var (
-		lastErr error
-		paths   [][]string
-		dgst    = refspec.Digest()
-		caps    = HostCapabilityPull
+		firstErr error
+		paths    [][]string
+		dgst     = refspec.Digest()
+		caps     = HostCapabilityPull
 	)
 
 	if dgst != "" {
@@ -283,8 +283,8 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
 					err = errors.Wrapf(err, "pull access denied, repository does not exist or may require authorization")
 				}
 				// Store the error for referencing later
-				if lastErr == nil {
-					lastErr = err
+				if firstErr == nil {
+					firstErr = err
 				}
 				log.G(ctx).WithError(err).Info("trying next host")
 				continue // try another host
@@ -296,7 +296,14 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
 					log.G(ctx).Info("trying next host - response was http.StatusNotFound")
 					continue
 				}
-				return "", ocispec.Descriptor{}, errors.Errorf("unexpected status code %v: %v", u, resp.Status)
+				if resp.StatusCode > 399 {
+					// Set firstErr when encountering the first non-404 status code.
+					if firstErr == nil {
+						firstErr = errors.Errorf("pulling from host %s failed with status code %v: %v", host.Host, u, resp.Status)
+					}
+					continue // try another host
+				}
+				return "", ocispec.Descriptor{}, errors.Errorf("pulling from host %s failed with unexpected status code %v: %v", host.Host, u, resp.Status)
 			}
 			size := resp.ContentLength
 			contentType := getManifestMediaType(resp)
@@ -359,8 +366,8 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
 			}
 			// Prevent resolving to excessively large manifests
 			if size > MaxManifestSize {
-				if lastErr == nil {
-					lastErr = errors.Wrapf(errdefs.ErrNotFound, "rejecting %d byte manifest for %s", size, ref)
+				if firstErr == nil {
+					firstErr = errors.Wrapf(errdefs.ErrNotFound, "rejecting %d byte manifest for %s", size, ref)
 				}
 				continue
 			}
@@ -376,11 +383,15 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
 		}
 	}
 
-	if lastErr == nil {
-		lastErr = errors.Wrap(errdefs.ErrNotFound, ref)
+	// If above loop terminates without return, then there was an error.
+	// "firstErr" contains the first non-404 error. That is, "firstErr == nil"
+	// means that either no registries were given or each registry returned 404.
+
+	if firstErr == nil {
+		firstErr = errors.Wrap(errdefs.ErrNotFound, ref)
 	}
 
-	return "", ocispec.Descriptor{}, lastErr
+	return "", ocispec.Descriptor{}, firstErr
 }
 
 func (r *dockerResolver) Fetcher(ctx context.Context, ref string) (remotes.Fetcher, error) {

Original file line number	Diff line number	Diff line change
`@@ -229,10 +229,10 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`229`	`229`	`}`
`230`	`230`
`231`	`231`	`var (`
`232`		`- lastErr error`
`233`		`- paths [][]string`
`234`		`- dgst = refspec.Digest()`
`235`		`- caps = HostCapabilityPull`
	`232`	`+ firstErr error`
	`233`	`+ paths [][]string`
	`234`	`+ dgst = refspec.Digest()`
	`235`	`+ caps = HostCapabilityPull`
`236`	`236`	`)`
`237`	`237`
`238`	`238`	`if dgst != "" {`
`@@ -283,8 +283,8 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`283`	`283`	`err = errors.Wrapf(err, "pull access denied, repository does not exist or may require authorization")`
`284`	`284`	`}`
`285`	`285`	`// Store the error for referencing later`
`286`		`- if lastErr == nil {`
`287`		`- lastErr = err`
	`286`	`+ if firstErr == nil {`
	`287`	`+ firstErr = err`
`288`	`288`	`}`
`289`	`289`	`log.G(ctx).WithError(err).Info("trying next host")`
`290`	`290`	`continue // try another host`
`@@ -296,7 +296,14 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`296`	`296`	`log.G(ctx).Info("trying next host - response was http.StatusNotFound")`
`297`	`297`	`continue`
`298`	`298`	`}`
`299`		`- return "", ocispec.Descriptor{}, errors.Errorf("unexpected status code %v: %v", u, resp.Status)`
	`299`	`+ if resp.StatusCode > 399 {`
	`300`	`+ // Set firstErr when encountering the first non-404 status code.`
	`301`	`+ if firstErr == nil {`
	`302`	`+ firstErr = errors.Errorf("pulling from host %s failed with status code %v: %v", host.Host, u, resp.Status)`
	`303`	`+ }`
	`304`	`+ continue // try another host`
	`305`	`+ }`
	`306`	`+ return "", ocispec.Descriptor{}, errors.Errorf("pulling from host %s failed with unexpected status code %v: %v", host.Host, u, resp.Status)`
`300`	`307`	`}`
`301`	`308`	`size := resp.ContentLength`
`302`	`309`	`contentType := getManifestMediaType(resp)`
`@@ -359,8 +366,8 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`359`	`366`	`}`
`360`	`367`	`// Prevent resolving to excessively large manifests`
`361`	`368`	`if size > MaxManifestSize {`
`362`		`- if lastErr == nil {`
`363`		`- lastErr = errors.Wrapf(errdefs.ErrNotFound, "rejecting %d byte manifest for %s", size, ref)`
	`369`	`+ if firstErr == nil {`
	`370`	`+ firstErr = errors.Wrapf(errdefs.ErrNotFound, "rejecting %d byte manifest for %s", size, ref)`
`364`	`371`	`}`
`365`	`372`	`continue`
`366`	`373`	`}`
`@@ -376,11 +383,15 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`376`	`383`	`}`
`377`	`384`	`}`
`378`	`385`
`379`		`- if lastErr == nil {`
`380`		`- lastErr = errors.Wrap(errdefs.ErrNotFound, ref)`
	`386`	`+ // If above loop terminates without return, then there was an error.`
	`387`	`+ // "firstErr" contains the first non-404 error. That is, "firstErr == nil"`
	`388`	`+ // means that either no registries were given or each registry returned 404.`
	`389`	`+`
	`390`	`+ if firstErr == nil {`
	`391`	`+ firstErr = errors.Wrap(errdefs.ErrNotFound, ref)`
`381`	`392`	`}`
`382`	`393`
`383`		`- return "", ocispec.Descriptor{}, lastErr`
	`394`	`+ return "", ocispec.Descriptor{}, firstErr`
`384`	`395`	`}`
`385`	`396`
`386`	`397`	`func (r *dockerResolver) Fetcher(ctx context.Context, ref string) (remotes.Fetcher, error) {`