c8d/integration-cli: Adjust DockerRegistryAuthTokenSuite#46865
c8d/integration-cli: Adjust DockerRegistryAuthTokenSuite#46865thaJeztah merged 1 commit intomoby:masterfrom
Conversation
The auth service error response is not a part of the spec and containerd doesn't parse it like the Docker's distribution does. Check for containerd specific errors instead. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
3c32515 to
e262cd3
Compare
| // Auth service errors are not part of the spec and containerd doesn't parse them. | ||
| if testEnv.UsingSnapshotter() { | ||
| assert.Check(c, is.Contains(out, "failed to authorize: failed to fetch anonymous token")) | ||
| assert.Check(c, is.Contains(out, "503 Service Unavailable")) |
There was a problem hiding this comment.
Still curious; is 503 the correct response for this? If this is docker hub returning it, I wonder if that should've been a 4XX (e.g. 429 https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429)
There was a problem hiding this comment.
We're using our test server for this, but curious if it reflects what docker hub returns;
moby/integration-cli/docker_cli_push_test.go
Lines 238 to 246 in c8b9dfb
There was a problem hiding this comment.
Yes, because that test server returns TOOMANYREQUESTS after the 3 tries (which give UNAVAILABLE).
Containerd doesn't do the retry and it fails fast with the UNAVAILABLE and doesn't reach the TOOMANYREQUESTS.
There was a problem hiding this comment.
Wondering now if the retry was added to account for "cron-jobs" (I recall some threads about some hours being notable for cron-jobs hammering Docker Hub, which may cause brief moments where services can return a 503).
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
let's not block this, because this matches current reality (but we can continue discussing if we want to make improvements / changes in containerd's handling of these auth requests, to match the old implementation)
The auth service error response is not a part of the spec and containerd doesn't parse it like the Docker's distribution does.
Check for containerd specific errors instead.
- What I did
- How I did it
- How to verify it
See
DockerRegistryAuthTokenSuitesuite in CI.- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)