Browse code

Disambiguate mirror -> other endpoint fallbacks from V2 -> V1

Signed-off-by: Jake Sanders <jsand@google.com>

Jake Sanders authored on 2017/11/15 09:06:17
Showing 4 changed files
... ...
@@ -126,21 +126,25 @@ func TranslatePullError(err error, ref reference.Named) error {
126 126
 
127 127
 // continueOnError returns true if we should fallback to the next endpoint
128 128
 // as a result of this error.
129
-func continueOnError(err error) bool {
129
+func continueOnError(err error, mirrorEndpoint bool) bool {
130 130
 	switch v := err.(type) {
131 131
 	case errcode.Errors:
132 132
 		if len(v) == 0 {
133 133
 			return true
134 134
 		}
135
-		return continueOnError(v[0])
135
+		return continueOnError(v[0], mirrorEndpoint)
136 136
 	case ErrNoSupport:
137
-		return continueOnError(v.Err)
137
+		return continueOnError(v.Err, mirrorEndpoint)
138 138
 	case errcode.Error:
139
-		return shouldV2Fallback(v)
139
+		return mirrorEndpoint || shouldV2Fallback(v)
140 140
 	case *client.UnexpectedHTTPResponseError:
141 141
 		return true
142 142
 	case ImageConfigPullError:
143
-		return false
143
+		// ImageConfigPullError only happens with v2 images, v1 fallback is
144
+		// unnecessary.
145
+		// Failures from a mirror endpoint should result in fallback to the
146
+		// canonical repo.
147
+		return mirrorEndpoint
144 148
 	case error:
145 149
 		return !strings.Contains(err.Error(), strings.ToLower(syscall.ESRCH.Error()))
146 150
 	}
147 151
new file mode 100644
... ...
@@ -0,0 +1,85 @@
0
+package distribution
1
+
2
+import (
3
+	"errors"
4
+	"strings"
5
+	"syscall"
6
+	"testing"
7
+
8
+	"github.com/docker/distribution/registry/api/errcode"
9
+	"github.com/docker/distribution/registry/api/v2"
10
+	"github.com/docker/distribution/registry/client"
11
+)
12
+
13
+var always_continue = []error{
14
+	&client.UnexpectedHTTPResponseError{},
15
+
16
+	// Some errcode.Errors that don't disprove the existence of a V1 image
17
+	errcode.Error{Code: errcode.ErrorCodeUnauthorized},
18
+	errcode.Error{Code: v2.ErrorCodeManifestUnknown},
19
+	errcode.Error{Code: v2.ErrorCodeNameUnknown},
20
+
21
+	errors.New("some totally unexpected error"),
22
+}
23
+
24
+var continue_from_mirror_endpoint = []error{
25
+	ImageConfigPullError{},
26
+
27
+	// Some other errcode.Error that doesn't indicate we should search for a V1 image.
28
+	errcode.Error{Code: errcode.ErrorCodeTooManyRequests},
29
+}
30
+
31
+var never_continue = []error{
32
+	errors.New(strings.ToLower(syscall.ESRCH.Error())), // No such process
33
+}
34
+
35
+func TestContinueOnError_NonMirrorEndpoint(t *testing.T) {
36
+	for _, err := range always_continue {
37
+		if !continueOnError(err, false) {
38
+			t.Errorf("Should continue from non-mirror endpoint: %T: '%s'", err, err.Error())
39
+		}
40
+	}
41
+
42
+	for _, err := range continue_from_mirror_endpoint {
43
+		if continueOnError(err, false) {
44
+			t.Errorf("Should only continue from mirror endpoint: %T: '%s'", err, err.Error())
45
+		}
46
+	}
47
+}
48
+
49
+func TestContinueOnError_MirrorEndpoint(t *testing.T) {
50
+	errs := []error{}
51
+	errs = append(errs, always_continue...)
52
+	errs = append(errs, continue_from_mirror_endpoint...)
53
+	for _, err := range errs {
54
+		if !continueOnError(err, true) {
55
+			t.Errorf("Should continue from mirror endpoint: %T: '%s'", err, err.Error())
56
+		}
57
+	}
58
+}
59
+
60
+func TestContinueOnError_NeverContinue(t *testing.T) {
61
+	for _, is_mirror_endpoint := range []bool{true, false} {
62
+		for _, err := range never_continue {
63
+			if continueOnError(err, is_mirror_endpoint) {
64
+				t.Errorf("Should never continue: %T: '%s'", err, err.Error())
65
+			}
66
+		}
67
+	}
68
+}
69
+
70
+func TestContinueOnError_UnnestsErrors(t *testing.T) {
71
+	// ContinueOnError should evaluate nested errcode.Errors.
72
+
73
+	// Assumes that v2.ErrorCodeNameUnknown is a continueable error code.
74
+	err := errcode.Errors{errcode.Error{Code: v2.ErrorCodeNameUnknown}}
75
+	if !continueOnError(err, false) {
76
+		t.Fatal("ContinueOnError should unnest, base return value on errcode.Errors")
77
+	}
78
+
79
+	// Assumes that errcode.ErrorCodeTooManyRequests is not a V1-fallback indication
80
+	err = errcode.Errors{errcode.Error{Code: errcode.ErrorCodeTooManyRequests}}
81
+	if continueOnError(err, false) {
82
+		t.Fatal("ContinueOnError should unnest, base return value on errcode.Errors")
83
+	}
84
+}
... ...
@@ -74,7 +74,7 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, platform strin
74 74
 		if _, ok := err.(fallbackError); ok {
75 75
 			return err
76 76
 		}
77
-		if continueOnError(err) {
77
+		if continueOnError(err, p.endpoint.Mirror) {
78 78
 			return fallbackError{
79 79
 				err:         err,
80 80
 				confirmedV2: p.confirmedV2,
... ...
@@ -67,7 +67,7 @@ func (p *v2Pusher) Push(ctx context.Context) (err error) {
67 67
 	}
68 68
 
69 69
 	if err = p.pushV2Repository(ctx); err != nil {
70
-		if continueOnError(err) {
70
+		if continueOnError(err, p.endpoint.Mirror) {
71 71
 			return fallbackError{
72 72
 				err:         err,
73 73
 				confirmedV2: p.pushState.confirmedV2,