Merge pull request containerd#6244 from fuweid/cherry-pick-4531

dmcgowan · web-flow · commit 8acff254ffcc · 2021-11-15T17:24:29.000-08:00
[release/1.4] Try next mirror in case of non-404 errors, too
diff --git a/remotes/docker/resolver.go b/remotes/docker/resolver.go
@@ -238,10 +238,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 != "" {
@@ -292,8 +292,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
@@ -305,7 +305,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)
@@ -368,8 +375,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
 			}
@@ -385,11 +392,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
`@@ -238,10 +238,10 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`238`	`238`	`}`
`239`	`239`
`240`	`240`	`var (`
`241`		`- lastErr error`
`242`		`- paths [][]string`
`243`		`- dgst = refspec.Digest()`
`244`		`- caps = HostCapabilityPull`
	`241`	`+ firstErr error`
	`242`	`+ paths [][]string`
	`243`	`+ dgst = refspec.Digest()`
	`244`	`+ caps = HostCapabilityPull`
`245`	`245`	`)`
`246`	`246`
`247`	`247`	`if dgst != "" {`
`@@ -292,8 +292,8 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`292`	`292`	`err = errors.Wrapf(err, "pull access denied, repository does not exist or may require authorization")`
`293`	`293`	`}`
`294`	`294`	`// Store the error for referencing later`
`295`		`- if lastErr == nil {`
`296`		`- lastErr = err`
	`295`	`+ if firstErr == nil {`
	`296`	`+ firstErr = err`
`297`	`297`	`}`
`298`	`298`	`log.G(ctx).WithError(err).Info("trying next host")`
`299`	`299`	`continue // try another host`
`@@ -305,7 +305,14 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`305`	`305`	`log.G(ctx).Info("trying next host - response was http.StatusNotFound")`
`306`	`306`	`continue`
`307`	`307`	`}`
`308`		`- return "", ocispec.Descriptor{}, errors.Errorf("unexpected status code %v: %v", u, resp.Status)`
	`308`	`+ if resp.StatusCode > 399 {`
	`309`	`+ // Set firstErr when encountering the first non-404 status code.`
	`310`	`+ if firstErr == nil {`
	`311`	`+ firstErr = errors.Errorf("pulling from host %s failed with status code %v: %v", host.Host, u, resp.Status)`
	`312`	`+ }`
	`313`	`+ continue // try another host`
	`314`	`+ }`
	`315`	`+ return "", ocispec.Descriptor{}, errors.Errorf("pulling from host %s failed with unexpected status code %v: %v", host.Host, u, resp.Status)`
`309`	`316`	`}`
`310`	`317`	`size := resp.ContentLength`
`311`	`318`	`contentType := getManifestMediaType(resp)`
`@@ -368,8 +375,8 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`368`	`375`	`}`
`369`	`376`	`// Prevent resolving to excessively large manifests`
`370`	`377`	`if size > MaxManifestSize {`
`371`		`- if lastErr == nil {`
`372`		`- lastErr = errors.Wrapf(errdefs.ErrNotFound, "rejecting %d byte manifest for %s", size, ref)`
	`378`	`+ if firstErr == nil {`
	`379`	`+ firstErr = errors.Wrapf(errdefs.ErrNotFound, "rejecting %d byte manifest for %s", size, ref)`
`373`	`380`	`}`
`374`	`381`	`continue`
`375`	`382`	`}`
`@@ -385,11 +392,15 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp`
`385`	`392`	`}`
`386`	`393`	`}`
`387`	`394`
`388`		`- if lastErr == nil {`
`389`		`- lastErr = errors.Wrap(errdefs.ErrNotFound, ref)`
	`395`	`+ // If above loop terminates without return, then there was an error.`
	`396`	`+ // "firstErr" contains the first non-404 error. That is, "firstErr == nil"`
	`397`	`+ // means that either no registries were given or each registry returned 404.`
	`398`	`+`
	`399`	`+ if firstErr == nil {`
	`400`	`+ firstErr = errors.Wrap(errdefs.ErrNotFound, ref)`
`390`	`401`	`}`
`391`	`402`
`392`		`- return "", ocispec.Descriptor{}, lastErr`
	`403`	`+ return "", ocispec.Descriptor{}, firstErr`
`393`	`404`	`}`
`394`	`405`
`395`	`406`	`func (r *dockerResolver) Fetcher(ctx context.Context, ref string) (remotes.Fetcher, error) {`