Add retries for temporary network issues

pkg/internal/retry/retry.go

@@ -0,0 +1,68 @@
+// 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 returns true if err implements Temporary() and it returns true.
+func IsTemporary(err error) bool {
+	if te, ok := err.(temporary); ok && te.Temporary() {
+		return true
+	}
+	return false
+}
+
+// IsNotNil returns true if err is not nil.
+func IsNotNil(err error) bool {
+	return err != nil
+}
+
+// Predicate determines whether an error should be retried.
+type Predicate func(error) (retry bool)
+
+// 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, p Predicate, backoff wait.Backoff) (err error) {
+	if f == nil {
+		return fmt.Errorf("nil f passed to retry")
+	}
+	if p == nil {
+		return fmt.Errorf("nil p passed to retry")
+	}
+
+	condition := func() (bool, error) {
+		err = f()
+		if p(err) {
+			return false, nil
+		}
+		return true, err
+	}
+
+	wait.ExponentialBackoff(backoff, condition)
+	return
+}

pkg/internal/retry/retry_test.go

@@ -0,0 +1,87 @@
+// 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, test.predicate, backoff)
+
+		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 TestNil(t *testing.T) {
+	if err := Retry(nil, nil, wait.Backoff{}); err == nil {
+		t.Errorf("got nil when passing in nil f")
+	}
+	if err := Retry(func() error { return nil }, nil, wait.Backoff{}); err == nil {
+		t.Errorf("got nil when passing in nil p")
+	}
+}

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
+}
+
+// WithRetryBackoff sets the backoff for retry operations.
+func WithRetryBackoff(backoff wait.Backoff) Option {
+	return func(o *options) {
+		o.backoff = backoff
+	}
+}
+
+// WithRetryPredicate sets the predicate for retry operations.
+func WithRetryPredicate(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, t.predicate, t.backoff)
+	return
+}