Add retries for temporary network issues

We pretty commonly see this issue when communicating with a registry:
```
Get https://gcr.io/v2/: net/http: TLS handshake timeout
```

This causes a huge amount of pain if it's a failure within e.g. a large
CI job, since the entire thing would fail.

Fixes google#77

Author: jonjohnsonjr

pkg/internal/retry/options.go

@@ -0,0 +1,72 @@
+// Copyright 2019 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package retry
+
+import (
+	"time"
+
+	"k8s.io/apimachinery/pkg/util/wait"
+)
+
+var (
+	defaultBackoff = wait.Backoff{
+		Duration: 100 * time.Millisecond,
+		Steps:    2,
+	}
+
+	defaultPredicate = IsNotNil
+)
+
+// Predicate determines whether an error should be retried.
+type Predicate func(error) (retry bool)
+
+// Option is a functional option for retry operations.
+type Option func(*options)
+
+type options struct {
+	backoff   wait.Backoff
+	predicate Predicate
+}
+
+func makeOptions(opts ...Option) *options {
+	o := &options{
+		backoff:   defaultBackoff,
+		predicate: defaultPredicate,
+	}
+	for _, option := range opts {
+		option(o)
+	}
+	return o
+}
+
+// WithBackoff sets the backoff for retry operations.
+//
+// By default, just retry the operation once after waiting 100ms. This is not
+// perfect for most situations and should be overridden by using this option.
+func WithBackoff(backoff wait.Backoff) Option {
+	return func(o *options) {
+		o.backoff = backoff
+	}
+}
+
+// WithPredicate sets the predicate for retry operations.
+//
+// By default, just retry all errors. This is not perfect for most situations
+// and should be overridden by using this option.
+func WithPredicate(predicate Predicate) Option {
+	return func(o *options) {
+		o.predicate = predicate
+	}
+}

pkg/internal/retry/retry.go

@@ -0,0 +1,63 @@
+// Copyright 2019 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package retry provides methods for retrying operations. It is a thin wrapper
+// around k8s.io/apimachinery/pkg/util/wait to make certain operations easier.
+package retry
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/util/wait"
+)
+
+// This is implemented by several errors in the net package as well as our
+// transport.Error.
+type temporary interface {
+	Temporary() bool
+}
+
+// IsTemporary is true if err implements temporary and Temporary() returns true.
+func IsTemporary(err error) bool {
+	if te, ok := err.(temporary); ok && te.Temporary() {
+		return true
+	}
+	return false
+}
+
+func IsNotNil(err error) bool {
+	return err != nil
+}
+
+// Retry retries a given function, f, until a predicate is satisfied, using
+// exponential backoff. If the predicate is never satisfied, it will return the
+// last error returned by f.
+func Retry(f func() error, opts ...Option) (err error) {
+	if f == nil {
+		return fmt.Errorf("nil func passed to retry")
+	}
+
+	o := makeOptions(opts...)
+
+	condition := func() (bool, error) {
+		err = f()
+		if o.predicate(err) {
+			return false, nil
+		}
+		return true, err
+	}
+
+	wait.ExponentialBackoff(o.backoff, condition)
+	return
+}

pkg/internal/retry/retry_test.go

@@ -0,0 +1,84 @@
+// Copyright 2018 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package retry
+
+import (
+	"fmt"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/util/wait"
+)
+
+type temp struct{}
+
+func (e temp) Error() string {
+	return "temporary error"
+}
+
+func (e temp) Temporary() bool {
+	return true
+}
+
+func TestRetry(t *testing.T) {
+	for i, test := range []struct {
+		predicate   Predicate
+		err         error
+		shouldRetry bool
+	}{{
+		predicate:   IsTemporary,
+		err:         nil,
+		shouldRetry: false,
+	}, {
+		predicate:   IsTemporary,
+		err:         fmt.Errorf("not temporary"),
+		shouldRetry: false,
+	}, {
+		predicate:   IsNotNil,
+		err:         fmt.Errorf("not temporary"),
+		shouldRetry: true,
+	}, {
+		predicate:   IsTemporary,
+		err:         temp{},
+		shouldRetry: true,
+	}} {
+		// Make sure we retry 5 times if we shouldRetry.
+		steps := 5
+		backoff := wait.Backoff{
+			Steps: steps,
+		}
+
+		// Count how many times this function is invoked.
+		count := 0
+		f := func() error {
+			count++
+			return test.err
+		}
+
+		Retry(f, WithBackoff(backoff), WithPredicate(test.predicate))
+
+		if test.shouldRetry && count != steps {
+			t.Errorf("expected %d to retry %v, did not", i, test.err)
+		} else if !test.shouldRetry && count == steps {
+			t.Errorf("expected %d not to retry %v, but did", i, test.err)
+		}
+	}
+}
+
+// Make sure we don't panic.
+func TestNilF(t *testing.T) {
+	if err := Retry(nil); err == nil {
+		t.Errorf("got nil when passing in nil f")
+	}
+}

pkg/v1/google/list.go

@@ -51,6 +51,9 @@ func newLister(repo name.Repository, options ...ListerOption) (*lister, error) {
 		}
 	}
 
+	// Wrap the transport in something that can retry network flakes.
+	l.transport = transport.NewRetry(l.transport)
+
 	scopes := []string{repo.Scope(transport.PullScope)}
 	tr, err := transport.New(repo.Registry, l.auth, l.transport, scopes)
 	if err != nil {

pkg/v1/remote/options.go

@@ -21,6 +21,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
 )
 
 // Option is a functional option for remote operations.
@@ -57,6 +58,9 @@ func makeOptions(reg name.Registry, opts ...Option) (*options, error) {
 		o.auth = auth
 	}
 
+	// Wrap the transport in something that can retry network flakes.
+	o.transport = transport.NewRetry(o.transport)
+
 	return o, nil
 }
 

pkg/v1/remote/transport/error.go

@@ -48,8 +48,8 @@ func (e *Error) Error() string {
 	}
 }
 
-// ShouldRetry returns whether the request that preceded the error should be retried.
-func (e *Error) ShouldRetry() bool {
+// Temporary returns whether the request that preceded the error is temporary.
+func (e *Error) Temporary() bool {
 	if len(e.Errors) == 0 {
 		return false
 	}

pkg/v1/remote/transport/error_test.go

@@ -24,7 +24,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
-func TestShouldRetry(t *testing.T) {
+func TestTemporary(t *testing.T) {
 	tests := []struct {
 		error *Error
 		retry bool
@@ -50,10 +50,10 @@ func TestShouldRetry(t *testing.T) {
 	}}
 
 	for _, test := range tests {
-		retry := test.error.ShouldRetry()
+		retry := test.error.Temporary()
 
 		if test.retry != retry {
-			t.Errorf("ShouldRetry(%s) = %t, wanted %t", test.error, retry, test.retry)
+			t.Errorf("Temporary(%s) = %t, wanted %t", test.error, retry, test.retry)
 		}
 	}
 }

pkg/v1/remote/transport/retry.go

@@ -0,0 +1,89 @@
+// Copyright 2018 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package transport
+
+import (
+	"net/http"
+	"time"
+
+	"github.com/google/go-containerregistry/pkg/internal/retry"
+	"k8s.io/apimachinery/pkg/util/wait"
+)
+
+// Sleep for 0.1, 0.3, 0.9, 2.7 seconds. This should cover networking blips.
+var defaultBackoff = wait.Backoff{
+	Duration: 100 * time.Millisecond,
+	Factor:   3.0,
+	Jitter:   0.1,
+	Steps:    5,
+}
+
+var _ http.RoundTripper = (*retryTransport)(nil)
+
+// retryTransport wraps a RoundTripper and retries temporary network errors.
+type retryTransport struct {
+	inner     http.RoundTripper
+	backoff   wait.Backoff
+	predicate retry.Predicate
+}
+
+// Option is a functional option for retryTransport.
+type Option func(*options)
+
+type options struct {
+	backoff   wait.Backoff
+	predicate retry.Predicate
+}
+
+// WithBackoff sets the backoff for retry operations.
+func WithBackoff(backoff wait.Backoff) Option {
+	return func(o *options) {
+		o.backoff = backoff
+	}
+}
+
+// WithPredicate sets the predicate for retry operations.
+func WithPredicate(predicate func(error) bool) Option {
+	return func(o *options) {
+		o.predicate = predicate
+	}
+}
+
+// NewRetry returns a transport that retries errors.
+func NewRetry(inner http.RoundTripper, opts ...Option) http.RoundTripper {
+	o := &options{
+		backoff:   defaultBackoff,
+		predicate: retry.IsTemporary,
+	}
+
+	for _, opt := range opts {
+		opt(o)
+	}
+
+	return &retryTransport{
+		inner:     inner,
+		backoff:   o.backoff,
+		predicate: o.predicate,
+	}
+}
+
+func (t *retryTransport) RoundTrip(in *http.Request) (out *http.Response, err error) {
+	roundtrip := func() error {
+		out, err = t.inner.RoundTrip(in)
+		return err
+	}
+	retry.Retry(roundtrip, retry.WithBackoff(t.backoff), retry.WithPredicate(t.predicate))
+	return
+}