Merge pull request #3868 from awprice/issue-3850

Improve host fallback behaviour in docker remote

remotes/docker/fetcher.go

@@ -95,41 +95,49 @@ func (r dockerFetcher) Fetch(ctx context.Context, desc ocispec.Descriptor) (io.R
 			images.MediaTypeDockerSchema1Manifest,
 			ocispec.MediaTypeImageManifest, ocispec.MediaTypeImageIndex:
 
+			var firstErr error
 			for _, host := range r.hosts {
 				req := r.request(host, http.MethodGet, "manifests", desc.Digest.String())
 
 				rc, err := r.open(ctx, req, desc.MediaType, offset)
 				if err != nil {
-					if errdefs.IsNotFound(err) {
-						continue // try another host
+					// Store the error for referencing later
+					if firstErr == nil {
+						firstErr = err
 					}
-
-					return nil, err
+					continue // try another host
 				}
 
 				return rc, nil
 			}
+
+			return nil, firstErr
 		}
 
 		// Finally use blobs endpoints
+		var firstErr error
 		for _, host := range r.hosts {
 			req := r.request(host, http.MethodGet, "blobs", desc.Digest.String())
 
 			rc, err := r.open(ctx, req, desc.MediaType, offset)
 			if err != nil {
-				if errdefs.IsNotFound(err) {
-					continue // try another host
+				// Store the error for referencing later
+				if firstErr == nil {
+					firstErr = err
 				}
-
-				return nil, err
+				continue // try another host
 			}
 
 			return rc, nil
 		}
 
-		return nil, errors.Wrapf(errdefs.ErrNotFound,
-			"could not fetch content descriptor %v (%v) from remote",
-			desc.Digest, desc.MediaType)
+		if errdefs.IsNotFound(firstErr) {
+			firstErr = errors.Wrapf(errdefs.ErrNotFound,
+				"could not fetch content descriptor %v (%v) from remote",
+				desc.Digest, desc.MediaType)
+		}
+
+		return nil, firstErr
 
 	})
 }

remotes/docker/resolver.go

@@ -286,7 +286,11 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
 				if errors.Cause(err) == ErrInvalidAuthorization {
 					err = errors.Wrapf(err, "pull access denied, repository does not exist or may require authorization")
 				}
-				return "", ocispec.Descriptor{}, err
+				// Store the error for referencing later
+				if lastErr == nil {
+					lastErr = err
+				}
+				continue // try another host
 			}
 			resp.Body.Close() // don't care about body contents.
 

remotes/docker/resolver_test.go

@@ -29,6 +29,7 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/containerd/containerd/remotes"
 	digest "github.com/opencontainers/go-digest"
@@ -192,6 +193,93 @@ func TestBadTokenResolver(t *testing.T) {
 	}
 }
 
+func TestHostFailureFallbackResolver(t *testing.T) {
+	sf := func(h http.Handler) (string, ResolverOptions, func()) {
+		s := httptest.NewServer(h)
+		base := s.URL[7:] // strip "http://"
+
+		options := ResolverOptions{}
+		createHost := func(host string) RegistryHost {
+			return RegistryHost{
+				Client: &http.Client{
+					// Set the timeout so we timeout waiting for the non-responsive HTTP server
+					Timeout: 500 * time.Millisecond,
+				},
+				Host:         host,
+				Scheme:       "http",
+				Path:         "/v2",
+				Capabilities: HostCapabilityPull | HostCapabilityResolve | HostCapabilityPush,
+			}
+		}
+
+		// Create an unstarted HTTP server. We use this to generate a random port.
+		notRunning := httptest.NewUnstartedServer(nil)
+		notRunningBase := notRunning.Listener.Addr().String()
+
+		// Override hosts with two hosts
+		options.Hosts = func(host string) ([]RegistryHost, error) {
+			return []RegistryHost{
+				createHost(notRunningBase), // This host IS running, but with a non-responsive HTTP server
+				createHost(base),           // This host IS running
+			}, nil
+		}
+
+		return base, options, s.Close
+	}
+
+	runBasicTest(t, "testname", sf)
+}
+
+func TestHostTLSFailureFallbackResolver(t *testing.T) {
+	sf := func(h http.Handler) (string, ResolverOptions, func()) {
+		// Start up two servers
+		server := httptest.NewServer(h)
+		httpBase := server.URL[7:] // strip "http://"
+
+		tlsServer := httptest.NewUnstartedServer(h)
+		tlsServer.StartTLS()
+		httpsBase := tlsServer.URL[8:] // strip "https://"
+
+		capool := x509.NewCertPool()
+		cert, _ := x509.ParseCertificate(tlsServer.TLS.Certificates[0].Certificate[0])
+		capool.AddCert(cert)
+
+		client := &http.Client{
+			Transport: &http.Transport{
+				TLSClientConfig: &tls.Config{
+					RootCAs: capool,
+				},
+			},
+		}
+
+		options := ResolverOptions{}
+		createHost := func(host string) RegistryHost {
+			return RegistryHost{
+				Client:       client,
+				Host:         host,
+				Scheme:       "https",
+				Path:         "/v2",
+				Capabilities: HostCapabilityPull | HostCapabilityResolve | HostCapabilityPush,
+			}
+		}
+
+		// Override hosts with two hosts
+		options.Hosts = func(host string) ([]RegistryHost, error) {
+			return []RegistryHost{
+				createHost(httpBase),  // This host is serving plain HTTP
+				createHost(httpsBase), // This host is serving TLS
+			}, nil
+		}
+
+		return httpBase, options, func() {
+			server.Close()
+			tlsServer.Close()
+		}
+	}
+
+	runBasicTest(t, "testname", sf)
+}
+
 func withTokenServer(th http.Handler, creds func(string) (string, string, error)) func(h http.Handler) (string, ResolverOptions, func()) {
 	return func(h http.Handler) (string, ResolverOptions, func()) {
 		s := httptest.NewUnstartedServer(th)