Browse code

Merge pull request #19424 from aaronlehmann/revert-multiple-pull-errors

Revert reporting of multiple pull errors

Tibor Vass authored on 2016/01/19 23:51:54
Showing 3 changed files
... ...
@@ -3,7 +3,6 @@ package distribution
3 3
 import (
4 4
 	"fmt"
5 5
 	"os"
6
-	"strings"
7 6
 
8 7
 	"github.com/Sirupsen/logrus"
9 8
 	"github.com/docker/docker/api"
... ...
@@ -97,13 +96,12 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
97 97
 	}
98 98
 
99 99
 	var (
100
-		// use a slice to append the error strings and return a joined string to caller
101
-		errors []string
100
+		lastErr error
102 101
 
103 102
 		// discardNoSupportErrors is used to track whether an endpoint encountered an error of type registry.ErrNoSupport
104
-		// By default it is false, which means that if a ErrNoSupport error is encountered, it will be saved in errors.
103
+		// By default it is false, which means that if a ErrNoSupport error is encountered, it will be saved in lastErr.
105 104
 		// As soon as another kind of error is encountered, discardNoSupportErrors is set to true, avoiding the saving of
106
-		// any subsequent ErrNoSupport errors in errors.
105
+		// any subsequent ErrNoSupport errors in lastErr.
107 106
 		// It's needed for pull-by-digest on v1 endpoints: if there are only v1 endpoints configured, the error should be
108 107
 		// returned and displayed, but if there was a v2 endpoint which supports pull-by-digest, then the last relevant
109 108
 		// error is the ones from v2 endpoints not v1.
... ...
@@ -123,7 +121,7 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
123 123
 
124 124
 		puller, err := newPuller(endpoint, repoInfo, imagePullConfig)
125 125
 		if err != nil {
126
-			errors = append(errors, err.Error())
126
+			lastErr = err
127 127
 			continue
128 128
 		}
129 129
 		if err := puller.Pull(ctx, ref); err != nil {
... ...
@@ -144,34 +142,28 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
144 144
 					// Because we found an error that's not ErrNoSupport, discard all subsequent ErrNoSupport errors.
145 145
 					discardNoSupportErrors = true
146 146
 					// append subsequent errors
147
-					errors = append(errors, err.Error())
147
+					lastErr = err
148 148
 				} else if !discardNoSupportErrors {
149 149
 					// Save the ErrNoSupport error, because it's either the first error or all encountered errors
150 150
 					// were also ErrNoSupport errors.
151 151
 					// append subsequent errors
152
-					errors = append(errors, err.Error())
152
+					lastErr = err
153 153
 				}
154 154
 				continue
155 155
 			}
156
-			errors = append(errors, err.Error())
157
-			logrus.Debugf("Not continuing with error: %v", fmt.Errorf(strings.Join(errors, "\n")))
158
-			if len(errors) > 0 {
159
-				return fmt.Errorf(strings.Join(errors, "\n"))
160
-			}
156
+			logrus.Debugf("Not continuing with error: %v", err)
157
+			return err
161 158
 		}
162 159
 
163 160
 		imagePullConfig.ImageEventLogger(ref.String(), repoInfo.Name(), "pull")
164 161
 		return nil
165 162
 	}
166 163
 
167
-	if len(errors) == 0 {
168
-		return fmt.Errorf("no endpoints found for %s", ref.String())
164
+	if lastErr == nil {
165
+		lastErr = fmt.Errorf("no endpoints found for %s", ref.String())
169 166
 	}
170 167
 
171
-	if len(errors) > 0 {
172
-		return fmt.Errorf(strings.Join(errors, "\n"))
173
-	}
174
-	return nil
168
+	return lastErr
175 169
 }
176 170
 
177 171
 // writeStatus writes a status message to out. If layersDownloaded is true, the
... ...
@@ -279,18 +279,6 @@ func (s *DockerSchema1RegistrySuite) TestPullIDStability(c *check.C) {
279 279
 	testPullIDStability(c)
280 280
 }
281 281
 
282
-// TestPullFallbackOn404 tries to pull a nonexistent manifest and confirms that
283
-// the pull falls back to the v1 protocol.
284
-//
285
-// Ref: docker/docker#18832
286
-func (s *DockerRegistrySuite) TestPullFallbackOn404(c *check.C) {
287
-	repoName := fmt.Sprintf("%v/does/not/exist", privateRegistryURL)
288
-
289
-	out, _, _ := dockerCmdWithError("pull", repoName)
290
-
291
-	c.Assert(out, checker.Contains, "v1 ping attempt")
292
-}
293
-
294 282
 func (s *DockerRegistrySuite) TestPullManifestList(c *check.C) {
295 283
 	testRequires(c, NotArm)
296 284
 	pushDigest, err := setupImage(c)
... ...
@@ -62,6 +62,7 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
62 62
 			out, err := s.CmdWithError("pull", "-a", e.Alias)
63 63
 			c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", out))
64 64
 			c.Assert(out, checker.Contains, fmt.Sprintf("Error: image %s not found", e.Repo), check.Commentf("expected image not found error messages"))
65
+			c.Assert(out, checker.Not(checker.Contains), "unauthorized", check.Commentf(`message should not contain "unauthorized"`))
65 66
 		}
66 67
 	}
67 68