Define a new remote.DefaultTransport. (#1165)

* Revert "Give the ping context a timeout. (#1163)"

This reverts commit 080751a.

* Define a new `remote.DefaultTransport`.

Previously we used `http.DefaultTransport` (on which this is based),
but this uses a default dial timeout of 30s, which when we (by default)
wrap things in 5x retries can lead to ~150s delays simply pinging
an http-based registry.

If folks encounter issues with this via the library they can restore
the current behavior with:
```go
remote.WithTransport(http.DefaultTransport)
```

If folks encounter issues with this via `crane` they can restore the
current behavior with:
```
--dial-timeout 30s
```

* Switch to an impossible domain name

* Back out the crane flag

cmd/crane/cmd/root.go

@@ -22,6 +22,7 @@ import (
 	"github.com/docker/cli/cli/config"
 	"github.com/google/go-containerregistry/pkg/crane"
 	"github.com/google/go-containerregistry/pkg/logs"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/spf13/cobra"
 )
 
@@ -69,7 +70,7 @@ func New(use, short string, options []crane.Option) *cobra.Command {
 
 			options = append(options, crane.WithPlatform(platform.platform))
 
-			transport := http.DefaultTransport.(*http.Transport).Clone()
+			transport := remote.DefaultTransport.Clone()
 			transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: insecure}
 
 			var rt http.RoundTripper = transport

pkg/gcrane/copy_test.go

@@ -129,8 +129,8 @@ func newFakeXCR(stuff map[name.Reference]partial.Describable, t *testing.T) (*fa
 
 func TestCopy(t *testing.T) {
 	logs.Warn.SetOutput(os.Stderr)
-	src := "xcr.io/test/gcrane"
-	dst := "xcr.io/test/gcrane/copy"
+	src := "registry.example.com/test/gcrane"
+	dst := "registry.example.com/test/gcrane/copy"
 
 	oneTag, err := random.Image(1024, 5)
 	if err != nil {
@@ -168,48 +168,49 @@ func TestCopy(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	s, err := ggcrtest.NewTLSServer("xcr.io", h)
+	s, err := ggcrtest.NewTLSServer("registry.example.com", h)
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer s.Close()
 
-	// Make sure we don't actually talk to XCR.
-	http.DefaultTransport = s.Client().Transport
+	// Route requests to our test registry.
+	opt := remote.WithTransport(s.Client().Transport)
+	copt := WithTransport(s.Client().Transport)
 
-	if err := remote.Write(latestRef, twoTags); err != nil {
+	if err := remote.Write(latestRef, twoTags, opt); err != nil {
 		t.Fatal(err)
 	}
-	if err := remote.Write(fooRef, twoTags); err != nil {
+	if err := remote.Write(fooRef, twoTags, opt); err != nil {
 		t.Fatal(err)
 	}
-	if err := remote.Write(oneTagRef, oneTag); err != nil {
+	if err := remote.Write(oneTagRef, oneTag, opt); err != nil {
 		t.Fatal(err)
 	}
-	if err := remote.Write(noTagsRef, noTags); err != nil {
+	if err := remote.Write(noTagsRef, noTags, opt); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := Copy(src, dst); err != nil {
+	if err := Copy(src, dst, copt); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := CopyRepository(context.Background(), src, dst); err != nil {
+	if err := CopyRepository(context.Background(), src, dst, copt); err != nil {
 		t.Fatal(err)
 	}
 }
 
 func TestRename(t *testing.T) {
 	c := copier{
-		srcRepo: name.MustParseReference("xcr.io/foo").Context(),
-		dstRepo: name.MustParseReference("xcr.io/bar").Context(),
+		srcRepo: name.MustParseReference("registry.example.com/foo").Context(),
+		dstRepo: name.MustParseReference("registry.example.com/bar").Context(),
 	}
 
-	got, err := c.rename(name.MustParseReference("xcr.io/foo/sub/repo").Context())
+	got, err := c.rename(name.MustParseReference("registry.example.com/foo/sub/repo").Context())
 	if err != nil {
 		t.Fatalf("unexpected err: %v", err)
 	}
-	want := name.MustParseReference("xcr.io/bar/sub/repo").Context()
+	want := name.MustParseReference("registry.example.com/bar/sub/repo").Context()
 
 	if want.String() != got.String() {
 		t.Errorf("%s != %s", want, got)

pkg/v1/remote/options.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"net"
 	"net/http"
 	"syscall"
 	"time"
@@ -81,10 +82,28 @@ const (
 	defaultPageSize = 1000
 )
 
+// DefaultTransport is based on http.DefaultTransport with modifications
+// documented inline below.
+var DefaultTransport = &http.Transport{
+	Proxy: http.ProxyFromEnvironment,
+	DialContext: (&net.Dialer{
+		// By default we wrap the transport in retries, so reduce the
+		// default dial timeout to 5s to avoid 5x 30s of connection
+		// timeouts when doing the "ping" on certain http registries.
+		Timeout:   5 * time.Second,
+		KeepAlive: 30 * time.Second,
+	}).DialContext,
+	ForceAttemptHTTP2:     true,
+	MaxIdleConns:          100,
+	IdleConnTimeout:       90 * time.Second,
+	TLSHandshakeTimeout:   10 * time.Second,
+	ExpectContinueTimeout: 1 * time.Second,
+}
+
 func makeOptions(target authn.Resource, opts ...Option) (*options, error) {
 	o := &options{
 		auth:           authn.Anonymous,
-		transport:      http.DefaultTransport,
+		transport:      DefaultTransport,
 		platform:       defaultPlatform,
 		context:        context.Background(),
 		jobs:           defaultJobs,
@@ -134,7 +153,7 @@ func makeOptions(target authn.Resource, opts ...Option) (*options, error) {
 // If transport.Wrapper is provided, this signals that the consumer does *not* want any further wrapping to occur.
 // i.e. logging, retry and useragent
 //
-// The default transport its http.DefaultTransport.
+// The default transport is DefaultTransport.
 func WithTransport(t http.RoundTripper) Option {
 	return func(o *options) error {
 		o.transport = t

pkg/v1/remote/transport/ping.go

@@ -22,7 +22,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"strings"
-	"time"
 
 	authchallenge "github.com/docker/distribution/registry/client/auth/challenge"
 	"github.com/google/go-containerregistry/pkg/name"
@@ -87,12 +86,7 @@ func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingRes
 		if err != nil {
 			return nil, err
 		}
-		// The ping handler should be extremely fast, but for registries that serve
-		// over http, it could take a while to fallback from https (esp. if the
-		// transport has retries).  So give each ping attempt a (generous) 5s timeout.
-		pctx, cancel := context.WithTimeout(ctx, 5*time.Second)
-		defer cancel()
-		resp, err := client.Do(req.WithContext(pctx))
+		resp, err := client.Do(req.WithContext(ctx))
 		if err != nil {
 			errs = append(errs, err.Error())
 			// Potentially retry with http.