소스 검색

Fix for #17168 misleading pull error

This fix avoids overwritting the previous error messages, ensures the client gets the correct error messages and not just the most recent message during the pull request.
For this `var lastErr` replaced with a slice which acts as a temp place holder for the list of returned error messages for every attempt.
The slice is later joined and returned to the caller function after searching for the image with diffirent versions(v2,v1,v0).

Updated the code with check for no space left on device error occurance and prevent the
daemon on falling back to v1,v0.

Incorporated the comments from @calavera, @RichardScothern, @cpuguy83

Signed-off-by: Anil Belur <askb23@gmail.com>
Anil Belur 9 년 전
부모
커밋
31cdc63419
2개의 변경된 파일27개의 추가작업 그리고 13개의 파일을 삭제
  1. 22 13
      graph/pull.go
  2. 5 0
      registry/registry.go

+ 22 - 13
graph/pull.go

@@ -3,6 +3,7 @@ package graph
 import (
 import (
 	"fmt"
 	"fmt"
 	"io"
 	"io"
+	"strings"
 
 
 	"github.com/Sirupsen/logrus"
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/cliconfig"
 	"github.com/docker/docker/cliconfig"
@@ -87,12 +88,13 @@ func (s *TagStore) Pull(image string, tag string, imagePullConfig *ImagePullConf
 	}
 	}
 
 
 	var (
 	var (
-		lastErr error
+		// use a slice to append the error strings and return a joined string to caller
+		errors []string
 
 
 		// discardNoSupportErrors is used to track whether an endpoint encountered an error of type registry.ErrNoSupport
 		// discardNoSupportErrors is used to track whether an endpoint encountered an error of type registry.ErrNoSupport
-		// By default it is false, which means that if a ErrNoSupport error is encountered, it will be saved in lastErr.
+		// By default it is false, which means that if a ErrNoSupport error is encountered, it will be saved in errors.
 		// As soon as another kind of error is encountered, discardNoSupportErrors is set to true, avoiding the saving of
 		// As soon as another kind of error is encountered, discardNoSupportErrors is set to true, avoiding the saving of
-		// any subsequent ErrNoSupport errors in lastErr.
+		// any subsequent ErrNoSupport errors in errors.
 		// It's needed for pull-by-digest on v1 endpoints: if there are only v1 endpoints configured, the error should be
 		// It's needed for pull-by-digest on v1 endpoints: if there are only v1 endpoints configured, the error should be
 		// returned and displayed, but if there was a v2 endpoint which supports pull-by-digest, then the last relevant
 		// returned and displayed, but if there was a v2 endpoint which supports pull-by-digest, then the last relevant
 		// error is the ones from v2 endpoints not v1.
 		// error is the ones from v2 endpoints not v1.
@@ -103,7 +105,7 @@ func (s *TagStore) Pull(image string, tag string, imagePullConfig *ImagePullConf
 
 
 		puller, err := newPuller(s, endpoint, repoInfo, imagePullConfig, sf)
 		puller, err := newPuller(s, endpoint, repoInfo, imagePullConfig, sf)
 		if err != nil {
 		if err != nil {
-			lastErr = err
+			errors = append(errors, err.Error())
 			continue
 			continue
 		}
 		}
 		if fallback, err := puller.Pull(tag); err != nil {
 		if fallback, err := puller.Pull(tag); err != nil {
@@ -111,28 +113,35 @@ func (s *TagStore) Pull(image string, tag string, imagePullConfig *ImagePullConf
 				if _, ok := err.(registry.ErrNoSupport); !ok {
 				if _, ok := err.(registry.ErrNoSupport); !ok {
 					// Because we found an error that's not ErrNoSupport, discard all subsequent ErrNoSupport errors.
 					// Because we found an error that's not ErrNoSupport, discard all subsequent ErrNoSupport errors.
 					discardNoSupportErrors = true
 					discardNoSupportErrors = true
-					// save the current error
-					lastErr = err
+					// append subsequent errors
+					errors = append(errors, err.Error())
 				} else if !discardNoSupportErrors {
 				} else if !discardNoSupportErrors {
 					// Save the ErrNoSupport error, because it's either the first error or all encountered errors
 					// Save the ErrNoSupport error, because it's either the first error or all encountered errors
 					// were also ErrNoSupport errors.
 					// were also ErrNoSupport errors.
-					lastErr = err
+					// append subsequent errors
+					errors = append(errors, err.Error())
 				}
 				}
 				continue
 				continue
 			}
 			}
-			logrus.Debugf("Not continuing with error: %v", err)
-			return err
-
+			errors = append(errors, err.Error())
+			logrus.Debugf("Not continuing with error: %v", fmt.Errorf(strings.Join(errors, "\n")))
+			if len(errors) > 0 {
+				return fmt.Errorf(strings.Join(errors, "\n"))
+			}
 		}
 		}
 
 
 		s.eventsService.Log("pull", logName, "")
 		s.eventsService.Log("pull", logName, "")
 		return nil
 		return nil
 	}
 	}
 
 
-	if lastErr == nil {
-		lastErr = fmt.Errorf("no endpoints found for %s", image)
+	if len(errors) == 0 {
+		return fmt.Errorf("no endpoints found for %s", image)
+	}
+
+	if len(errors) > 0 {
+		return fmt.Errorf(strings.Join(errors, "\n"))
 	}
 	}
-	return lastErr
+	return nil
 }
 }
 
 
 // writeStatus writes a status message to out. If layersDownloaded is true, the
 // writeStatus writes a status message to out. If layersDownloaded is true, the

+ 5 - 0
registry/registry.go

@@ -13,6 +13,7 @@ import (
 	"path/filepath"
 	"path/filepath"
 	"runtime"
 	"runtime"
 	"strings"
 	"strings"
+	"syscall"
 	"time"
 	"time"
 
 
 	"github.com/Sirupsen/logrus"
 	"github.com/Sirupsen/logrus"
@@ -219,6 +220,10 @@ func ContinueOnError(err error) bool {
 		return shouldV2Fallback(v)
 		return shouldV2Fallback(v)
 	case *client.UnexpectedHTTPResponseError:
 	case *client.UnexpectedHTTPResponseError:
 		return true
 		return true
+	case error:
+		if val := strings.Contains(err.Error(), strings.ToLower(syscall.ENOSPC.Error())); val {
+			return false
+		}
 	}
 	}
 	// let's be nice and fallback if the error is a completely
 	// let's be nice and fallback if the error is a completely
 	// unexpected one.
 	// unexpected one.