Skip to content

Commit 8048663

Browse files
jonjohnsonjrjonjohnsonjr
authored
Race http fallback ping (#1521)
This implements a "happy eyeballs" (RFC 6555) style of race in order to speed up http fallback during registry pings. This heavily borrows code from net/dial.go which implements this same kind of race for dual stack DNS. Rather than waiting for the initial "https" ping to time out completely before attempting to use "http", we will instead start a second fallback ping after 300ms and return whichever response comes back first. This partially reverts the portion of #1165 that reduced the dial timeout to workaround this issue. Also, drop unused parseChallenge to appease the linter.
1 parent e64ff3a commit 8048663

3 files changed

Lines changed: 123 additions & 113 deletions

File tree

‎pkg/v1/remote/options.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,7 @@ const (
103103
var DefaultTransport http.RoundTripper = &http.Transport{
104104
Proxy: http.ProxyFromEnvironment,
105105
DialContext: (&net.Dialer{
106-
// By default we wrap the transport in retries, so reduce the
107-
// default dial timeout to 5s to avoid 5x 30s of connection
108-
// timeouts when doing the "ping" on certain http registries.
109-
Timeout: 5 * time.Second,
106+
Timeout: 30 * time.Second,
110107
KeepAlive: 30 * time.Second,
111108
}).DialContext,
112109
ForceAttemptHTTP2: true,

‎pkg/v1/remote/transport/ping.go‎

Lines changed: 110 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ import (
2121
"io"
2222
"net/http"
2323
"strings"
24+
"time"
2425

2526
authchallenge "github.com/docker/distribution/registry/client/auth/challenge"
27+
"github.com/google/go-containerregistry/pkg/logs"
2628
"github.com/google/go-containerregistry/pkg/name"
2729
)
2830

@@ -34,6 +36,9 @@ const (
3436
bearer challenge = "bearer"
3537
)
3638

39+
// 300ms is the default fallback period for go's DNS dialer but we could make this configurable.
40+
var fallbackDelay = 300 * time.Millisecond
41+
3742
type pingResp struct {
3843
challenge challenge
3944

@@ -49,82 +54,125 @@ func (c challenge) Canonical() challenge {
4954
return challenge(strings.ToLower(string(c)))
5055
}
5156

52-
func parseChallenge(suffix string) map[string]string {
53-
kv := make(map[string]string)
54-
for _, token := range strings.Split(suffix, ",") {
55-
// Trim any whitespace around each token.
56-
token = strings.Trim(token, " ")
57-
58-
// Break the token into a key/value pair
59-
if parts := strings.SplitN(token, "=", 2); len(parts) == 2 {
60-
// Unquote the value, if it is quoted.
61-
kv[parts[0]] = strings.Trim(parts[1], `"`)
62-
} else {
63-
// If there was only one part, treat is as a key with an empty value
64-
kv[token] = ""
65-
}
66-
}
67-
return kv
68-
}
69-
7057
func ping(ctx context.Context, reg name.Registry, t http.RoundTripper) (*pingResp, error) {
71-
client := http.Client{Transport: t}
72-
7358
// This first attempts to use "https" for every request, falling back to http
7459
// if the registry matches our localhost heuristic or if it is intentionally
7560
// set to insecure via name.NewInsecureRegistry.
7661
schemes := []string{"https"}
7762
if reg.Scheme() == "http" {
7863
schemes = append(schemes, "http")
7964
}
65+
if len(schemes) == 1 {
66+
return pingSingle(ctx, reg, t, schemes[0])
67+
}
68+
return pingParallel(ctx, reg, t, schemes)
69+
}
8070

81-
var errs []error
82-
for _, scheme := range schemes {
83-
url := fmt.Sprintf("%s://%s/v2/", scheme, reg.Name())
84-
req, err := http.NewRequest(http.MethodGet, url, nil)
85-
if err != nil {
86-
return nil, err
87-
}
88-
resp, err := client.Do(req.WithContext(ctx))
89-
if err != nil {
90-
errs = append(errs, err)
91-
// Potentially retry with http.
92-
continue
93-
}
94-
defer func() {
95-
// By draining the body, make sure to reuse the connection made by
96-
// the ping for the following access to the registry
97-
io.Copy(io.Discard, resp.Body)
98-
resp.Body.Close()
99-
}()
100-
101-
switch resp.StatusCode {
102-
case http.StatusOK:
103-
// If we get a 200, then no authentication is needed.
71+
func pingSingle(ctx context.Context, reg name.Registry, t http.RoundTripper, scheme string) (*pingResp, error) {
72+
client := http.Client{Transport: t}
73+
url := fmt.Sprintf("%s://%s/v2/", scheme, reg.Name())
74+
req, err := http.NewRequest(http.MethodGet, url, nil)
75+
if err != nil {
76+
return nil, err
77+
}
78+
resp, err := client.Do(req.WithContext(ctx))
79+
if err != nil {
80+
return nil, err
81+
}
82+
defer func() {
83+
// By draining the body, make sure to reuse the connection made by
84+
// the ping for the following access to the registry
85+
io.Copy(io.Discard, resp.Body)
86+
resp.Body.Close()
87+
}()
88+
89+
switch resp.StatusCode {
90+
case http.StatusOK:
91+
// If we get a 200, then no authentication is needed.
92+
return &pingResp{
93+
challenge: anonymous,
94+
scheme: scheme,
95+
}, nil
96+
case http.StatusUnauthorized:
97+
if challenges := authchallenge.ResponseChallenges(resp); len(challenges) != 0 {
98+
// If we hit more than one, let's try to find one that we know how to handle.
99+
wac := pickFromMultipleChallenges(challenges)
104100
return &pingResp{
105-
challenge: anonymous,
106-
scheme: scheme,
101+
challenge: challenge(wac.Scheme).Canonical(),
102+
parameters: wac.Parameters,
103+
scheme: scheme,
107104
}, nil
108-
case http.StatusUnauthorized:
109-
if challenges := authchallenge.ResponseChallenges(resp); len(challenges) != 0 {
110-
// If we hit more than one, let's try to find one that we know how to handle.
111-
wac := pickFromMultipleChallenges(challenges)
112-
return &pingResp{
113-
challenge: challenge(wac.Scheme).Canonical(),
114-
parameters: wac.Parameters,
115-
scheme: scheme,
116-
}, nil
105+
}
106+
// Otherwise, just return the challenge without parameters.
107+
return &pingResp{
108+
challenge: challenge(resp.Header.Get("WWW-Authenticate")).Canonical(),
109+
scheme: scheme,
110+
}, nil
111+
default:
112+
return nil, CheckError(resp, http.StatusOK, http.StatusUnauthorized)
113+
}
114+
}
115+
116+
// Based on the golang happy eyeballs dialParallel impl in net/dial.go.
117+
func pingParallel(ctx context.Context, reg name.Registry, t http.RoundTripper, schemes []string) (*pingResp, error) {
118+
returned := make(chan struct{})
119+
defer close(returned)
120+
121+
type pingResult struct {
122+
*pingResp
123+
error
124+
primary bool
125+
done bool
126+
}
127+
128+
results := make(chan pingResult)
129+
130+
startRacer := func(ctx context.Context, scheme string) {
131+
pr, err := pingSingle(ctx, reg, t, scheme)
132+
select {
133+
case results <- pingResult{pingResp: pr, error: err, primary: scheme == "https", done: true}:
134+
case <-returned:
135+
if pr != nil {
136+
logs.Debug.Printf("%s lost race", scheme)
137+
}
138+
}
139+
}
140+
141+
var primary, fallback pingResult
142+
143+
primaryCtx, primaryCancel := context.WithCancel(ctx)
144+
defer primaryCancel()
145+
go startRacer(primaryCtx, schemes[0])
146+
147+
fallbackTimer := time.NewTimer(fallbackDelay)
148+
defer fallbackTimer.Stop()
149+
150+
for {
151+
select {
152+
case <-fallbackTimer.C:
153+
fallbackCtx, fallbackCancel := context.WithCancel(ctx)
154+
defer fallbackCancel()
155+
go startRacer(fallbackCtx, schemes[1])
156+
157+
case res := <-results:
158+
if res.error == nil {
159+
return res.pingResp, nil
160+
}
161+
if res.primary {
162+
primary = res
163+
} else {
164+
fallback = res
165+
}
166+
if primary.done && fallback.done {
167+
return nil, multierrs([]error{primary.error, fallback.error})
168+
}
169+
if res.primary && fallbackTimer.Stop() {
170+
// Primary failed and we haven't started the fallback,
171+
// reset time to start fallback immediately.
172+
fallbackTimer.Reset(0)
117173
}
118-
// Otherwise, just return the challenge without parameters.
119-
return &pingResp{
120-
challenge: challenge(resp.Header.Get("WWW-Authenticate")).Canonical(),
121-
scheme: scheme,
122-
}, nil
123-
default:
124-
return nil, CheckError(resp, http.StatusOK, http.StatusUnauthorized)
125174
}
126175
}
127-
return nil, multierrs(errs)
128176
}
129177

130178
func pickFromMultipleChallenges(challenges []authchallenge.Challenge) authchallenge.Challenge {

‎pkg/v1/remote/transport/ping_test.go‎

Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -20,59 +20,17 @@ import (
2020
"net/http/httptest"
2121
"net/url"
2222
"strings"
23+
"sync/atomic"
2324
"testing"
25+
"time"
2426

25-
"github.com/google/go-cmp/cmp"
2627
"github.com/google/go-containerregistry/pkg/name"
2728
)
2829

2930
var (
3031
testRegistry, _ = name.NewRegistry("localhost:8080", name.StrictValidation)
3132
)
3233

33-
func TestChallengeParsing(t *testing.T) {
34-
tests := []struct {
35-
input string
36-
output map[string]string
37-
}{{
38-
input: `foo="bar"`,
39-
output: map[string]string{
40-
"foo": "bar",
41-
},
42-
}, {
43-
input: `foo`,
44-
output: map[string]string{
45-
"foo": "",
46-
},
47-
}, {
48-
input: `foo="bar",baz="blah"`,
49-
output: map[string]string{
50-
"foo": "bar",
51-
"baz": "blah",
52-
},
53-
}, {
54-
input: `baz="blah", foo="bar"`,
55-
output: map[string]string{
56-
"foo": "bar",
57-
"baz": "blah",
58-
},
59-
}, {
60-
input: `realm="https://gcr.io/v2/token", service="gcr.io", scope="repository:foo/bar:pull"`,
61-
output: map[string]string{
62-
"realm": "https://gcr.io/v2/token",
63-
"service": "gcr.io",
64-
"scope": "repository:foo/bar:pull",
65-
},
66-
}}
67-
68-
for _, test := range tests {
69-
params := parseChallenge(test.input)
70-
if diff := cmp.Diff(test.output, params); diff != "" {
71-
t.Errorf("parseChallenge(%s); (-want +got) %s", test.input, diff)
72-
}
73-
}
74-
}
75-
7634
func TestPingNoChallenge(t *testing.T) {
7735
server := httptest.NewServer(
7836
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -218,7 +176,7 @@ func TestUnsupportedStatus(t *testing.T) {
218176
func TestPingHttpFallback(t *testing.T) {
219177
tests := []struct {
220178
reg name.Registry
221-
wantCount int
179+
wantCount int64
222180
err string
223181
contains []string
224182
}{{
@@ -234,10 +192,15 @@ func TestPingHttpFallback(t *testing.T) {
234192
contains: []string{"https://us.gcr.io/v2/", "http://us.gcr.io/v2/"},
235193
}}
236194

237-
gotCount := 0
195+
gotCount := int64(0)
238196
server := httptest.NewServer(
239197
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
240-
gotCount++
198+
atomic.AddInt64(&gotCount, 1)
199+
if r.URL.Scheme != "http" {
200+
// Sleep a little bit so we can exercise the
201+
// happy eyeballs race.
202+
time.Sleep(5 * time.Millisecond)
203+
}
241204
w.WriteHeader(http.StatusOK)
242205
}))
243206
defer server.Close()
@@ -248,6 +211,8 @@ func TestPingHttpFallback(t *testing.T) {
248211
},
249212
}
250213

214+
fallbackDelay = 2 * time.Millisecond
215+
251216
for _, test := range tests {
252217
// This is the last one, fatal error it.
253218
if strings.Contains(test.reg.String(), "us.gcr.io") {

0 commit comments

Comments
 (0)