Browse code

Merge pull request #21159 from runcom/fix-retry-push-bug

distribution: errors: do not retry if no credentials provided

Aaron Lehmann authored on 2016/03/15 01:47:13
Showing 3 changed files
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"github.com/docker/distribution/registry/api/errcode"
9 9
 	"github.com/docker/distribution/registry/api/v2"
10 10
 	"github.com/docker/distribution/registry/client"
11
+	"github.com/docker/distribution/registry/client/auth"
11 12
 	"github.com/docker/docker/distribution/xfer"
12 13
 )
13 14
 
... ...
@@ -90,6 +91,9 @@ func retryOnError(err error) error {
90 90
 			return xfer.DoNotRetry{Err: err}
91 91
 		}
92 92
 	case *url.Error:
93
+		if v.Err == auth.ErrNoBasicAuthCredentials {
94
+			return xfer.DoNotRetry{Err: v.Err}
95
+		}
93 96
 		return retryOnError(v.Err)
94 97
 	case *client.UnexpectedHTTPResponseError:
95 98
 		return xfer.DoNotRetry{Err: err}
... ...
@@ -254,3 +254,12 @@ func (s *DockerHubPullSuite) TestPullClientDisconnect(c *check.C) {
254 254
 	_, err = s.CmdWithError("inspect", repoName)
255 255
 	c.Assert(err, checker.NotNil, check.Commentf("image was pulled after client disconnected"))
256 256
 }
257
+
258
+func (s *DockerRegistryAuthSuite) TestPullNoCredentialsNotFound(c *check.C) {
259
+	// we don't care about the actual image, we just want to see image not found
260
+	// because that means v2 call returned 401 and we fell back to v1 which usually
261
+	// gives a 404 (in this case the test registry doesn't handle v1 at all)
262
+	out, _, err := dockerCmdWithError("pull", privateRegistryURL+"/busybox")
263
+	c.Assert(err, check.NotNil, check.Commentf(out))
264
+	c.Assert(out, checker.Contains, "Error: image busybox not found")
265
+}
... ...
@@ -527,3 +527,22 @@ func (s *DockerTrustSuite) TestTrustedPushWithReleasesDelegation(c *check.C) {
527 527
 	c.Assert(err, check.IsNil, check.Commentf("Unable to read targets/releases metadata"))
528 528
 	c.Assert(string(contents), checker.Contains, `"latest"`, check.Commentf(string(contents)))
529 529
 }
530
+
531
+func (s *DockerRegistryAuthSuite) TestPushNoCredentialsNoRetry(c *check.C) {
532
+	repoName := fmt.Sprintf("%s/busybox", privateRegistryURL)
533
+	dockerCmd(c, "tag", "busybox", repoName)
534
+	out, _, err := dockerCmdWithError("push", repoName)
535
+	c.Assert(err, check.NotNil, check.Commentf(out))
536
+	c.Assert(out, check.Not(checker.Contains), "Retrying")
537
+	c.Assert(out, checker.Contains, "no basic auth credentials")
538
+}
539
+
540
+// This may be flaky but it's needed not to regress on unauthorized push, see #21054
541
+func (s *DockerSuite) TestPushToCentralRegistryUnauthorized(c *check.C) {
542
+	testRequires(c, Network)
543
+	repoName := "test/busybox"
544
+	dockerCmd(c, "tag", "busybox", repoName)
545
+	out, _, err := dockerCmdWithError("push", repoName)
546
+	c.Assert(err, check.NotNil, check.Commentf(out))
547
+	c.Assert(out, checker.Contains, "unauthorized: access to the requested resource is not authorized")
548
+}