From ebcb7d6b406fe50ea9a237c73004d75884184c33 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 19 Jul 2017 10:20:13 -0400 Subject: [PATCH] Remove string checking in API error handling Use strongly typed errors to set HTTP status codes. Error interfaces are defined in the api/errors package and errors returned from controllers are checked against these interfaces. Errors can be wraeped in a pkg/errors.Causer, as long as somewhere in the line of causes one of the interfaces is implemented. The special error interfaces take precedence over Causer, meaning if both Causer and one of the new error interfaces are implemented, the Causer is not traversed. Signed-off-by: Brian Goff --- api/errdefs/defs.go | 54 +++++ api/errdefs/doc.go | 8 + api/errdefs/is.go | 86 +++++++ api/errors/errors.go | 47 ---- api/errors/errors_test.go | 64 ------ api/server/httputils/errors.go | 78 +++---- api/server/httputils/form.go | 24 +- api/server/httputils/httputils.go | 20 +- api/server/middleware/version.go | 13 +- api/server/router/build/build_routes.go | 32 ++- api/server/router/container/backend.go | 2 +- api/server/router/container/container.go | 12 +- .../router/container/container_routes.go | 51 ++--- api/server/router/container/copy.go | 20 +- api/server/router/container/exec.go | 14 +- api/server/router/experimental.go | 16 +- api/server/router/image/image_routes.go | 28 ++- api/server/router/network/filter.go | 12 +- api/server/router/network/network_routes.go | 28 ++- api/server/router/session/session_routes.go | 17 +- api/server/router/swarm/cluster_routes.go | 40 ++-- api/server/router/system/system_routes.go | 13 +- api/server/server.go | 13 +- api/swagger.yaml | 4 - api/types/filters/parse.go | 19 +- builder/dockerfile/builder.go | 8 +- builder/dockerfile/dispatchers.go | 2 +- builder/dockerfile/errors.go | 15 ++ builder/dockerfile/evaluator.go | 10 +- builder/remotecontext/errors.go | 75 +++++++ builder/remotecontext/remote.go | 31 ++- container/container.go | 17 +- container/container_unix.go | 21 +- daemon/archive.go | 55 ++++- daemon/attach.go | 10 +- daemon/cluster/cluster.go | 21 +- .../cluster/controllers/plugin/controller.go | 4 +- daemon/cluster/errors.go | 114 ++++++++++ daemon/cluster/executor/backend.go | 2 +- daemon/cluster/executor/container/adapter.go | 2 +- daemon/cluster/helpers.go | 26 +-- daemon/cluster/listen_addr.go | 29 ++- daemon/cluster/networks.go | 12 +- daemon/cluster/nodes.go | 3 +- daemon/cluster/services.go | 5 +- daemon/cluster/swarm.go | 25 +-- daemon/container.go | 31 ++- daemon/container_linux.go | 2 +- daemon/container_operations.go | 3 +- daemon/container_operations_unix.go | 2 +- daemon/create.go | 28 ++- daemon/daemon_unix.go | 9 +- daemon/daemon_windows.go | 4 +- daemon/delete.go | 7 +- daemon/errors.go | 211 ++++++++++++++++-- daemon/exec.go | 12 +- daemon/exec/exec.go | 5 + daemon/image.go | 16 +- daemon/image_delete.go | 10 +- daemon/image_pull.go | 6 +- daemon/images.go | 2 +- daemon/import.go | 8 +- daemon/inspect.go | 8 +- daemon/kill.go | 10 +- daemon/list.go | 12 +- daemon/logger/logger.go | 12 +- daemon/logs.go | 18 +- daemon/names.go | 7 +- daemon/network.go | 11 +- daemon/oci_linux.go | 2 +- daemon/pause.go | 2 +- daemon/prune.go | 2 +- daemon/rename.go | 11 +- daemon/resize.go | 2 +- daemon/search.go | 7 +- daemon/start.go | 82 +++---- daemon/start_unix.go | 5 +- daemon/stats/collector.go | 4 +- daemon/stats_unix.go | 5 +- daemon/stop.go | 9 +- daemon/top_unix.go | 2 +- daemon/update.go | 8 +- daemon/volumes.go | 9 +- distribution/errors.go | 84 +++++-- distribution/pull.go | 3 +- distribution/pull_v1.go | 6 +- distribution/pull_v2.go | 6 +- distribution/push_v1.go | 6 +- image/store.go | 12 +- integration-cli/docker_api_containers_test.go | 37 ++- integration-cli/docker_api_create_test.go | 10 +- .../docker_api_exec_resize_test.go | 2 +- integration-cli/docker_api_exec_test.go | 6 +- integration-cli/docker_api_resize_test.go | 4 +- integration-cli/docker_api_test.go | 4 +- .../docker_cli_cp_from_container_test.go | 10 +- .../docker_cli_cp_to_container_test.go | 7 +- integration-cli/docker_cli_cp_utils_test.go | 2 +- integration-cli/docker_cli_inspect_test.go | 3 +- integration-cli/docker_cli_links_test.go | 2 +- integration-cli/docker_cli_netmode_test.go | 4 +- integration-cli/docker_cli_ps_test.go | 2 +- integration-cli/docker_cli_swarm_test.go | 4 +- .../docker_deprecated_api_v124_test.go | 7 +- opts/env.go | 4 +- pkg/authorization/authz.go | 5 +- plugin/backend_linux.go | 33 ++- plugin/errors.go | 94 ++++++++ plugin/manager_linux.go | 15 +- plugin/store.go | 42 ++-- reference/errors.go | 25 +++ reference/store.go | 16 +- registry/auth.go | 59 +++-- registry/endpoint_v1.go | 6 +- registry/errors.go | 86 +++++++ registry/service.go | 9 +- registry/session.go | 12 +- runconfig/config.go | 22 +- runconfig/config_test.go | 4 +- runconfig/errors.go | 72 +++--- runconfig/hostconfig.go | 5 +- runconfig/hostconfig_test.go | 2 +- volume/drivers/extpoint.go | 13 +- volume/local/local.go | 67 ++++-- volume/store/errors.go | 48 +++- volume/validate.go | 22 +- volume/volume.go | 8 +- 127 files changed, 1791 insertions(+), 885 deletions(-) create mode 100644 api/errdefs/defs.go create mode 100644 api/errdefs/doc.go create mode 100644 api/errdefs/is.go delete mode 100644 api/errors/errors.go delete mode 100644 api/errors/errors_test.go create mode 100644 builder/dockerfile/errors.go create mode 100644 builder/remotecontext/errors.go create mode 100644 daemon/cluster/errors.go create mode 100644 plugin/errors.go create mode 100644 reference/errors.go create mode 100644 registry/errors.go diff --git a/api/errdefs/defs.go b/api/errdefs/defs.go new file mode 100644 index 0000000000..4987c623f0 --- /dev/null +++ b/api/errdefs/defs.go @@ -0,0 +1,54 @@ +package errdefs + +// ErrNotFound signals that the requested object doesn't exist +type ErrNotFound interface { + NotFound() +} + +// ErrInvalidParameter signals that the user input is invalid +type ErrInvalidParameter interface { + InvalidParameter() +} + +// ErrConflict signals that some internal state conflicts with the requested action and can't be performed. +// A change in state should be able to clear this error. +type ErrConflict interface { + Conflict() +} + +// ErrUnauthorized is used to signify that the user is not authorized to perform a specific action +type ErrUnauthorized interface { + Unauthorized() +} + +// ErrUnavailable signals that the requested action/subsystem is not available. +type ErrUnavailable interface { + Unavailable() +} + +// ErrForbidden signals that the requested action cannot be performed under any circumstances. +// When a ErrForbidden is returned, the caller should never retry the action. +type ErrForbidden interface { + Forbidden() +} + +// ErrSystem signals that some internal error occurred. +// An example of this would be a failed mount request. +type ErrSystem interface { + ErrSystem() +} + +// ErrNotModified signals that an action can't be performed because it's already in the desired state +type ErrNotModified interface { + NotModified() +} + +// ErrNotImplemented signals that the requested action/feature is not implemented on the system as configured. +type ErrNotImplemented interface { + NotImplemented() +} + +// ErrUnknown signals that the kind of error that occurred is not known. +type ErrUnknown interface { + Unknown() +} diff --git a/api/errdefs/doc.go b/api/errdefs/doc.go new file mode 100644 index 0000000000..065346aa29 --- /dev/null +++ b/api/errdefs/doc.go @@ -0,0 +1,8 @@ +// Package errdefs defines a set of error interfaces that packages should use for communicating classes of errors. +// Errors that cross the package boundary should implement one (and only one) of these interfaces. +// +// Packages should not reference these interfaces directly, only implement them. +// To check if a particular error implements one of these interfaces, there are helper +// functions provided (e.g. `Is`) which can be used rather than asserting the interfaces directly. +// If you must assert on these interfaces, be sure to check the causal chain (`err.Cause()`). +package errdefs diff --git a/api/errdefs/is.go b/api/errdefs/is.go new file mode 100644 index 0000000000..ce574cdd1e --- /dev/null +++ b/api/errdefs/is.go @@ -0,0 +1,86 @@ +package errdefs + +type causer interface { + Cause() error +} + +func getImplementer(err error) error { + switch e := err.(type) { + case + ErrNotFound, + ErrInvalidParameter, + ErrConflict, + ErrUnauthorized, + ErrUnavailable, + ErrForbidden, + ErrSystem, + ErrNotModified, + ErrNotImplemented, + ErrUnknown: + return e + case causer: + return getImplementer(e.Cause()) + default: + return err + } +} + +// IsNotFound returns if the passed in error is a ErrNotFound +func IsNotFound(err error) bool { + _, ok := getImplementer(err).(ErrNotFound) + return ok +} + +// IsInvalidParameter returns if the passed in error is an ErrInvalidParameter +func IsInvalidParameter(err error) bool { + _, ok := getImplementer(err).(ErrInvalidParameter) + return ok +} + +// IsConflict returns if the passed in error is a ErrConflict +func IsConflict(err error) bool { + _, ok := getImplementer(err).(ErrConflict) + return ok +} + +// IsUnauthorized returns if the the passed in error is an ErrUnauthorized +func IsUnauthorized(err error) bool { + _, ok := getImplementer(err).(ErrUnauthorized) + return ok +} + +// IsUnavailable returns if the passed in error is an ErrUnavailable +func IsUnavailable(err error) bool { + _, ok := getImplementer(err).(ErrUnavailable) + return ok +} + +// IsForbidden returns if the passed in error is a ErrForbidden +func IsForbidden(err error) bool { + _, ok := getImplementer(err).(ErrForbidden) + return ok +} + +// IsSystem returns if the passed in error is a ErrSystem +func IsSystem(err error) bool { + _, ok := getImplementer(err).(ErrSystem) + return ok +} + +// IsNotModified returns if the passed in error is a NotModified error +func IsNotModified(err error) bool { + _, ok := getImplementer(err).(ErrNotModified) + return ok +} + +// IsNotImplemented returns if the passed in error is a ErrNotImplemented +func IsNotImplemented(err error) bool { + _, ok := getImplementer(err).(ErrNotImplemented) + return ok +} + +// IsUnknown returns if the passed in error is an ErrUnknown +func IsUnknown(err error) bool { + _, ok := getImplementer(err).(ErrUnknown) + return ok +} diff --git a/api/errors/errors.go b/api/errors/errors.go deleted file mode 100644 index 39d52e1a07..0000000000 --- a/api/errors/errors.go +++ /dev/null @@ -1,47 +0,0 @@ -package errors - -import "net/http" - -// apiError is an error wrapper that also -// holds information about response status codes. -type apiError struct { - error - statusCode int -} - -// HTTPErrorStatusCode returns a status code. -func (e apiError) HTTPErrorStatusCode() int { - return e.statusCode -} - -// NewErrorWithStatusCode allows you to associate -// a specific HTTP Status Code to an error. -// The server will take that code and set -// it as the response status. -func NewErrorWithStatusCode(err error, code int) error { - return apiError{err, code} -} - -// NewBadRequestError creates a new API error -// that has the 400 HTTP status code associated to it. -func NewBadRequestError(err error) error { - return NewErrorWithStatusCode(err, http.StatusBadRequest) -} - -// NewRequestForbiddenError creates a new API error -// that has the 403 HTTP status code associated to it. -func NewRequestForbiddenError(err error) error { - return NewErrorWithStatusCode(err, http.StatusForbidden) -} - -// NewRequestNotFoundError creates a new API error -// that has the 404 HTTP status code associated to it. -func NewRequestNotFoundError(err error) error { - return NewErrorWithStatusCode(err, http.StatusNotFound) -} - -// NewRequestConflictError creates a new API error -// that has the 409 HTTP status code associated to it. -func NewRequestConflictError(err error) error { - return NewErrorWithStatusCode(err, http.StatusConflict) -} diff --git a/api/errors/errors_test.go b/api/errors/errors_test.go deleted file mode 100644 index 1d6a596ac3..0000000000 --- a/api/errors/errors_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package errors - -import ( - "fmt" - "github.com/stretchr/testify/assert" - "net/http" - "testing" -) - -func newError(errorname string) error { - - return fmt.Errorf("test%v", errorname) -} - -func TestErrors(t *testing.T) { - errmsg := newError("apiError") - err := apiError{ - error: errmsg, - statusCode: 0, - } - assert.Equal(t, err.HTTPErrorStatusCode(), err.statusCode) - - errmsg = newError("ErrorWithStatusCode") - errcode := 1 - serr := NewErrorWithStatusCode(errmsg, errcode) - apierr, ok := serr.(apiError) - if !ok { - t.Fatal("excepted err is apiError type") - } - assert.Equal(t, errcode, apierr.statusCode) - - errmsg = newError("NewBadRequestError") - baderr := NewBadRequestError(errmsg) - apierr, ok = baderr.(apiError) - if !ok { - t.Fatal("excepted err is apiError type") - } - assert.Equal(t, http.StatusBadRequest, apierr.statusCode) - - errmsg = newError("RequestForbiddenError") - ferr := NewRequestForbiddenError(errmsg) - apierr, ok = ferr.(apiError) - if !ok { - t.Fatal("excepted err is apiError type") - } - assert.Equal(t, http.StatusForbidden, apierr.statusCode) - - errmsg = newError("RequestNotFoundError") - nerr := NewRequestNotFoundError(errmsg) - apierr, ok = nerr.(apiError) - if !ok { - t.Fatal("excepted err is apiError type") - } - assert.Equal(t, http.StatusNotFound, apierr.statusCode) - - errmsg = newError("RequestConflictError") - cerr := NewRequestConflictError(errmsg) - apierr, ok = cerr.(apiError) - if !ok { - t.Fatal("excepted err is apiError type") - } - assert.Equal(t, http.StatusConflict, apierr.statusCode) - -} diff --git a/api/server/httputils/errors.go b/api/server/httputils/errors.go index b677f95d6a..10628f6b33 100644 --- a/api/server/httputils/errors.go +++ b/api/server/httputils/errors.go @@ -1,9 +1,10 @@ package httputils import ( + "fmt" "net/http" - "strings" + "github.com/docker/docker/api/errdefs" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/versions" "github.com/gorilla/mux" @@ -20,13 +21,8 @@ type httpStatusError interface { HTTPErrorStatusCode() int } -// inputValidationError is an interface -// that errors generated by invalid -// inputs can implement to tell the -// api layer to set a 400 status code -// in the response. -type inputValidationError interface { - IsValidationError() bool +type causer interface { + Cause() error } // GetHTTPErrorStatusCode retrieves status code from error message. @@ -37,49 +33,44 @@ func GetHTTPErrorStatusCode(err error) int { } var statusCode int - errMsg := err.Error() - switch e := err.(type) { - case httpStatusError: - statusCode = e.HTTPErrorStatusCode() - case inputValidationError: + // Stop right there + // Are you sure you should be adding a new error class here? Do one of the existing ones work? + + // Note that the below functions are already checking the error causal chain for matches. + switch { + case errdefs.IsNotFound(err): + statusCode = http.StatusNotFound + case errdefs.IsInvalidParameter(err): statusCode = http.StatusBadRequest + case errdefs.IsConflict(err): + statusCode = http.StatusConflict + case errdefs.IsUnauthorized(err): + statusCode = http.StatusUnauthorized + case errdefs.IsUnavailable(err): + statusCode = http.StatusServiceUnavailable + case errdefs.IsForbidden(err): + statusCode = http.StatusForbidden + case errdefs.IsNotModified(err): + statusCode = http.StatusNotModified + case errdefs.IsNotImplemented(err): + statusCode = http.StatusNotImplemented + case errdefs.IsSystem(err) || errdefs.IsUnknown(err): + statusCode = http.StatusInternalServerError default: statusCode = statusCodeFromGRPCError(err) if statusCode != http.StatusInternalServerError { return statusCode } - // FIXME: this is brittle and should not be necessary, but we still need to identify if - // there are errors falling back into this logic. - // If we need to differentiate between different possible error types, - // we should create appropriate error types that implement the httpStatusError interface. - errStr := strings.ToLower(errMsg) - - for _, status := range []struct { - keyword string - code int - }{ - {"not found", http.StatusNotFound}, - {"cannot find", http.StatusNotFound}, - {"no such", http.StatusNotFound}, - {"bad parameter", http.StatusBadRequest}, - {"no command", http.StatusBadRequest}, - {"conflict", http.StatusConflict}, - {"impossible", http.StatusNotAcceptable}, - {"wrong login/password", http.StatusUnauthorized}, - {"unauthorized", http.StatusUnauthorized}, - {"hasn't been activated", http.StatusForbidden}, - {"this node", http.StatusServiceUnavailable}, - {"needs to be unlocked", http.StatusServiceUnavailable}, - {"certificates have expired", http.StatusServiceUnavailable}, - {"repository does not exist", http.StatusNotFound}, - } { - if strings.Contains(errStr, status.keyword) { - statusCode = status.code - break - } + if e, ok := err.(causer); ok { + return GetHTTPErrorStatusCode(e.Cause()) } + + logrus.WithFields(logrus.Fields{ + "module": "api", + "error_type": fmt.Sprintf("%T", err), + }).Debugf("FIXME: Got an API for which error does not match any expected type!!!: %+v", err) } if statusCode == 0 { @@ -133,6 +124,9 @@ func statusCodeFromGRPCError(err error) int { case codes.Unavailable: // code 14 return http.StatusServiceUnavailable default: + if e, ok := err.(causer); ok { + return statusCodeFromGRPCError(e.Cause()) + } // codes.Canceled(1) // codes.Unknown(2) // codes.DeadlineExceeded(4) diff --git a/api/server/httputils/form.go b/api/server/httputils/form.go index 78bd379c7a..a5f62287ec 100644 --- a/api/server/httputils/form.go +++ b/api/server/httputils/form.go @@ -1,7 +1,6 @@ package httputils import ( - "errors" "net/http" "path/filepath" "strconv" @@ -49,6 +48,16 @@ type ArchiveOptions struct { Path string } +type badParameterError struct { + param string +} + +func (e badParameterError) Error() string { + return "bad parameter: " + e.param + "cannot be empty" +} + +func (e badParameterError) InvalidParameter() {} + // ArchiveFormValues parses form values and turns them into ArchiveOptions. // It fails if the archive name and path are not in the request. func ArchiveFormValues(r *http.Request, vars map[string]string) (ArchiveOptions, error) { @@ -57,14 +66,13 @@ func ArchiveFormValues(r *http.Request, vars map[string]string) (ArchiveOptions, } name := vars["name"] - path := filepath.FromSlash(r.Form.Get("path")) - - switch { - case name == "": - return ArchiveOptions{}, errors.New("bad parameter: 'name' cannot be empty") - case path == "": - return ArchiveOptions{}, errors.New("bad parameter: 'path' cannot be empty") + if name == "" { + return ArchiveOptions{}, badParameterError{"name"} } + path := filepath.FromSlash(r.Form.Get("path")) + if path == "" { + return ArchiveOptions{}, badParameterError{"path"} + } return ArchiveOptions{name, path}, nil } diff --git a/api/server/httputils/httputils.go b/api/server/httputils/httputils.go index 4708d7d16e..57eeb4beba 100644 --- a/api/server/httputils/httputils.go +++ b/api/server/httputils/httputils.go @@ -1,12 +1,12 @@ package httputils import ( - "fmt" "io" "mime" "net/http" "strings" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -43,6 +43,20 @@ func CloseStreams(streams ...interface{}) { } } +type validationError struct { + cause error +} + +func (e validationError) Error() string { + return e.cause.Error() +} + +func (e validationError) Cause() error { + return e.cause +} + +func (e validationError) InvalidParameter() {} + // CheckForJSON makes sure that the request's Content-Type is application/json. func CheckForJSON(r *http.Request) error { ct := r.Header.Get("Content-Type") @@ -58,7 +72,7 @@ func CheckForJSON(r *http.Request) error { if matchesContentType(ct, "application/json") { return nil } - return fmt.Errorf("Content-Type specified (%s) must be 'application/json'", ct) + return validationError{errors.Errorf("Content-Type specified (%s) must be 'application/json'", ct)} } // ParseForm ensures the request form is parsed even with invalid content types. @@ -68,7 +82,7 @@ func ParseForm(r *http.Request) error { return nil } if err := r.ParseForm(); err != nil && !strings.HasPrefix(err.Error(), "mime:") { - return err + return validationError{err} } return nil } diff --git a/api/server/middleware/version.go b/api/server/middleware/version.go index 390a7f0594..c8f9c7340b 100644 --- a/api/server/middleware/version.go +++ b/api/server/middleware/version.go @@ -5,7 +5,6 @@ import ( "net/http" "runtime" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types/versions" "golang.org/x/net/context" ) @@ -28,6 +27,16 @@ func NewVersionMiddleware(s, d, m string) VersionMiddleware { } } +type versionUnsupportedError struct { + version, minVersion string +} + +func (e versionUnsupportedError) Error() string { + return fmt.Sprintf("client version %s is too old. Minimum supported API version is %s, please upgrade your client to a newer version", e.version, e.minVersion) +} + +func (e versionUnsupportedError) InvalidParameter() {} + // WrapHandler returns a new handler function wrapping the previous one in the request chain. func (v VersionMiddleware) WrapHandler(handler func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error) func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { return func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { @@ -37,7 +46,7 @@ func (v VersionMiddleware) WrapHandler(handler func(ctx context.Context, w http. } if versions.LessThan(apiVersion, v.minVersion) { - return errors.NewBadRequestError(fmt.Errorf("client version %s is too old. Minimum supported API version is %s, please upgrade your client to a newer version", apiVersion, v.minVersion)) + return versionUnsupportedError{apiVersion, v.minVersion} } header := fmt.Sprintf("Docker/%s (%s)", v.serverVersion, runtime.GOOS) diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index d9a123915d..2c9a94795e 100644 --- a/api/server/router/build/build_routes.go +++ b/api/server/router/build/build_routes.go @@ -12,7 +12,6 @@ import ( "strings" "sync" - apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" @@ -27,6 +26,14 @@ import ( "golang.org/x/net/context" ) +type invalidIsolationError string + +func (e invalidIsolationError) Error() string { + return fmt.Sprintf("Unsupported isolation: %q", string(e)) +} + +func (e invalidIsolationError) InvalidParameter() {} + func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBuildOptions, error) { version := httputils.VersionFromContext(ctx) options := &types.ImageBuildOptions{} @@ -71,20 +78,20 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui if i := container.Isolation(r.FormValue("isolation")); i != "" { if !container.Isolation.IsValid(i) { - return nil, fmt.Errorf("Unsupported isolation: %q", i) + return nil, invalidIsolationError(i) } options.Isolation = i } if runtime.GOOS != "windows" && options.SecurityOpt != nil { - return nil, fmt.Errorf("The daemon on this platform does not support setting security options on build") + return nil, validationError{fmt.Errorf("The daemon on this platform does not support setting security options on build")} } var buildUlimits = []*units.Ulimit{} ulimitsJSON := r.FormValue("ulimits") if ulimitsJSON != "" { if err := json.Unmarshal([]byte(ulimitsJSON), &buildUlimits); err != nil { - return nil, err + return nil, errors.Wrap(validationError{err}, "error reading ulimit settings") } options.Ulimits = buildUlimits } @@ -105,7 +112,7 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui if buildArgsJSON != "" { var buildArgs = map[string]*string{} if err := json.Unmarshal([]byte(buildArgsJSON), &buildArgs); err != nil { - return nil, err + return nil, errors.Wrap(validationError{err}, "error reading build args") } options.BuildArgs = buildArgs } @@ -114,7 +121,7 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui if labelsJSON != "" { var labels = map[string]string{} if err := json.Unmarshal([]byte(labelsJSON), &labels); err != nil { - return nil, err + return nil, errors.Wrap(validationError{err}, "error reading labels") } options.Labels = labels } @@ -140,6 +147,16 @@ func (br *buildRouter) postPrune(ctx context.Context, w http.ResponseWriter, r * return httputils.WriteJSON(w, http.StatusOK, report) } +type validationError struct { + cause error +} + +func (e validationError) Error() string { + return e.cause.Error() +} + +func (e validationError) InvalidParameter() {} + func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { var ( notVerboseBuffer = bytes.NewBuffer(nil) @@ -173,8 +190,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * buildOptions.AuthConfigs = getAuthConfigs(r.Header) if buildOptions.Squash && !br.daemon.HasExperimental() { - return apierrors.NewBadRequestError( - errors.New("squash is only supported with experimental mode")) + return validationError{errors.New("squash is only supported with experimental mode")} } out := io.Writer(output) diff --git a/api/server/router/container/backend.go b/api/server/router/container/backend.go index d51ed81776..5e261622f8 100644 --- a/api/server/router/container/backend.go +++ b/api/server/router/container/backend.go @@ -51,7 +51,7 @@ type stateBackend interface { type monitorBackend interface { ContainerChanges(name string) ([]archive.Change, error) ContainerInspect(name string, size bool, version string) (interface{}, error) - ContainerLogs(ctx context.Context, name string, config *types.ContainerLogsOptions) (<-chan *backend.LogMessage, error) + ContainerLogs(ctx context.Context, name string, config *types.ContainerLogsOptions) (msgs <-chan *backend.LogMessage, tty bool, err error) ContainerStats(ctx context.Context, name string, config *backend.ContainerStatsConfig) error ContainerTop(name string, psArgs string) (*container.ContainerTopOKBody, error) diff --git a/api/server/router/container/container.go b/api/server/router/container/container.go index 24c3224ee8..90ae6dc38e 100644 --- a/api/server/router/container/container.go +++ b/api/server/router/container/container.go @@ -6,13 +6,19 @@ import ( ) type validationError struct { - error + cause error } -func (validationError) IsValidationError() bool { - return true +func (e validationError) Error() string { + return e.cause.Error() } +func (e validationError) Cause() error { + return e.cause +} + +func (e validationError) InvalidParameter() {} + // containerRouter is a router to talk with the container controller type containerRouter struct { backend Backend diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 57204b2e88..c5c38c093b 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -8,7 +8,7 @@ import ( "strconv" "syscall" - "github.com/docker/docker/api" + "github.com/docker/docker/api/errdefs" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" @@ -18,6 +18,7 @@ import ( containerpkg "github.com/docker/docker/container" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/signal" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" "golang.org/x/net/websocket" @@ -87,7 +88,7 @@ func (s *containerRouter) getContainersLogs(ctx context.Context, w http.Response // with the appropriate status code. stdout, stderr := httputils.BoolValue(r, "stdout"), httputils.BoolValue(r, "stderr") if !(stdout || stderr) { - return fmt.Errorf("Bad parameters: you must choose at least one stream") + return validationError{errors.New("Bad parameters: you must choose at least one stream")} } containerName := vars["name"] @@ -101,19 +102,7 @@ func (s *containerRouter) getContainersLogs(ctx context.Context, w http.Response Details: httputils.BoolValue(r, "details"), } - // doesn't matter what version the client is on, we're using this internally only - // also do we need size? i'm thinking no we don't - raw, err := s.backend.ContainerInspect(containerName, false, api.DefaultVersion) - if err != nil { - return err - } - container, ok := raw.(*types.ContainerJSON) - if !ok { - // %T prints the type. handy! - return fmt.Errorf("expected container to be *types.ContainerJSON but got %T", raw) - } - - msgs, err := s.backend.ContainerLogs(ctx, containerName, logsConfig) + msgs, tty, err := s.backend.ContainerLogs(ctx, containerName, logsConfig) if err != nil { return err } @@ -122,7 +111,7 @@ func (s *containerRouter) getContainersLogs(ctx context.Context, w http.Response // this is the point of no return for writing a response. once we call // WriteLogStream, the response has been started and errors will be // returned in band by WriteLogStream - httputils.WriteLogStream(ctx, w, msgs, logsConfig, !container.Config.Tty) + httputils.WriteLogStream(ctx, w, msgs, logsConfig, !tty) return nil } @@ -130,6 +119,14 @@ func (s *containerRouter) getContainersExport(ctx context.Context, w http.Respon return s.backend.ContainerExport(vars["name"], w) } +type bodyOnStartError struct{} + +func (bodyOnStartError) Error() string { + return "starting container with non-empty request body was deprecated since API v1.22 and removed in v1.24" +} + +func (bodyOnStartError) InvalidParameter() {} + func (s *containerRouter) postContainersStart(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { // If contentLength is -1, we can assumed chunked encoding // or more technically that the length is unknown @@ -143,7 +140,7 @@ func (s *containerRouter) postContainersStart(ctx context.Context, w http.Respon // A non-nil json object is at least 7 characters. if r.ContentLength > 7 || r.ContentLength == -1 { if versions.GreaterThanOrEqualTo(version, "1.24") { - return validationError{fmt.Errorf("starting container with non-empty request body was deprecated since v1.10 and removed in v1.12")} + return bodyOnStartError{} } if err := httputils.CheckForJSON(r); err != nil { @@ -193,10 +190,6 @@ func (s *containerRouter) postContainersStop(ctx context.Context, w http.Respons return nil } -type errContainerIsRunning interface { - ContainerIsRunning() bool -} - func (s *containerRouter) postContainersKill(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := httputils.ParseForm(r); err != nil { return err @@ -209,14 +202,14 @@ func (s *containerRouter) postContainersKill(ctx context.Context, w http.Respons if sigStr := r.Form.Get("signal"); sigStr != "" { var err error if sig, err = signal.ParseSignal(sigStr); err != nil { - return err + return validationError{err} } } if err := s.backend.ContainerKill(name, uint64(sig)); err != nil { var isStopped bool - if e, ok := err.(errContainerIsRunning); ok { - isStopped = !e.ContainerIsRunning() + if errdefs.IsConflict(err) { + isStopped = true } // Return error that's not caused because the container is stopped. @@ -224,7 +217,7 @@ func (s *containerRouter) postContainersKill(ctx context.Context, w http.Respons // to keep backwards compatibility. version := httputils.VersionFromContext(ctx) if versions.GreaterThanOrEqualTo(version, "1.20") || !isStopped { - return fmt.Errorf("Cannot kill container %s: %v", name, err) + return errors.Wrapf(err, "Cannot kill container: %s", name) } } @@ -458,11 +451,11 @@ func (s *containerRouter) postContainersResize(ctx context.Context, w http.Respo height, err := strconv.Atoi(r.Form.Get("h")) if err != nil { - return err + return validationError{err} } width, err := strconv.Atoi(r.Form.Get("w")) if err != nil { - return err + return validationError{err} } return s.backend.ContainerResize(vars["name"], height, width) @@ -480,7 +473,7 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo hijacker, ok := w.(http.Hijacker) if !ok { - return fmt.Errorf("error attaching to container %s, hijack connection missing", containerName) + return validationError{errors.Errorf("error attaching to container %s, hijack connection missing", containerName)} } setupStreams := func() (io.ReadCloser, io.Writer, io.Writer, error) { @@ -597,7 +590,7 @@ func (s *containerRouter) postContainersPrune(ctx context.Context, w http.Respon pruneFilters, err := filters.FromParam(r.Form.Get("filters")) if err != nil { - return err + return validationError{err} } pruneReport, err := s.backend.ContainersPrune(ctx, pruneFilters) diff --git a/api/server/router/container/copy.go b/api/server/router/container/copy.go index 5cfe8d7ba1..356775f4d0 100644 --- a/api/server/router/container/copy.go +++ b/api/server/router/container/copy.go @@ -3,11 +3,8 @@ package container import ( "encoding/base64" "encoding/json" - "fmt" "io" "net/http" - "os" - "strings" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" @@ -15,6 +12,14 @@ import ( "golang.org/x/net/context" ) +type pathError struct{} + +func (pathError) Error() string { + return "Path cannot be empty" +} + +func (pathError) InvalidParameter() {} + // postContainersCopy is deprecated in favor of getContainersArchive. func (s *containerRouter) postContainersCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { // Deprecated since 1.8, Errors out since 1.12 @@ -33,18 +38,11 @@ func (s *containerRouter) postContainersCopy(ctx context.Context, w http.Respons } if cfg.Resource == "" { - return fmt.Errorf("Path cannot be empty") + return pathError{} } data, err := s.backend.ContainerCopy(vars["name"], cfg.Resource) if err != nil { - if strings.Contains(strings.ToLower(err.Error()), "no such container") { - w.WriteHeader(http.StatusNotFound) - return nil - } - if os.IsNotExist(err) { - return fmt.Errorf("Could not find the file %s in container %s", cfg.Resource, vars["name"]) - } return err } defer data.Close() diff --git a/api/server/router/container/exec.go b/api/server/router/container/exec.go index 64629b69ae..aa2ebb187b 100644 --- a/api/server/router/container/exec.go +++ b/api/server/router/container/exec.go @@ -24,6 +24,14 @@ func (s *containerRouter) getExecByID(ctx context.Context, w http.ResponseWriter return httputils.WriteJSON(w, http.StatusOK, eConfig) } +type execCommandError struct{} + +func (execCommandError) Error() string { + return "No exec command specified" +} + +func (execCommandError) InvalidParameter() {} + func (s *containerRouter) postContainerExecCreate(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := httputils.ParseForm(r); err != nil { return err @@ -39,7 +47,7 @@ func (s *containerRouter) postContainerExecCreate(ctx context.Context, w http.Re } if len(execConfig.Cmd) == 0 { - return fmt.Errorf("No exec command specified") + return execCommandError{} } // Register an instance of Exec in container. @@ -129,11 +137,11 @@ func (s *containerRouter) postContainerExecResize(ctx context.Context, w http.Re } height, err := strconv.Atoi(r.Form.Get("h")) if err != nil { - return err + return validationError{err} } width, err := strconv.Atoi(r.Form.Get("w")) if err != nil { - return err + return validationError{err} } return s.backend.ContainerExecResize(vars["name"], height, width) diff --git a/api/server/router/experimental.go b/api/server/router/experimental.go index ac31f04877..4045de5433 100644 --- a/api/server/router/experimental.go +++ b/api/server/router/experimental.go @@ -1,19 +1,13 @@ package router import ( - "errors" "net/http" "golang.org/x/net/context" - apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/server/httputils" ) -var ( - errExperimentalFeature = errors.New("This experimental feature is disabled by default. Start the Docker daemon in experimental mode in order to enable it.") -) - // ExperimentalRoute defines an experimental API route that can be enabled or disabled. type ExperimentalRoute interface { Route @@ -39,8 +33,16 @@ func (r *experimentalRoute) Disable() { r.handler = experimentalHandler } +type notImplementedError struct{} + +func (notImplementedError) Error() string { + return "This experimental feature is disabled by default. Start the Docker daemon in experimental mode in order to enable it." +} + +func (notImplementedError) NotImplemented() {} + func experimentalHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - return apierrors.NewErrorWithStatusCode(errExperimentalFeature, http.StatusNotImplemented) + return notImplementedError{} } // Handler returns returns the APIFunc to let the server wrap it in middlewares. diff --git a/api/server/router/image/image_routes.go b/api/server/router/image/image_routes.go index 9b99a585f3..eb1394a780 100644 --- a/api/server/router/image/image_routes.go +++ b/api/server/router/image/image_routes.go @@ -3,7 +3,6 @@ package image import ( "encoding/base64" "encoding/json" - "fmt" "io" "net/http" "runtime" @@ -20,6 +19,7 @@ import ( "github.com/docker/docker/pkg/streamformatter" "github.com/docker/docker/pkg/system" "github.com/docker/docker/registry" + "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -161,6 +161,20 @@ func (s *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrite return nil } +type validationError struct { + cause error +} + +func (e validationError) Error() string { + return e.cause.Error() +} + +func (e validationError) Cause() error { + return e.cause +} + +func (validationError) InvalidParameter() {} + func (s *imageRouter) postImagesPush(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { metaHeaders := map[string][]string{} for k, v := range r.Header { @@ -184,7 +198,7 @@ func (s *imageRouter) postImagesPush(ctx context.Context, w http.ResponseWriter, } else { // the old format is supported for compatibility if there was no authConfig header if err := json.NewDecoder(r.Body).Decode(authConfig); err != nil { - return fmt.Errorf("Bad parameters and missing X-Registry-Auth: %v", err) + return errors.Wrap(validationError{err}, "Bad parameters and missing X-Registry-Auth") } } @@ -246,6 +260,14 @@ func (s *imageRouter) postImagesLoad(ctx context.Context, w http.ResponseWriter, return nil } +type missingImageError struct{} + +func (missingImageError) Error() string { + return "image name cannot be blank" +} + +func (missingImageError) InvalidParameter() {} + func (s *imageRouter) deleteImages(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := httputils.ParseForm(r); err != nil { return err @@ -254,7 +276,7 @@ func (s *imageRouter) deleteImages(ctx context.Context, w http.ResponseWriter, r name := vars["name"] if strings.TrimSpace(name) == "" { - return fmt.Errorf("image name cannot be blank") + return missingImageError{} } force := httputils.BoolValue(r, "force") diff --git a/api/server/router/network/filter.go b/api/server/router/network/filter.go index afe4e235e3..21fc828b8b 100644 --- a/api/server/router/network/filter.go +++ b/api/server/router/network/filter.go @@ -1,8 +1,6 @@ package network import ( - "fmt" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/runconfig" @@ -24,11 +22,19 @@ func filterNetworkByType(nws []types.NetworkResource, netType string) ([]types.N } } default: - return nil, fmt.Errorf("Invalid filter: 'type'='%s'", netType) + return nil, invalidFilter(netType) } return retNws, nil } +type invalidFilter string + +func (e invalidFilter) Error() string { + return "Invalid filter: 'type'='" + string(e) + "'" +} + +func (e invalidFilter) InvalidParameter() {} + // filterNetworks filters network list according to user specified filter // and returns user chosen networks func filterNetworks(nws []types.NetworkResource, filter filters.Args) ([]types.NetworkResource, error) { diff --git a/api/server/router/network/network_routes.go b/api/server/router/network/network_routes.go index f439d65ad9..ad3e74e6eb 100644 --- a/api/server/router/network/network_routes.go +++ b/api/server/router/network/network_routes.go @@ -2,14 +2,12 @@ package network import ( "encoding/json" - "fmt" "net/http" "strconv" "strings" "golang.org/x/net/context" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" @@ -18,6 +16,7 @@ import ( "github.com/docker/libnetwork" netconst "github.com/docker/libnetwork/datastore" "github.com/docker/libnetwork/networkdb" + "github.com/pkg/errors" ) var ( @@ -83,6 +82,24 @@ SKIP: return httputils.WriteJSON(w, http.StatusOK, list) } +type invalidRequestError struct { + cause error +} + +func (e invalidRequestError) Error() string { + return e.cause.Error() +} + +func (e invalidRequestError) InvalidParameter() {} + +type ambigousResultsError string + +func (e ambigousResultsError) Error() string { + return "network " + string(e) + " is ambiguous" +} + +func (ambigousResultsError) InvalidParameter() {} + func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := httputils.ParseForm(r); err != nil { return err @@ -95,8 +112,7 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r ) if v := r.URL.Query().Get("verbose"); v != "" { if verbose, err = strconv.ParseBool(v); err != nil { - err = fmt.Errorf("invalid value for verbose: %s", v) - return errors.NewBadRequestError(err) + return errors.Wrapf(invalidRequestError{err}, "invalid value for verbose: %s", v) } } scope := r.URL.Query().Get("scope") @@ -177,7 +193,7 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r } } if len(listByFullName) > 1 { - return fmt.Errorf("network %s is ambiguous (%d matches found based on name)", term, len(listByFullName)) + return errors.Wrapf(ambigousResultsError(term), "%d matches found based on name", len(listByFullName)) } // Find based on partial ID, returns true only if no duplicates @@ -187,7 +203,7 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r } } if len(listByPartialID) > 1 { - return fmt.Errorf("network %s is ambiguous (%d matches found based on ID prefix)", term, len(listByPartialID)) + return errors.Wrapf(ambigousResultsError(term), "%d matches found based on ID prefix", len(listByPartialID)) } return libnetwork.ErrNoSuchNetwork(term) diff --git a/api/server/router/session/session_routes.go b/api/server/router/session/session_routes.go index ef9753c6ea..bccd764226 100644 --- a/api/server/router/session/session_routes.go +++ b/api/server/router/session/session_routes.go @@ -3,14 +3,27 @@ package session import ( "net/http" - apierrors "github.com/docker/docker/api/errors" "golang.org/x/net/context" ) +type invalidRequest struct { + cause error +} + +func (e invalidRequest) Error() string { + return e.cause.Error() +} + +func (e invalidRequest) Cause() error { + return e.cause +} + +func (e invalidRequest) InvalidParameter() {} + func (sr *sessionRouter) startSession(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { err := sr.backend.HandleHTTPRequest(ctx, w, r) if err != nil { - return apierrors.NewBadRequestError(err) + return invalidRequest{err} } return nil } diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go index a1e18f5fad..7bd3aff182 100644 --- a/api/server/router/swarm/cluster_routes.go +++ b/api/server/router/swarm/cluster_routes.go @@ -6,13 +6,13 @@ import ( "net/http" "strconv" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/server/httputils" basictypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/filters" types "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/api/types/versions" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -57,6 +57,20 @@ func (sr *swarmRouter) inspectCluster(ctx context.Context, w http.ResponseWriter return httputils.WriteJSON(w, http.StatusOK, swarm) } +type invalidRequestError struct { + err error +} + +func (e invalidRequestError) Error() string { + return e.err.Error() +} + +func (e invalidRequestError) Cause() error { + return e.err +} + +func (e invalidRequestError) InvalidParameter() {} + func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { var swarm types.Spec if err := json.NewDecoder(r.Body).Decode(&swarm); err != nil { @@ -67,7 +81,7 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { err := fmt.Errorf("invalid swarm version '%s': %v", rawVersion, err) - return errors.NewBadRequestError(err) + return invalidRequestError{err} } var flags types.UpdateFlags @@ -76,7 +90,7 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, rot, err := strconv.ParseBool(value) if err != nil { err := fmt.Errorf("invalid value for rotateWorkerToken: %s", value) - return errors.NewBadRequestError(err) + return invalidRequestError{err} } flags.RotateWorkerToken = rot @@ -86,7 +100,7 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, rot, err := strconv.ParseBool(value) if err != nil { err := fmt.Errorf("invalid value for rotateManagerToken: %s", value) - return errors.NewBadRequestError(err) + return invalidRequestError{err} } flags.RotateManagerToken = rot @@ -95,7 +109,7 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, if value := r.URL.Query().Get("rotateManagerUnlockKey"); value != "" { rot, err := strconv.ParseBool(value) if err != nil { - return errors.NewBadRequestError(fmt.Errorf("invalid value for rotateManagerUnlockKey: %s", value)) + return invalidRequestError{fmt.Errorf("invalid value for rotateManagerUnlockKey: %s", value)} } flags.RotateManagerUnlockKey = rot @@ -139,7 +153,7 @@ func (sr *swarmRouter) getServices(ctx context.Context, w http.ResponseWriter, r } filter, err := filters.FromParam(r.Form.Get("filters")) if err != nil { - return err + return invalidRequestError{err} } services, err := sr.backend.GetServices(basictypes.ServiceListOptions{Filters: filter}) @@ -158,7 +172,7 @@ func (sr *swarmRouter) getService(ctx context.Context, w http.ResponseWriter, r insertDefaults, err = strconv.ParseBool(value) if err != nil { err := fmt.Errorf("invalid value for insertDefaults: %s", value) - return errors.NewBadRequestError(err) + return errors.Wrapf(invalidRequestError{err}, "invalid value for insertDefaults: %s", value) } } @@ -204,7 +218,7 @@ func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter, version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { err := fmt.Errorf("invalid service version '%s': %v", rawVersion, err) - return errors.NewBadRequestError(err) + return invalidRequestError{err} } var flags basictypes.ServiceUpdateOptions @@ -297,7 +311,7 @@ func (sr *swarmRouter) updateNode(ctx context.Context, w http.ResponseWriter, r version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { err := fmt.Errorf("invalid node version '%s': %v", rawVersion, err) - return errors.NewBadRequestError(err) + return invalidRequestError{err} } if err := sr.backend.UpdateNode(vars["id"], version, node); err != nil { @@ -403,13 +417,13 @@ func (sr *swarmRouter) getSecret(ctx context.Context, w http.ResponseWriter, r * func (sr *swarmRouter) updateSecret(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { var secret types.SecretSpec if err := json.NewDecoder(r.Body).Decode(&secret); err != nil { - return errors.NewBadRequestError(err) + return invalidRequestError{err} } rawVersion := r.URL.Query().Get("version") version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { - return errors.NewBadRequestError(fmt.Errorf("invalid secret version")) + return invalidRequestError{fmt.Errorf("invalid secret version")} } id := vars["id"] @@ -474,13 +488,13 @@ func (sr *swarmRouter) getConfig(ctx context.Context, w http.ResponseWriter, r * func (sr *swarmRouter) updateConfig(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { var config types.ConfigSpec if err := json.NewDecoder(r.Body).Decode(&config); err != nil { - return errors.NewBadRequestError(err) + return invalidRequestError{err} } rawVersion := r.URL.Query().Get("version") version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { - return errors.NewBadRequestError(fmt.Errorf("invalid config version")) + return invalidRequestError{fmt.Errorf("invalid config version")} } id := vars["id"] diff --git a/api/server/router/system/system_routes.go b/api/server/router/system/system_routes.go index cd8c3a9fb8..5884388ebe 100644 --- a/api/server/router/system/system_routes.go +++ b/api/server/router/system/system_routes.go @@ -7,7 +7,6 @@ import ( "time" "github.com/docker/docker/api" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/events" @@ -85,6 +84,16 @@ func (s *systemRouter) getDiskUsage(ctx context.Context, w http.ResponseWriter, return httputils.WriteJSON(w, http.StatusOK, du) } +type invalidRequestError struct { + Err error +} + +func (e invalidRequestError) Error() string { + return e.Err.Error() +} + +func (e invalidRequestError) InvalidParameter() {} + func (s *systemRouter) getEvents(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := httputils.ParseForm(r); err != nil { return err @@ -105,7 +114,7 @@ func (s *systemRouter) getEvents(ctx context.Context, w http.ResponseWriter, r * ) if !until.IsZero() { if until.Before(since) { - return errors.NewBadRequestError(fmt.Errorf("`since` time (%s) cannot be after `until` time (%s)", r.Form.Get("since"), r.Form.Get("until"))) + return invalidRequestError{fmt.Errorf("`since` time (%s) cannot be after `until` time (%s)", r.Form.Get("since"), r.Form.Get("until"))} } now := time.Now() diff --git a/api/server/server.go b/api/server/server.go index 55edcec335..7ba8a6ce39 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -2,12 +2,10 @@ package server import ( "crypto/tls" - "fmt" "net" "net/http" "strings" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/server/middleware" "github.com/docker/docker/api/server/router" @@ -158,6 +156,14 @@ func (s *Server) InitRouter(routers ...router.Router) { } } +type pageNotFoundError struct{} + +func (pageNotFoundError) Error() string { + return "page not found" +} + +func (pageNotFoundError) NotFound() {} + // createMux initializes the main router the server uses. func (s *Server) createMux() *mux.Router { m := mux.NewRouter() @@ -180,8 +186,7 @@ func (s *Server) createMux() *mux.Router { m.Path("/debug" + r.Path()).Handler(f) } - err := errors.NewRequestNotFoundError(fmt.Errorf("page not found")) - notFoundHandler := httputils.MakeErrorHandler(err) + notFoundHandler := httputils.MakeErrorHandler(pageNotFoundError{}) m.HandleFunc(versionMatcher+"/{path:.*}", notFoundHandler) m.NotFoundHandler = notFoundHandler diff --git a/api/swagger.yaml b/api/swagger.yaml index 2ce4240956..451cc5e6c0 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -3311,10 +3311,6 @@ paths: examples: application/json: message: "No such container: c2ada9df5af8" - 406: - description: "impossible to attach" - schema: - $ref: "#/definitions/ErrorResponse" 409: description: "conflict" schema: diff --git a/api/types/filters/parse.go b/api/types/filters/parse.go index beec3d4940..411eafd6de 100644 --- a/api/types/filters/parse.go +++ b/api/types/filters/parse.go @@ -5,7 +5,6 @@ package filters import ( "encoding/json" "errors" - "fmt" "regexp" "strings" @@ -258,17 +257,33 @@ func (filters Args) Include(field string) bool { return ok } +type invalidFilter string + +func (e invalidFilter) Error() string { + return "Invalid filter '" + string(e) + "'" +} + +func (invalidFilter) InvalidParameter() {} + // Validate ensures that all the fields in the filter are valid. // It returns an error as soon as it finds an invalid field. func (filters Args) Validate(accepted map[string]bool) error { for name := range filters.fields { if !accepted[name] { - return fmt.Errorf("Invalid filter '%s'", name) + return invalidFilter(name) } } return nil } +type invalidFilterError string + +func (e invalidFilterError) Error() string { + return "Invalid filter: '" + string(e) + "'" +} + +func (invalidFilterError) InvalidParameter() {} + // WalkValues iterates over the list of filtered values for a field. // It stops the iteration if it finds an error and it returns that error. func (filters Args) WalkValues(field string, op func(value string) error) error { diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 1d7fa413f8..34db3786f1 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -241,7 +241,7 @@ func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*buil if err := checkDispatchDockerfile(dockerfile.AST); err != nil { buildsFailed.WithValues(metricsDockerfileSyntaxError).Inc() - return nil, err + return nil, validationError{err} } dispatchState, err := b.dispatchDockerfileWithCancellation(dockerfile, source) @@ -357,7 +357,7 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con dockerfile, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n"))) if err != nil { - return nil, err + return nil, validationError{err} } // TODO @jhowardmsft LCOW support. For now, if LCOW enabled, switch to linux. @@ -374,7 +374,7 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con // ensure that the commands are valid for _, n := range dockerfile.AST.Children { if !validCommitCommands[n.Value] { - return nil, fmt.Errorf("%s is not a valid change command", n.Value) + return nil, validationError{errors.Errorf("%s is not a valid change command", n.Value)} } } @@ -383,7 +383,7 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con b.disableCommit = true if err := checkDispatchDockerfile(dockerfile.AST); err != nil { - return nil, err + return nil, validationError{err} } dispatchState := newDispatchState() dispatchState.runConfig = config diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 24a0a1c4df..fdc28bc7cd 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -784,7 +784,7 @@ func stopSignal(req dispatchRequest) error { sig := req.args[0] _, err := signal.ParseSignal(sig) if err != nil { - return err + return validationError{err} } req.state.runConfig.StopSignal = sig diff --git a/builder/dockerfile/errors.go b/builder/dockerfile/errors.go new file mode 100644 index 0000000000..25664dee78 --- /dev/null +++ b/builder/dockerfile/errors.go @@ -0,0 +1,15 @@ +package dockerfile + +type validationError struct { + err error +} + +func (e validationError) Error() string { + return e.err.Error() +} + +func (e validationError) InvalidParameter() {} + +func (e validationError) Cause() error { + return e.err +} diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index ba4315940a..a3ca201e75 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -139,7 +139,7 @@ func (b *Builder) dispatch(options dispatchOptions) (*dispatchState, error) { // on which the daemon is running does not support a builder command. if err := platformSupports(strings.ToLower(cmd)); err != nil { buildsFailed.WithValues(metricsCommandNotSupportedError).Inc() - return nil, err + return nil, validationError{err} } msg := bytes.NewBufferString(fmt.Sprintf("Step %s : %s%s", @@ -151,7 +151,7 @@ func (b *Builder) dispatch(options dispatchOptions) (*dispatchState, error) { var err error ast, args, err = handleOnBuildNode(node, msg) if err != nil { - return nil, err + return nil, validationError{err} } } @@ -161,7 +161,7 @@ func (b *Builder) dispatch(options dispatchOptions) (*dispatchState, error) { words, err := getDispatchArgsFromNode(ast, processFunc, msg) if err != nil { buildsFailed.WithValues(metricsErrorProcessingCommandsError).Inc() - return nil, err + return nil, validationError{err} } args = append(args, words...) @@ -170,7 +170,7 @@ func (b *Builder) dispatch(options dispatchOptions) (*dispatchState, error) { f, ok := evaluateTable[cmd] if !ok { buildsFailed.WithValues(metricsUnknownInstructionError).Inc() - return nil, fmt.Errorf("unknown instruction: %s", upperCasedCmd) + return nil, validationError{errors.Errorf("unknown instruction: %s", upperCasedCmd)} } options.state.updateRunConfig() err = f(newDispatchRequestFromOptions(options, b, args)) @@ -247,7 +247,7 @@ func (s *dispatchState) setDefaultPath() { func handleOnBuildNode(ast *parser.Node, msg *bytes.Buffer) (*parser.Node, []string, error) { if ast.Next == nil { - return nil, nil, errors.New("ONBUILD requires at least one argument") + return nil, nil, validationError{errors.New("ONBUILD requires at least one argument")} } ast = ast.Next.Children[0] msg.WriteString(" " + ast.Value + formatFlags(ast.Flags)) diff --git a/builder/remotecontext/errors.go b/builder/remotecontext/errors.go new file mode 100644 index 0000000000..8ee33bc607 --- /dev/null +++ b/builder/remotecontext/errors.go @@ -0,0 +1,75 @@ +package remotecontext + +type notFoundError string + +func (e notFoundError) Error() string { + return string(e) +} + +func (notFoundError) NotFound() {} + +type requestError string + +func (e requestError) Error() string { + return string(e) +} + +func (e requestError) InvalidParameter() {} + +type unauthorizedError string + +func (e unauthorizedError) Error() string { + return string(e) +} + +func (unauthorizedError) Unauthorized() {} + +type forbiddenError string + +func (e forbiddenError) Error() string { + return string(e) +} + +func (forbiddenError) Forbidden() {} + +type dnsError struct { + cause error +} + +func (e dnsError) Error() string { + return e.cause.Error() +} + +func (e dnsError) NotFound() {} + +func (e dnsError) Cause() error { + return e.cause +} + +type systemError struct { + cause error +} + +func (e systemError) Error() string { + return e.cause.Error() +} + +func (e systemError) SystemError() {} + +func (e systemError) Cause() error { + return e.cause +} + +type unknownError struct { + cause error +} + +func (e unknownError) Error() string { + return e.cause.Error() +} + +func (unknownError) Unknown() {} + +func (e unknownError) Cause() error { + return e.cause +} diff --git a/builder/remotecontext/remote.go b/builder/remotecontext/remote.go index 4afd516be5..706eefd7e4 100644 --- a/builder/remotecontext/remote.go +++ b/builder/remotecontext/remote.go @@ -5,7 +5,9 @@ import ( "fmt" "io" "io/ioutil" + "net" "net/http" + "net/url" "regexp" "github.com/docker/docker/builder" @@ -68,20 +70,37 @@ func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io. // GetWithStatusError does an http.Get() and returns an error if the // status code is 4xx or 5xx. -func GetWithStatusError(url string) (resp *http.Response, err error) { - if resp, err = http.Get(url); err != nil { - return nil, err +func GetWithStatusError(address string) (resp *http.Response, err error) { + if resp, err = http.Get(address); err != nil { + if uerr, ok := err.(*url.Error); ok { + if derr, ok := uerr.Err.(*net.DNSError); ok && !derr.IsTimeout { + return nil, dnsError{err} + } + } + return nil, systemError{err} } if resp.StatusCode < 400 { return resp, nil } - msg := fmt.Sprintf("failed to GET %s with status %s", url, resp.Status) + msg := fmt.Sprintf("failed to GET %s with status %s", address, resp.Status) body, err := ioutil.ReadAll(resp.Body) resp.Body.Close() if err != nil { - return nil, errors.Wrapf(err, msg+": error reading body") + return nil, errors.Wrap(systemError{err}, msg+": error reading body") } - return nil, errors.Errorf(msg+": %s", bytes.TrimSpace(body)) + + msg += ": " + string(bytes.TrimSpace(body)) + switch resp.StatusCode { + case http.StatusNotFound: + return nil, notFoundError(msg) + case http.StatusBadRequest: + return nil, requestError(msg) + case http.StatusUnauthorized: + return nil, unauthorizedError(msg) + case http.StatusForbidden: + return nil, forbiddenError(msg) + } + return nil, unknownError{errors.New(msg)} } // inspectResponse looks into the http response data at r to determine whether its diff --git a/container/container.go b/container/container.go index 19079adbc7..0713ec6be2 100644 --- a/container/container.go +++ b/container/container.go @@ -43,6 +43,7 @@ import ( "github.com/docker/libnetwork/options" "github.com/docker/libnetwork/types" agentexec "github.com/docker/swarmkit/agent/exec" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -55,8 +56,8 @@ const ( ) var ( - errInvalidEndpoint = fmt.Errorf("invalid endpoint while building port map info") - errInvalidNetwork = fmt.Errorf("invalid network settings while building port map info") + errInvalidEndpoint = errors.New("invalid endpoint while building port map info") + errInvalidNetwork = errors.New("invalid network settings while building port map info") ) // Container holds the structure defining a container object. @@ -274,7 +275,7 @@ func (container *Container) SetupWorkingDirectory(rootIDs idtools.IDPair) error if err := idtools.MkdirAllAndChownNew(pth, 0755, rootIDs); err != nil { pthInfo, err2 := os.Stat(pth) if err2 == nil && pthInfo != nil && !pthInfo.IsDir() { - return fmt.Errorf("Cannot mkdir: %s is not a directory", container.Config.WorkingDir) + return errors.Errorf("Cannot mkdir: %s is not a directory", container.Config.WorkingDir) } return err @@ -357,7 +358,7 @@ func (container *Container) StartLogger() (logger.Logger, error) { cfg := container.HostConfig.LogConfig initDriver, err := logger.GetLogDriver(cfg.Type) if err != nil { - return nil, fmt.Errorf("failed to get logging factory: %v", err) + return nil, errors.Wrap(err, "failed to get logging factory") } info := logger.Info{ Config: cfg.Config, @@ -723,18 +724,18 @@ func (container *Container) BuildCreateEndpointOptions(n libnetwork.Network, epC for _, ips := range ipam.LinkLocalIPs { if linkip = net.ParseIP(ips); linkip == nil && ips != "" { - return nil, fmt.Errorf("Invalid link-local IP address:%s", ipam.LinkLocalIPs) + return nil, errors.Errorf("Invalid link-local IP address: %s", ipam.LinkLocalIPs) } ipList = append(ipList, linkip) } if ip = net.ParseIP(ipam.IPv4Address); ip == nil && ipam.IPv4Address != "" { - return nil, fmt.Errorf("Invalid IPv4 address:%s)", ipam.IPv4Address) + return nil, errors.Errorf("Invalid IPv4 address: %s)", ipam.IPv4Address) } if ip6 = net.ParseIP(ipam.IPv6Address); ip6 == nil && ipam.IPv6Address != "" { - return nil, fmt.Errorf("Invalid IPv6 address:%s)", ipam.IPv6Address) + return nil, errors.Errorf("Invalid IPv6 address: %s)", ipam.IPv6Address) } createOptions = append(createOptions, @@ -838,7 +839,7 @@ func (container *Container) BuildCreateEndpointOptions(n libnetwork.Network, epC portStart, portEnd, err = newP.Range() } if err != nil { - return nil, fmt.Errorf("Error parsing HostPort value(%s):%v", binding[i].HostPort, err) + return nil, errors.Wrapf(err, "Error parsing HostPort value (%s)", binding[i].HostPort) } pbCopy.HostPort = uint16(portStart) pbCopy.HostPortEnd = uint16(portEnd) diff --git a/container/container_unix.go b/container/container_unix.go index e7a27d212c..8212cb9d7c 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -3,7 +3,6 @@ package container import ( - "fmt" "io/ioutil" "os" "path/filepath" @@ -262,6 +261,14 @@ func (container *Container) ConfigMounts() []Mount { return mounts } +type conflictingUpdateOptions string + +func (e conflictingUpdateOptions) Error() string { + return string(e) +} + +func (e conflictingUpdateOptions) Conflict() {} + // UpdateContainer updates configuration of a container. Callers must hold a Lock on the Container. func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfig) error { // update resources of container @@ -273,16 +280,16 @@ func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfi // once NanoCPU is already set, updating CPUPeriod/CPUQuota will be blocked, and vice versa. // In the following we make sure the intended update (resources) does not conflict with the existing (cResource). if resources.NanoCPUs > 0 && cResources.CPUPeriod > 0 { - return fmt.Errorf("Conflicting options: Nano CPUs cannot be updated as CPU Period has already been set") + return conflictingUpdateOptions("Conflicting options: Nano CPUs cannot be updated as CPU Period has already been set") } if resources.NanoCPUs > 0 && cResources.CPUQuota > 0 { - return fmt.Errorf("Conflicting options: Nano CPUs cannot be updated as CPU Quota has already been set") + return conflictingUpdateOptions("Conflicting options: Nano CPUs cannot be updated as CPU Quota has already been set") } if resources.CPUPeriod > 0 && cResources.NanoCPUs > 0 { - return fmt.Errorf("Conflicting options: CPU Period cannot be updated as NanoCPUs has already been set") + return conflictingUpdateOptions("Conflicting options: CPU Period cannot be updated as NanoCPUs has already been set") } if resources.CPUQuota > 0 && cResources.NanoCPUs > 0 { - return fmt.Errorf("Conflicting options: CPU Quota cannot be updated as NanoCPUs has already been set") + return conflictingUpdateOptions("Conflicting options: CPU Quota cannot be updated as NanoCPUs has already been set") } if resources.BlkioWeight != 0 { @@ -310,7 +317,7 @@ func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfi // if memory limit smaller than already set memoryswap limit and doesn't // update the memoryswap limit, then error out. if resources.Memory > cResources.MemorySwap && resources.MemorySwap == 0 { - return fmt.Errorf("Memory limit should be smaller than already set memoryswap limit, update the memoryswap at the same time") + return conflictingUpdateOptions("Memory limit should be smaller than already set memoryswap limit, update the memoryswap at the same time") } cResources.Memory = resources.Memory } @@ -327,7 +334,7 @@ func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfi // update HostConfig of container if hostConfig.RestartPolicy.Name != "" { if container.HostConfig.AutoRemove && !hostConfig.RestartPolicy.IsNone() { - return fmt.Errorf("Restart policy cannot be updated because AutoRemove is enabled for the container") + return conflictingUpdateOptions("Restart policy cannot be updated because AutoRemove is enabled for the container") } container.HostConfig.RestartPolicy = hostConfig.RestartPolicy } diff --git a/daemon/archive.go b/daemon/archive.go index bd00daca53..4bcf8d0a0c 100644 --- a/daemon/archive.go +++ b/daemon/archive.go @@ -28,16 +28,20 @@ func (daemon *Daemon) ContainerCopy(name string, res string) (io.ReadCloser, err return nil, err } - if res[0] == '/' || res[0] == '\\' { - res = res[1:] - } - // Make sure an online file-system operation is permitted. if err := daemon.isOnlineFSOperationPermitted(container); err != nil { - return nil, err + return nil, systemError{err} } - return daemon.containerCopy(container, res) + data, err := daemon.containerCopy(container, res) + if err == nil { + return data, nil + } + + if os.IsNotExist(err) { + return nil, containerFileNotFound{res, name} + } + return nil, systemError{err} } // ContainerStatPath stats the filesystem resource at the specified path in the @@ -50,10 +54,18 @@ func (daemon *Daemon) ContainerStatPath(name string, path string) (stat *types.C // Make sure an online file-system operation is permitted. if err := daemon.isOnlineFSOperationPermitted(container); err != nil { - return nil, err + return nil, systemError{err} } - return daemon.containerStatPath(container, path) + stat, err = daemon.containerStatPath(container, path) + if err == nil { + return stat, nil + } + + if os.IsNotExist(err) { + return nil, containerFileNotFound{path, name} + } + return nil, systemError{err} } // ContainerArchivePath creates an archive of the filesystem resource at the @@ -67,10 +79,18 @@ func (daemon *Daemon) ContainerArchivePath(name string, path string) (content io // Make sure an online file-system operation is permitted. if err := daemon.isOnlineFSOperationPermitted(container); err != nil { - return nil, nil, err + return nil, nil, systemError{err} } - return daemon.containerArchivePath(container, path) + content, stat, err = daemon.containerArchivePath(container, path) + if err == nil { + return content, stat, nil + } + + if os.IsNotExist(err) { + return nil, nil, containerFileNotFound{path, name} + } + return nil, nil, systemError{err} } // ContainerExtractToDir extracts the given archive to the specified location @@ -87,10 +107,18 @@ func (daemon *Daemon) ContainerExtractToDir(name, path string, copyUIDGID, noOve // Make sure an online file-system operation is permitted. if err := daemon.isOnlineFSOperationPermitted(container); err != nil { - return err + return systemError{err} } - return daemon.containerExtractToDir(container, path, copyUIDGID, noOverwriteDirNonDir, content) + err = daemon.containerExtractToDir(container, path, copyUIDGID, noOverwriteDirNonDir, content) + if err == nil { + return nil + } + + if os.IsNotExist(err) { + return containerFileNotFound{path, name} + } + return systemError{err} } // containerStatPath stats the filesystem resource at the specified path in this @@ -297,6 +325,9 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path } func (daemon *Daemon) containerCopy(container *container.Container, resource string) (rc io.ReadCloser, err error) { + if resource[0] == '/' || resource[0] == '\\' { + resource = resource[1:] + } container.Lock() defer func() { diff --git a/daemon/attach.go b/daemon/attach.go index 115c315764..ef1765af92 100644 --- a/daemon/attach.go +++ b/daemon/attach.go @@ -5,13 +5,13 @@ import ( "fmt" "io" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/container" "github.com/docker/docker/container/stream" "github.com/docker/docker/daemon/logger" "github.com/docker/docker/pkg/stdcopy" "github.com/docker/docker/pkg/term" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -22,7 +22,7 @@ func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerA if c.DetachKeys != "" { keys, err = term.ToBytes(c.DetachKeys) if err != nil { - return fmt.Errorf("Invalid detach keys (%s) provided", c.DetachKeys) + return validationError{errors.Errorf("Invalid detach keys (%s) provided", c.DetachKeys)} } } @@ -32,11 +32,11 @@ func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerA } if container.IsPaused() { err := fmt.Errorf("Container %s is paused, unpause the container before attach.", prefixOrName) - return errors.NewRequestConflictError(err) + return stateConflictError{err} } if container.IsRestarting() { err := fmt.Errorf("Container %s is restarting, wait until the container is running.", prefixOrName) - return errors.NewRequestConflictError(err) + return stateConflictError{err} } cfg := stream.AttachConfig{ @@ -119,7 +119,7 @@ func (daemon *Daemon) containerAttach(c *container.Container, cfg *stream.Attach } cLog, ok := logDriver.(logger.LogReader) if !ok { - return logger.ErrReadLogsNotSupported + return logger.ErrReadLogsNotSupported{} } logs := cLog.ReadLogs(logger.ReadConfig{Tail: -1}) defer logs.Close() diff --git a/daemon/cluster/cluster.go b/daemon/cluster/cluster.go index 55540fc06c..6a7db153b7 100644 --- a/daemon/cluster/cluster.go +++ b/daemon/cluster/cluster.go @@ -72,21 +72,6 @@ const ( contextPrefix = "com.docker.swarm" ) -// errNoSwarm is returned on leaving a cluster that was never initialized -var errNoSwarm = errors.New("This node is not part of a swarm") - -// errSwarmExists is returned on initialize or join request for a cluster that has already been activated -var errSwarmExists = errors.New("This node is already part of a swarm. Use \"docker swarm leave\" to leave this swarm and join another one.") - -// errSwarmJoinTimeoutReached is returned when cluster join could not complete before timeout was reached. -var errSwarmJoinTimeoutReached = errors.New("Timeout was reached before node was joined. The attempt to join the swarm will continue in the background. Use the \"docker info\" command to see the current swarm status of your node.") - -// errSwarmLocked is returned if the swarm is encrypted and needs a key to unlock it. -var errSwarmLocked = errors.New("Swarm is encrypted and needs to be unlocked before it can be used. Please use \"docker swarm unlock\" to unlock it.") - -// errSwarmCertificatesExpired is returned if docker was not started for the whole validity period and they had no chance to renew automatically. -var errSwarmCertificatesExpired = errors.New("Swarm certificates have expired. To replace them, leave the swarm and join again.") - // NetworkSubnetsProvider exposes functions for retrieving the subnets // of networks managed by Docker, so they can be filtered. type NetworkSubnetsProvider interface { @@ -343,12 +328,12 @@ func (c *Cluster) errNoManager(st nodeState) error { if st.err == errSwarmCertificatesExpired { return errSwarmCertificatesExpired } - return errors.New("This node is not a swarm manager. Use \"docker swarm init\" or \"docker swarm join\" to connect this node to swarm and try again.") + return errors.WithStack(notAvailableError("This node is not a swarm manager. Use \"docker swarm init\" or \"docker swarm join\" to connect this node to swarm and try again.")) } if st.swarmNode.Manager() != nil { - return errors.New("This node is not a swarm manager. Manager is being prepared or has trouble connecting to the cluster.") + return errors.WithStack(notAvailableError("This node is not a swarm manager. Manager is being prepared or has trouble connecting to the cluster.")) } - return errors.New("This node is not a swarm manager. Worker nodes can't be used to view or modify cluster state. Please run this command on a manager node or promote the current node to a manager.") + return errors.WithStack(notAvailableError("This node is not a swarm manager. Worker nodes can't be used to view or modify cluster state. Please run this command on a manager node or promote the current node to a manager.")) } // Cleanup stops active swarm node. This is run before daemon shutdown. diff --git a/daemon/cluster/controllers/plugin/controller.go b/daemon/cluster/controllers/plugin/controller.go index 8cc9b04e36..b48c9fd4e2 100644 --- a/daemon/cluster/controllers/plugin/controller.go +++ b/daemon/cluster/controllers/plugin/controller.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/docker/distribution/reference" + "github.com/docker/docker/api/errdefs" enginetypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm/runtime" "github.com/docker/docker/plugin" @@ -198,8 +199,7 @@ func (p *Controller) Wait(ctx context.Context) error { } func isNotFound(err error) bool { - _, ok := errors.Cause(err).(plugin.ErrNotFound) - return ok + return errdefs.IsNotFound(err) } // Shutdown is the shutdown phase from swarmkit diff --git a/daemon/cluster/errors.go b/daemon/cluster/errors.go new file mode 100644 index 0000000000..1698229427 --- /dev/null +++ b/daemon/cluster/errors.go @@ -0,0 +1,114 @@ +package cluster + +const ( + // errNoSwarm is returned on leaving a cluster that was never initialized + errNoSwarm notAvailableError = "This node is not part of a swarm" + + // errSwarmExists is returned on initialize or join request for a cluster that has already been activated + errSwarmExists notAvailableError = "This node is already part of a swarm. Use \"docker swarm leave\" to leave this swarm and join another one." + + // errSwarmJoinTimeoutReached is returned when cluster join could not complete before timeout was reached. + errSwarmJoinTimeoutReached notAvailableError = "Timeout was reached before node joined. The attempt to join the swarm will continue in the background. Use the \"docker info\" command to see the current swarm status of your node." + + // errSwarmLocked is returned if the swarm is encrypted and needs a key to unlock it. + errSwarmLocked notAvailableError = "Swarm is encrypted and needs to be unlocked before it can be used. Please use \"docker swarm unlock\" to unlock it." + + // errSwarmCertificatesExpired is returned if docker was not started for the whole validity period and they had no chance to renew automatically. + errSwarmCertificatesExpired notAvailableError = "Swarm certificates have expired. To replace them, leave the swarm and join again." +) + +type notFoundError struct { + cause error +} + +func (e notFoundError) Error() string { + return e.cause.Error() +} + +func (e notFoundError) NotFound() {} + +func (e notFoundError) Cause() error { + return e.cause +} + +type ambiguousResultsError struct { + cause error +} + +func (e ambiguousResultsError) Error() string { + return e.cause.Error() +} + +func (e ambiguousResultsError) InvalidParameter() {} + +func (e ambiguousResultsError) Cause() error { + return e.cause +} + +type convertError struct { + cause error +} + +func (e convertError) Error() string { + return e.cause.Error() +} + +func (e convertError) InvalidParameter() {} + +func (e convertError) Cause() error { + return e.cause +} + +type notAllowedError string + +func (e notAllowedError) Error() string { + return string(e) +} + +func (e notAllowedError) Forbidden() {} + +type validationError struct { + cause error +} + +func (e validationError) Error() string { + return e.cause.Error() +} + +func (e validationError) InvalidParameter() {} + +func (e validationError) Cause() error { + return e.cause +} + +type notAvailableError string + +func (e notAvailableError) Error() string { + return string(e) +} + +func (e notAvailableError) Unavailable() {} + +type configError string + +func (e configError) Error() string { + return string(e) +} + +func (e configError) InvalidParameter() {} + +type invalidUnlockKey struct{} + +func (invalidUnlockKey) Error() string { + return "swarm could not be unlocked: invalid key provided" +} + +func (invalidUnlockKey) Unauthorized() {} + +type notLockedError struct{} + +func (notLockedError) Error() string { + return "swarm is not locked" +} + +func (notLockedError) Conflict() {} diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index fbe9006561..2b269793fb 100644 --- a/daemon/cluster/executor/backend.go +++ b/daemon/cluster/executor/backend.go @@ -34,7 +34,7 @@ type Backend interface { CreateManagedContainer(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error) ContainerStart(name string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error ContainerStop(name string, seconds *int) error - ContainerLogs(context.Context, string, *types.ContainerLogsOptions) (<-chan *backend.LogMessage, error) + ContainerLogs(context.Context, string, *types.ContainerLogsOptions) (msgs <-chan *backend.LogMessage, tty bool, err error) ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error ActivateContainerServiceBinding(containerName string) error DeactivateContainerServiceBinding(containerName string) error diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 0a33739669..ded3533515 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -451,7 +451,7 @@ func (c *containerAdapter) logs(ctx context.Context, options api.LogSubscription } } } - msgs, err := c.backend.ContainerLogs(ctx, c.container.name(), apiOptions) + msgs, _, err := c.backend.ContainerLogs(ctx, c.container.name(), apiOptions) if err != nil { return nil, err } diff --git a/daemon/cluster/helpers.go b/daemon/cluster/helpers.go index a74118c422..fd50eef66e 100644 --- a/daemon/cluster/helpers.go +++ b/daemon/cluster/helpers.go @@ -3,8 +3,8 @@ package cluster import ( "fmt" - "github.com/docker/docker/api/errors" swarmapi "github.com/docker/swarmkit/api" + "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -15,7 +15,7 @@ func getSwarm(ctx context.Context, c swarmapi.ControlClient) (*swarmapi.Cluster, } if len(rl.Clusters) == 0 { - return nil, errors.NewRequestNotFoundError(errNoSwarm) + return nil, errors.WithStack(errNoSwarm) } // TODO: assume one cluster only @@ -48,11 +48,11 @@ func getNode(ctx context.Context, c swarmapi.ControlClient, input string) (*swar if len(rl.Nodes) == 0 { err := fmt.Errorf("node %s not found", input) - return nil, errors.NewRequestNotFoundError(err) + return nil, notFoundError{err} } if l := len(rl.Nodes); l > 1 { - return nil, fmt.Errorf("node %s is ambiguous (%d matches found)", input, l) + return nil, ambiguousResultsError{fmt.Errorf("node %s is ambiguous (%d matches found)", input, l)} } return rl.Nodes[0], nil @@ -84,11 +84,11 @@ func getService(ctx context.Context, c swarmapi.ControlClient, input string, ins if len(rl.Services) == 0 { err := fmt.Errorf("service %s not found", input) - return nil, errors.NewRequestNotFoundError(err) + return nil, notFoundError{err} } if l := len(rl.Services); l > 1 { - return nil, fmt.Errorf("service %s is ambiguous (%d matches found)", input, l) + return nil, ambiguousResultsError{fmt.Errorf("service %s is ambiguous (%d matches found)", input, l)} } if !insertDefaults { @@ -128,11 +128,11 @@ func getTask(ctx context.Context, c swarmapi.ControlClient, input string) (*swar if len(rl.Tasks) == 0 { err := fmt.Errorf("task %s not found", input) - return nil, errors.NewRequestNotFoundError(err) + return nil, notFoundError{err} } if l := len(rl.Tasks); l > 1 { - return nil, fmt.Errorf("task %s is ambiguous (%d matches found)", input, l) + return nil, ambiguousResultsError{fmt.Errorf("task %s is ambiguous (%d matches found)", input, l)} } return rl.Tasks[0], nil @@ -164,11 +164,11 @@ func getSecret(ctx context.Context, c swarmapi.ControlClient, input string) (*sw if len(rl.Secrets) == 0 { err := fmt.Errorf("secret %s not found", input) - return nil, errors.NewRequestNotFoundError(err) + return nil, notFoundError{err} } if l := len(rl.Secrets); l > 1 { - return nil, fmt.Errorf("secret %s is ambiguous (%d matches found)", input, l) + return nil, ambiguousResultsError{fmt.Errorf("secret %s is ambiguous (%d matches found)", input, l)} } return rl.Secrets[0], nil @@ -200,11 +200,11 @@ func getConfig(ctx context.Context, c swarmapi.ControlClient, input string) (*sw if len(rl.Configs) == 0 { err := fmt.Errorf("config %s not found", input) - return nil, errors.NewRequestNotFoundError(err) + return nil, notFoundError{err} } if l := len(rl.Configs); l > 1 { - return nil, fmt.Errorf("config %s is ambiguous (%d matches found)", input, l) + return nil, ambiguousResultsError{fmt.Errorf("config %s is ambiguous (%d matches found)", input, l)} } return rl.Configs[0], nil @@ -238,7 +238,7 @@ func getNetwork(ctx context.Context, c swarmapi.ControlClient, input string) (*s } if l := len(rl.Networks); l > 1 { - return nil, fmt.Errorf("network %s is ambiguous (%d matches found)", input, l) + return nil, ambiguousResultsError{fmt.Errorf("network %s is ambiguous (%d matches found)", input, l)} } return rl.Networks[0], nil diff --git a/daemon/cluster/listen_addr.go b/daemon/cluster/listen_addr.go index 993ccb62ad..0d6590765b 100644 --- a/daemon/cluster/listen_addr.go +++ b/daemon/cluster/listen_addr.go @@ -1,20 +1,19 @@ package cluster import ( - "errors" "fmt" "net" ) -var ( - errNoSuchInterface = errors.New("no such interface") - errNoIP = errors.New("could not find the system's IP address") - errMustSpecifyListenAddr = errors.New("must specify a listening address because the address to advertise is not recognized as a system address, and a system's IP address to use could not be uniquely identified") - errBadNetworkIdentifier = errors.New("must specify a valid IP address or interface name") - errBadListenAddr = errors.New("listen address must be an IP address or network interface (with optional port number)") - errBadAdvertiseAddr = errors.New("advertise address must be a non-zero IP address or network interface (with optional port number)") - errBadDataPathAddr = errors.New("data path address must be a non-zero IP address or network interface (without a port number)") - errBadDefaultAdvertiseAddr = errors.New("default advertise address must be a non-zero IP address or network interface (without a port number)") +const ( + errNoSuchInterface configError = "no such interface" + errNoIP configError = "could not find the system's IP address" + errMustSpecifyListenAddr configError = "must specify a listening address because the address to advertise is not recognized as a system address, and a system's IP address to use could not be uniquely identified" + errBadNetworkIdentifier configError = "must specify a valid IP address or interface name" + errBadListenAddr configError = "listen address must be an IP address or network interface (with optional port number)" + errBadAdvertiseAddr configError = "advertise address must be a non-zero IP address or network interface (with optional port number)" + errBadDataPathAddr configError = "data path address must be a non-zero IP address or network interface (without a port number)" + errBadDefaultAdvertiseAddr configError = "default advertise address must be a non-zero IP address or network interface (without a port number)" ) func resolveListenAddr(specifiedAddr string) (string, string, error) { @@ -125,13 +124,13 @@ func resolveInterfaceAddr(specifiedInterface string) (net.IP, error) { if ipAddr.IP.To4() != nil { // IPv4 if interfaceAddr4 != nil { - return nil, fmt.Errorf("interface %s has more than one IPv4 address (%s and %s)", specifiedInterface, interfaceAddr4, ipAddr.IP) + return nil, configError(fmt.Sprintf("interface %s has more than one IPv4 address (%s and %s)", specifiedInterface, interfaceAddr4, ipAddr.IP)) } interfaceAddr4 = ipAddr.IP } else { // IPv6 if interfaceAddr6 != nil { - return nil, fmt.Errorf("interface %s has more than one IPv6 address (%s and %s)", specifiedInterface, interfaceAddr6, ipAddr.IP) + return nil, configError(fmt.Sprintf("interface %s has more than one IPv6 address (%s and %s)", specifiedInterface, interfaceAddr6, ipAddr.IP)) } interfaceAddr6 = ipAddr.IP } @@ -139,7 +138,7 @@ func resolveInterfaceAddr(specifiedInterface string) (net.IP, error) { } if interfaceAddr4 == nil && interfaceAddr6 == nil { - return nil, fmt.Errorf("interface %s has no usable IPv4 or IPv6 address", specifiedInterface) + return nil, configError(fmt.Sprintf("interface %s has no usable IPv4 or IPv6 address", specifiedInterface)) } // In the case that there's exactly one IPv4 address @@ -296,7 +295,7 @@ func listSystemIPs() []net.IP { func errMultipleIPs(interfaceA, interfaceB string, addrA, addrB net.IP) error { if interfaceA == interfaceB { - return fmt.Errorf("could not choose an IP address to advertise since this system has multiple addresses on interface %s (%s and %s)", interfaceA, addrA, addrB) + return configError(fmt.Sprintf("could not choose an IP address to advertise since this system has multiple addresses on interface %s (%s and %s)", interfaceA, addrA, addrB)) } - return fmt.Errorf("could not choose an IP address to advertise since this system has multiple addresses on different interfaces (%s on %s and %s on %s)", addrA, interfaceA, addrB, interfaceB) + return configError(fmt.Sprintf("could not choose an IP address to advertise since this system has multiple addresses on different interfaces (%s on %s and %s on %s)", addrA, interfaceA, addrB, interfaceB)) } diff --git a/daemon/cluster/networks.go b/daemon/cluster/networks.go index 3b271dbc8d..04582eb31e 100644 --- a/daemon/cluster/networks.go +++ b/daemon/cluster/networks.go @@ -3,7 +3,6 @@ package cluster import ( "fmt" - apierrors "github.com/docker/docker/api/errors" apitypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/network" types "github.com/docker/docker/api/types/swarm" @@ -250,8 +249,8 @@ func (c *Cluster) DetachNetwork(target string, containerID string) error { // CreateNetwork creates a new cluster managed network. func (c *Cluster) CreateNetwork(s apitypes.NetworkCreateRequest) (string, error) { if runconfig.IsPreDefinedNetwork(s.Name) { - err := fmt.Errorf("%s is a pre-defined network and cannot be created", s.Name) - return "", apierrors.NewRequestForbiddenError(err) + err := notAllowedError(fmt.Sprintf("%s is a pre-defined network and cannot be created", s.Name)) + return "", errors.WithStack(err) } var resp *swarmapi.CreateNetworkResponse @@ -299,14 +298,13 @@ func (c *Cluster) populateNetworkID(ctx context.Context, client swarmapi.Control // and use its id for the request. apiNetwork, err = getNetwork(ctx, client, ln.Name()) if err != nil { - err = fmt.Errorf("could not find the corresponding predefined swarm network: %v", err) - return apierrors.NewRequestNotFoundError(err) + return errors.Wrap(notFoundError{err}, "could not find the corresponding predefined swarm network") } goto setid } if ln != nil && !ln.Info().Dynamic() { - err = fmt.Errorf("The network %s cannot be used with services. Only networks scoped to the swarm can be used, such as those created with the overlay driver.", ln.Name()) - return apierrors.NewRequestForbiddenError(err) + errMsg := fmt.Sprintf("The network %s cannot be used with services. Only networks scoped to the swarm can be used, such as those created with the overlay driver.", ln.Name()) + return errors.WithStack(notAllowedError(errMsg)) } return err } diff --git a/daemon/cluster/nodes.go b/daemon/cluster/nodes.go index 839c8f78e5..582e7cdc74 100644 --- a/daemon/cluster/nodes.go +++ b/daemon/cluster/nodes.go @@ -1,7 +1,6 @@ package cluster import ( - apierrors "github.com/docker/docker/api/errors" apitypes "github.com/docker/docker/api/types" types "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/daemon/cluster/convert" @@ -65,7 +64,7 @@ func (c *Cluster) UpdateNode(input string, version uint64, spec types.NodeSpec) return c.lockedManagerAction(func(ctx context.Context, state nodeState) error { nodeSpec, err := convert.NodeSpecToGRPC(spec) if err != nil { - return apierrors.NewBadRequestError(err) + return convertError{err} } ctx, cancel := c.getRequestContext() diff --git a/daemon/cluster/services.go b/daemon/cluster/services.go index ba5ef040ac..0edb407d3a 100644 --- a/daemon/cluster/services.go +++ b/daemon/cluster/services.go @@ -11,7 +11,6 @@ import ( "time" "github.com/docker/distribution/reference" - apierrors "github.com/docker/docker/api/errors" apitypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" types "github.com/docker/docker/api/types/swarm" @@ -129,7 +128,7 @@ func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string, queryRe serviceSpec, err := convert.ServiceSpecToGRPC(s) if err != nil { - return apierrors.NewBadRequestError(err) + return convertError{err} } resp = &apitypes.ServiceCreateResponse{} @@ -233,7 +232,7 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ serviceSpec, err := convert.ServiceSpecToGRPC(spec) if err != nil { - return apierrors.NewBadRequestError(err) + return convertError{err} } currentService, err := getService(ctx, state.controlClient, serviceIDOrName, false) diff --git a/daemon/cluster/swarm.go b/daemon/cluster/swarm.go index 2b34d89a6a..1fa62920e3 100644 --- a/daemon/cluster/swarm.go +++ b/daemon/cluster/swarm.go @@ -6,7 +6,6 @@ import ( "strings" "time" - apierrors "github.com/docker/docker/api/errors" apitypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" types "github.com/docker/docker/api/types/swarm" @@ -41,7 +40,7 @@ func (c *Cluster) Init(req types.InitRequest) (string, error) { } if err := validateAndSanitizeInitRequest(&req); err != nil { - return "", apierrors.NewBadRequestError(err) + return "", validationError{err} } listenHost, listenPort, err := resolveListenAddr(req.ListenAddr) @@ -132,12 +131,12 @@ func (c *Cluster) Join(req types.JoinRequest) error { c.mu.Lock() if c.nr != nil { c.mu.Unlock() - return errSwarmExists + return errors.WithStack(errSwarmExists) } c.mu.Unlock() if err := validateAndSanitizeJoinRequest(&req); err != nil { - return apierrors.NewBadRequestError(err) + return validationError{err} } listenHost, listenPort, err := resolveListenAddr(req.ListenAddr) @@ -222,7 +221,7 @@ func (c *Cluster) Update(version uint64, spec types.Spec, flags types.UpdateFlag // will be used to swarmkit. clusterSpec, err := convert.SwarmSpecToGRPC(spec) if err != nil { - return apierrors.NewBadRequestError(err) + return convertError{err} } _, err = state.controlClient.UpdateCluster( @@ -284,7 +283,7 @@ func (c *Cluster) UnlockSwarm(req types.UnlockRequest) error { } else { // when manager is active, return an error of "not locked" c.mu.RUnlock() - return errors.New("swarm is not locked") + return notLockedError{} } // only when swarm is locked, code running reaches here @@ -293,7 +292,7 @@ func (c *Cluster) UnlockSwarm(req types.UnlockRequest) error { key, err := encryption.ParseHumanReadableKey(req.UnlockKey) if err != nil { - return err + return validationError{err} } config := nr.config @@ -312,9 +311,9 @@ func (c *Cluster) UnlockSwarm(req types.UnlockRequest) error { if err := <-nr.Ready(); err != nil { if errors.Cause(err) == errSwarmLocked { - return errors.New("swarm could not be unlocked: invalid key provided") + return invalidUnlockKey{} } - return fmt.Errorf("swarm component could not be started: %v", err) + return errors.Errorf("swarm component could not be started: %v", err) } return nil } @@ -328,7 +327,7 @@ func (c *Cluster) Leave(force bool) error { nr := c.nr if nr == nil { c.mu.Unlock() - return errNoSwarm + return errors.WithStack(errNoSwarm) } state := c.currentNodeState() @@ -337,7 +336,7 @@ func (c *Cluster) Leave(force bool) error { if errors.Cause(state.err) == errSwarmLocked && !force { // leave a locked swarm without --force is not allowed - return errors.New("Swarm is encrypted and locked. Please unlock it first or use `--force` to ignore this message.") + return errors.WithStack(notAvailableError("Swarm is encrypted and locked. Please unlock it first or use `--force` to ignore this message.")) } if state.IsManager() && !force { @@ -348,7 +347,7 @@ func (c *Cluster) Leave(force bool) error { if active && removingManagerCausesLossOfQuorum(reachable, unreachable) { if isLastManager(reachable, unreachable) { msg += "Removing the last manager erases all current state of the swarm. Use `--force` to ignore this message. " - return errors.New(msg) + return errors.WithStack(notAvailableError(msg)) } msg += fmt.Sprintf("Removing this node leaves %v managers out of %v. Without a Raft quorum your swarm will be inaccessible. ", reachable-1, reachable+unreachable) } @@ -358,7 +357,7 @@ func (c *Cluster) Leave(force bool) error { } msg += "The only way to restore a swarm that has lost consensus is to reinitialize it with `--force-new-cluster`. Use `--force` to suppress this message." - return errors.New(msg) + return errors.WithStack(notAvailableError(msg)) } // release readers in here if err := nr.Stop(); err != nil { diff --git a/daemon/container.go b/daemon/container.go index 4c015b70de..ddb8a2ba0c 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -6,7 +6,6 @@ import ( "path/filepath" "time" - "github.com/docker/docker/api/errors" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/container" @@ -19,6 +18,7 @@ import ( "github.com/docker/docker/runconfig" "github.com/docker/go-connections/nat" "github.com/opencontainers/selinux/go-selinux/label" + "github.com/pkg/errors" ) // GetContainer looks for a container using the provided information, which could be @@ -30,7 +30,7 @@ import ( // If none of these searches succeed, an error is returned func (daemon *Daemon) GetContainer(prefixOrName string) (*container.Container, error) { if len(prefixOrName) == 0 { - return nil, errors.NewBadRequestError(fmt.Errorf("No container name or ID supplied")) + return nil, errors.WithStack(invalidIdentifier(prefixOrName)) } if containerByID := daemon.containers.Get(prefixOrName); containerByID != nil { @@ -48,10 +48,9 @@ func (daemon *Daemon) GetContainer(prefixOrName string) (*container.Container, e if indexError != nil { // When truncindex defines an error type, use that instead if indexError == truncindex.ErrNotExist { - err := fmt.Errorf("No such container: %s", prefixOrName) - return nil, errors.NewRequestNotFoundError(err) + return nil, containerNotFound(prefixOrName) } - return nil, indexError + return nil, systemError{indexError} } return daemon.containers.Get(containerID), nil } @@ -136,7 +135,7 @@ func (daemon *Daemon) newContainer(name string, platform string, config *contain if config.Hostname == "" { config.Hostname, err = os.Hostname() if err != nil { - return nil, err + return nil, systemError{err} } } } else { @@ -255,19 +254,19 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon // Validate the healthcheck params of Config if config.Healthcheck != nil { if config.Healthcheck.Interval != 0 && config.Healthcheck.Interval < containertypes.MinimumDuration { - return nil, fmt.Errorf("Interval in Healthcheck cannot be less than %s", containertypes.MinimumDuration) + return nil, errors.Errorf("Interval in Healthcheck cannot be less than %s", containertypes.MinimumDuration) } if config.Healthcheck.Timeout != 0 && config.Healthcheck.Timeout < containertypes.MinimumDuration { - return nil, fmt.Errorf("Timeout in Healthcheck cannot be less than %s", containertypes.MinimumDuration) + return nil, errors.Errorf("Timeout in Healthcheck cannot be less than %s", containertypes.MinimumDuration) } if config.Healthcheck.Retries < 0 { - return nil, fmt.Errorf("Retries in Healthcheck cannot be negative") + return nil, errors.Errorf("Retries in Healthcheck cannot be negative") } if config.Healthcheck.StartPeriod != 0 && config.Healthcheck.StartPeriod < containertypes.MinimumDuration { - return nil, fmt.Errorf("StartPeriod in Healthcheck cannot be less than %s", containertypes.MinimumDuration) + return nil, errors.Errorf("StartPeriod in Healthcheck cannot be less than %s", containertypes.MinimumDuration) } } } @@ -277,7 +276,7 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon } if hostConfig.AutoRemove && !hostConfig.RestartPolicy.IsNone() { - return nil, fmt.Errorf("can't create 'AutoRemove' container with restart policy") + return nil, errors.Errorf("can't create 'AutoRemove' container with restart policy") } for _, extraHost := range hostConfig.ExtraHosts { @@ -289,12 +288,12 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon for port := range hostConfig.PortBindings { _, portStr := nat.SplitProtoPort(string(port)) if _, err := nat.ParsePort(portStr); err != nil { - return nil, fmt.Errorf("invalid port specification: %q", portStr) + return nil, errors.Errorf("invalid port specification: %q", portStr) } for _, pb := range hostConfig.PortBindings[port] { _, err := nat.NewPort(nat.SplitProtoPort(pb.HostPort)) if err != nil { - return nil, fmt.Errorf("invalid port specification: %q", pb.HostPort) + return nil, errors.Errorf("invalid port specification: %q", pb.HostPort) } } } @@ -304,16 +303,16 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon switch p.Name { case "always", "unless-stopped", "no": if p.MaximumRetryCount != 0 { - return nil, fmt.Errorf("maximum retry count cannot be used with restart policy '%s'", p.Name) + return nil, errors.Errorf("maximum retry count cannot be used with restart policy '%s'", p.Name) } case "on-failure": if p.MaximumRetryCount < 0 { - return nil, fmt.Errorf("maximum retry count cannot be negative") + return nil, errors.Errorf("maximum retry count cannot be negative") } case "": // do nothing default: - return nil, fmt.Errorf("invalid restart policy '%s'", p.Name) + return nil, errors.Errorf("invalid restart policy '%s'", p.Name) } // Now do platform-specific verification diff --git a/daemon/container_linux.go b/daemon/container_linux.go index 2c8771575a..d05992d0fe 100644 --- a/daemon/container_linux.go +++ b/daemon/container_linux.go @@ -14,7 +14,7 @@ func (daemon *Daemon) saveApparmorConfig(container *container.Container) error { } if err := parseSecurityOpt(container, container.HostConfig); err != nil { - return err + return validationError{err} } if !container.HostConfig.Privileged { diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 70eee6e003..fc08f8263d 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -10,7 +10,6 @@ import ( "strings" "time" - derr "github.com/docker/docker/api/errors" containertypes "github.com/docker/docker/api/types/container" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/container" @@ -923,7 +922,7 @@ func (daemon *Daemon) getNetworkedContainer(containerID, connectedContainerID st } if !nc.IsRunning() { err := fmt.Errorf("cannot join network of a non running container: %s", connectedContainerID) - return nil, derr.NewRequestConflictError(err) + return nil, stateConflictError{err} } if nc.IsRestarting() { return nil, errContainerIsRestarting(connectedContainerID) diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index f47e9e2835..84b7eb352f 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -91,7 +91,7 @@ func (daemon *Daemon) getPidContainer(container *container.Container) (*containe func containerIsRunning(c *container.Container) error { if !c.IsRunning() { - return errors.Errorf("container %s is not running", c.ID) + return stateConflictError{errors.Errorf("container %s is not running", c.ID)} } return nil } diff --git a/daemon/create.go b/daemon/create.go index cf61336597..f420c6c3e1 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -9,7 +9,6 @@ import ( "github.com/pkg/errors" - apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" networktypes "github.com/docker/docker/api/types/network" @@ -37,17 +36,17 @@ func (daemon *Daemon) ContainerCreate(params types.ContainerCreateConfig) (conta func (daemon *Daemon) containerCreate(params types.ContainerCreateConfig, managed bool) (containertypes.ContainerCreateCreatedBody, error) { start := time.Now() if params.Config == nil { - return containertypes.ContainerCreateCreatedBody{}, fmt.Errorf("Config cannot be empty in order to create a container") + return containertypes.ContainerCreateCreatedBody{}, validationError{errors.New("Config cannot be empty in order to create a container")} } warnings, err := daemon.verifyContainerSettings(params.HostConfig, params.Config, false) if err != nil { - return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, err + return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, validationError{err} } - err = daemon.verifyNetworkingConfig(params.NetworkingConfig) + err = verifyNetworkingConfig(params.NetworkingConfig) if err != nil { - return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, err + return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, validationError{err} } if params.HostConfig == nil { @@ -55,12 +54,12 @@ func (daemon *Daemon) containerCreate(params types.ContainerCreateConfig, manage } err = daemon.adaptContainerSettings(params.HostConfig, params.AdjustCPUShares) if err != nil { - return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, err + return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, validationError{err} } container, err := daemon.create(params, managed) if err != nil { - return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, daemon.imageNotExistToErrcode(err) + return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, err } containerActions.WithValues("create").UpdateSince(start) @@ -113,11 +112,11 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( } if err := daemon.mergeAndVerifyConfig(params.Config, img); err != nil { - return nil, err + return nil, validationError{err} } if err := daemon.mergeAndVerifyLogConfig(¶ms.HostConfig.LogConfig); err != nil { - return nil, err + return nil, validationError{err} } if container, err = daemon.newContainer(params.Name, params.Platform, params.Config, params.HostConfig, imgID, managed); err != nil { @@ -139,7 +138,7 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( // Set RWLayer for container after mount labels have been set if err := daemon.setRWLayer(container); err != nil { - return nil, err + return nil, systemError{err} } rootIDs := daemon.idMappings.RootPair() @@ -295,7 +294,7 @@ func (daemon *Daemon) mergeAndVerifyConfig(config *containertypes.Config, img *i // Checks if the client set configurations for more than one network while creating a container // Also checks if the IPAMConfig is valid -func (daemon *Daemon) verifyNetworkingConfig(nwConfig *networktypes.NetworkingConfig) error { +func verifyNetworkingConfig(nwConfig *networktypes.NetworkingConfig) error { if nwConfig == nil || len(nwConfig.EndpointsConfig) == 0 { return nil } @@ -303,14 +302,14 @@ func (daemon *Daemon) verifyNetworkingConfig(nwConfig *networktypes.NetworkingCo for _, v := range nwConfig.EndpointsConfig { if v != nil && v.IPAMConfig != nil { if v.IPAMConfig.IPv4Address != "" && net.ParseIP(v.IPAMConfig.IPv4Address).To4() == nil { - return apierrors.NewBadRequestError(fmt.Errorf("invalid IPv4 address: %s", v.IPAMConfig.IPv4Address)) + return errors.Errorf("invalid IPv4 address: %s", v.IPAMConfig.IPv4Address) } if v.IPAMConfig.IPv6Address != "" { n := net.ParseIP(v.IPAMConfig.IPv6Address) // if the address is an invalid network address (ParseIP == nil) or if it is // an IPv4 address (To4() != nil), then it is an invalid IPv6 address if n == nil || n.To4() != nil { - return apierrors.NewBadRequestError(fmt.Errorf("invalid IPv6 address: %s", v.IPAMConfig.IPv6Address)) + return errors.Errorf("invalid IPv6 address: %s", v.IPAMConfig.IPv6Address) } } } @@ -321,6 +320,5 @@ func (daemon *Daemon) verifyNetworkingConfig(nwConfig *networktypes.NetworkingCo for k := range nwConfig.EndpointsConfig { l = append(l, k) } - err := fmt.Errorf("Container cannot be connected to network endpoints: %s", strings.Join(l, ", ")) - return apierrors.NewBadRequestError(err) + return errors.Errorf("Container cannot be connected to network endpoints: %s", strings.Join(l, ", ")) } diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 5b317293c2..1ebebf6628 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -470,6 +470,7 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi warnings = append(warnings, "Your kernel does not support BPS Block I/O write limit or the cgroup is not mounted. Block I/O BPS write limit discarded.") logrus.Warn("Your kernel does not support BPS Block I/O write limit or the cgroup is not mounted. Block I/O BPS write limit discarded.") resources.BlkioDeviceWriteBps = []*pblkiodev.ThrottleDevice{} + } if len(resources.BlkioDeviceReadIOps) > 0 && !sysInfo.BlkioReadIOpsDevice { warnings = append(warnings, "Your kernel does not support IOPS Block read limit or the cgroup is not mounted. Block I/O IOPS read limit discarded.") @@ -1144,13 +1145,13 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig * } child, err := daemon.GetContainer(name) if err != nil { - return fmt.Errorf("Could not get container for %s", name) + return errors.Wrapf(err, "could not get container for %s", name) } for child.HostConfig.NetworkMode.IsContainer() { parts := strings.SplitN(string(child.HostConfig.NetworkMode), ":", 2) child, err = daemon.GetContainer(parts[1]) if err != nil { - return fmt.Errorf("Could not get container for %s", parts[1]) + return errors.Wrapf(err, "Could not get container for %s", parts[1]) } } if child.HostConfig.NetworkMode.IsHost() { @@ -1181,12 +1182,12 @@ func (daemon *Daemon) conditionalUnmountOnCleanup(container *container.Container func (daemon *Daemon) stats(c *container.Container) (*types.StatsJSON, error) { if !c.IsRunning() { - return nil, errNotRunning{c.ID} + return nil, errNotRunning(c.ID) } stats, err := daemon.containerd.Stats(c.ID) if err != nil { if strings.Contains(err.Error(), "container not found") { - return nil, errNotFound{c.ID} + return nil, containerNotFound(c.ID) } return nil, err } diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 4cb7560fc2..c8ae577eac 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -520,14 +520,14 @@ func driverOptions(config *config.Config) []nwconfig.Option { func (daemon *Daemon) stats(c *container.Container) (*types.StatsJSON, error) { if !c.IsRunning() { - return nil, errNotRunning{c.ID} + return nil, errNotRunning(c.ID) } // Obtain the stats from HCS via libcontainerd stats, err := daemon.containerd.Stats(c.ID) if err != nil { if strings.Contains(err.Error(), "container not found") { - return nil, errNotFound{c.ID} + return nil, containerNotFound(c.ID) } return nil, err } diff --git a/daemon/delete.go b/daemon/delete.go index 222f33cba2..99b515d014 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -7,7 +7,6 @@ import ( "strings" "time" - apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" "github.com/docker/docker/container" "github.com/docker/docker/layer" @@ -31,7 +30,7 @@ func (daemon *Daemon) ContainerRm(name string, config *types.ContainerRmConfig) // Container state RemovalInProgress should be used to avoid races. if inProgress := container.SetRemovalInProgress(); inProgress { err := fmt.Errorf("removal of container %s is already in progress", name) - return apierrors.NewBadRequestError(err) + return stateConflictError{err} } defer container.ResetRemovalInProgress() @@ -87,7 +86,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo procedure = "Unpause and then " + strings.ToLower(procedure) } err := fmt.Errorf("You cannot remove a %s container %s. %s", state, container.ID, procedure) - return apierrors.NewRequestConflictError(err) + return stateConflictError{err} } if err := daemon.Kill(container); err != nil { return fmt.Errorf("Could not kill running container %s, cannot remove - %v", container.ID, err) @@ -151,7 +150,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo func (daemon *Daemon) VolumeRm(name string, force bool) error { err := daemon.volumeRm(name) if err != nil && volumestore.IsInUse(err) { - return apierrors.NewRequestConflictError(err) + return stateConflictError{err} } if err == nil || force { daemon.volumes.Purge(name) diff --git a/daemon/errors.go b/daemon/errors.go index 5050f87e4c..cd8de4dc7d 100644 --- a/daemon/errors.go +++ b/daemon/errors.go @@ -2,52 +2,215 @@ package daemon import ( "fmt" + "strings" + "syscall" - "github.com/docker/docker/api/errors" + "github.com/pkg/errors" + "google.golang.org/grpc" ) -func (d *Daemon) imageNotExistToErrcode(err error) error { - if dne, isDNE := err.(ErrImageDoesNotExist); isDNE { - return errors.NewRequestNotFoundError(dne) - } - return err +func errNotRunning(id string) error { + return stateConflictError{errors.Errorf("Container %s is not running", id)} } -type errNotRunning struct { - containerID string +func containerNotFound(id string) error { + return objNotFoundError{"container", id} } -func (e errNotRunning) Error() string { - return fmt.Sprintf("Container %s is not running", e.containerID) +func volumeNotFound(id string) error { + return objNotFoundError{"volume", id} } -func (e errNotRunning) ContainerIsRunning() bool { - return false +func networkNotFound(id string) error { + return objNotFoundError{"network", id} } +type objNotFoundError struct { + object string + id string +} + +func (e objNotFoundError) Error() string { + return "No such " + e.object + ": " + e.id +} + +func (e objNotFoundError) NotFound() {} + +type stateConflictError struct { + cause error +} + +func (e stateConflictError) Error() string { + return e.cause.Error() +} + +func (e stateConflictError) Cause() error { + return e.cause +} + +func (e stateConflictError) Conflict() {} + func errContainerIsRestarting(containerID string) error { - err := fmt.Errorf("Container %s is restarting, wait until the container is running", containerID) - return errors.NewRequestConflictError(err) + cause := errors.Errorf("Container %s is restarting, wait until the container is running", containerID) + return stateConflictError{cause} } func errExecNotFound(id string) error { - err := fmt.Errorf("No such exec instance '%s' found in daemon", id) - return errors.NewRequestNotFoundError(err) + return objNotFoundError{"exec instance", id} } func errExecPaused(id string) error { - err := fmt.Errorf("Container %s is paused, unpause the container before exec", id) - return errors.NewRequestConflictError(err) + cause := errors.Errorf("Container %s is paused, unpause the container before exec", id) + return stateConflictError{cause} } -type errNotFound struct { - containerID string +type validationError struct { + cause error } -func (e errNotFound) Error() string { - return fmt.Sprintf("Container %s is not found", e.containerID) +func (e validationError) Error() string { + return e.cause.Error() } -func (e errNotFound) ContainerNotFound() bool { - return true +func (e validationError) InvalidParameter() {} + +func (e validationError) Cause() error { + return e.cause +} + +type notAllowedError struct { + cause error +} + +func (e notAllowedError) Error() string { + return e.cause.Error() +} + +func (e notAllowedError) Forbidden() {} + +func (e notAllowedError) Cause() error { + return e.cause +} + +type containerNotModifiedError struct { + running bool +} + +func (e containerNotModifiedError) Error() string { + if e.running { + return "Container is already started" + } + return "Container is already stopped" +} + +func (e containerNotModifiedError) NotModified() {} + +type systemError struct { + cause error +} + +func (e systemError) Error() string { + return e.cause.Error() +} + +func (e systemError) SystemError() {} + +func (e systemError) Cause() error { + return e.cause +} + +type invalidIdentifier string + +func (e invalidIdentifier) Error() string { + return fmt.Sprintf("invalid name or ID supplied: %q", string(e)) +} + +func (invalidIdentifier) InvalidParameter() {} + +type duplicateMountPointError string + +func (e duplicateMountPointError) Error() string { + return "Duplicate mount point: " + string(e) +} +func (duplicateMountPointError) InvalidParameter() {} + +type containerFileNotFound struct { + file string + container string +} + +func (e containerFileNotFound) Error() string { + return "Could not find the file " + e.file + " in container " + e.container +} + +func (containerFileNotFound) NotFound() {} + +type invalidFilter struct { + filter string + value interface{} +} + +func (e invalidFilter) Error() string { + msg := "Invalid filter '" + e.filter + if e.value != nil { + msg += fmt.Sprintf("=%s", e.value) + } + return msg + "'" +} + +func (e invalidFilter) InvalidParameter() {} + +type unknownError struct { + cause error +} + +func (e unknownError) Error() string { + return e.cause.Error() +} + +func (unknownError) Unknown() {} + +func (e unknownError) Cause() error { + return e.cause +} + +type startInvalidConfigError string + +func (e startInvalidConfigError) Error() string { + return string(e) +} + +func (e startInvalidConfigError) InvalidParameter() {} // Is this right??? + +func translateContainerdStartErr(cmd string, setExitCode func(int), err error) error { + errDesc := grpc.ErrorDesc(err) + contains := func(s1, s2 string) bool { + return strings.Contains(strings.ToLower(s1), s2) + } + var retErr error = unknownError{errors.New(errDesc)} + // if we receive an internal error from the initial start of a container then lets + // return it instead of entering the restart loop + // set to 127 for container cmd not found/does not exist) + if contains(errDesc, cmd) && + (contains(errDesc, "executable file not found") || + contains(errDesc, "no such file or directory") || + contains(errDesc, "system cannot find the file specified")) { + setExitCode(127) + retErr = startInvalidConfigError(errDesc) + } + // set to 126 for container cmd can't be invoked errors + if contains(errDesc, syscall.EACCES.Error()) { + setExitCode(126) + retErr = startInvalidConfigError(errDesc) + } + + // attempted to mount a file onto a directory, or a directory onto a file, maybe from user specified bind mounts + if contains(errDesc, syscall.ENOTDIR.Error()) { + errDesc += ": Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified host path exists and is the expected type" + setExitCode(127) + retErr = startInvalidConfigError(errDesc) + } + + // TODO: it would be nice to get some better errors from containerd so we can return better errors here + return retErr } diff --git a/daemon/exec.go b/daemon/exec.go index e77970968c..c913ffb75d 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -8,7 +8,6 @@ import ( "golang.org/x/net/context" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/container" @@ -18,6 +17,7 @@ import ( "github.com/docker/docker/pkg/pools" "github.com/docker/docker/pkg/signal" "github.com/docker/docker/pkg/term" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -81,7 +81,7 @@ func (d *Daemon) getActiveContainer(name string) (*container.Container, error) { } if !container.IsRunning() { - return nil, errNotRunning{container.ID} + return nil, errNotRunning(container.ID) } if container.IsPaused() { return nil, errExecPaused(name) @@ -157,12 +157,12 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R if ec.ExitCode != nil { ec.Unlock() err := fmt.Errorf("Error: Exec command %s has already run", ec.ID) - return errors.NewRequestConflictError(err) + return stateConflictError{err} } if ec.Running { ec.Unlock() - return fmt.Errorf("Error: Exec command %s is already running", ec.ID) + return stateConflictError{fmt.Errorf("Error: Exec command %s is already running", ec.ID)} } ec.Running = true ec.Unlock() @@ -233,7 +233,7 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R systemPid, err := d.containerd.AddProcess(ctx, c.ID, name, p, ec.InitializeStdio) if err != nil { - return err + return translateContainerdStartErr(ec.Entrypoint, ec.SetExitCode, err) } ec.Lock() ec.Pid = systemPid @@ -254,7 +254,7 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R case err := <-attachErr: if err != nil { if _, ok := err.(term.EscapeError); !ok { - return fmt.Errorf("exec attach failed with error: %v", err) + return errors.Wrap(systemError{err}, "exec attach failed") } d.LogContainerEvent(c, "exec_detach") } diff --git a/daemon/exec/exec.go b/daemon/exec/exec.go index 471113cc4a..2348ad8fd5 100644 --- a/daemon/exec/exec.go +++ b/daemon/exec/exec.go @@ -62,6 +62,11 @@ func (c *Config) CloseStreams() error { return c.StreamConfig.CloseStreams() } +// SetExitCode sets the exec config's exit code +func (c *Config) SetExitCode(code int) { + c.ExitCode = &code +} + // Store keeps track of the exec configurations. type Store struct { commands map[string]*Config diff --git a/daemon/image.go b/daemon/image.go index a51049dbb5..ac0d6bc654 100644 --- a/daemon/image.go +++ b/daemon/image.go @@ -8,12 +8,12 @@ import ( "github.com/docker/docker/pkg/stringid" ) -// ErrImageDoesNotExist is error returned when no image can be found for a reference. -type ErrImageDoesNotExist struct { +// errImageDoesNotExist is error returned when no image can be found for a reference. +type errImageDoesNotExist struct { ref reference.Reference } -func (e ErrImageDoesNotExist) Error() string { +func (e errImageDoesNotExist) Error() string { ref := e.ref if named, ok := ref.(reference.Named); ok { ref = reference.TagNameOnly(named) @@ -21,18 +21,20 @@ func (e ErrImageDoesNotExist) Error() string { return fmt.Sprintf("No such image: %s", reference.FamiliarString(ref)) } +func (e errImageDoesNotExist) NotFound() {} + // GetImageIDAndPlatform returns an image ID and platform corresponding to the image referred to by // refOrID. func (daemon *Daemon) GetImageIDAndPlatform(refOrID string) (image.ID, string, error) { ref, err := reference.ParseAnyReference(refOrID) if err != nil { - return "", "", err + return "", "", validationError{err} } namedRef, ok := ref.(reference.Named) if !ok { digested, ok := ref.(reference.Digested) if !ok { - return "", "", ErrImageDoesNotExist{ref} + return "", "", errImageDoesNotExist{ref} } id := image.IDFromDigest(digested.Digest()) for platform := range daemon.stores { @@ -40,7 +42,7 @@ func (daemon *Daemon) GetImageIDAndPlatform(refOrID string) (image.ID, string, e return id, platform, nil } } - return "", "", ErrImageDoesNotExist{ref} + return "", "", errImageDoesNotExist{ref} } for platform := range daemon.stores { @@ -71,7 +73,7 @@ func (daemon *Daemon) GetImageIDAndPlatform(refOrID string) (image.ID, string, e } } - return "", "", ErrImageDoesNotExist{ref} + return "", "", errImageDoesNotExist{ref} } // GetImage returns an image corresponding to the image referred to by refOrID. diff --git a/daemon/image_delete.go b/daemon/image_delete.go index 4e228594bc..84b86580c6 100644 --- a/daemon/image_delete.go +++ b/daemon/image_delete.go @@ -6,11 +6,11 @@ import ( "time" "github.com/docker/distribution/reference" - "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" "github.com/docker/docker/container" "github.com/docker/docker/image" "github.com/docker/docker/pkg/stringid" + "github.com/pkg/errors" ) type conflictType int @@ -67,7 +67,7 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I imgID, platform, err := daemon.GetImageIDAndPlatform(imageRef) if err != nil { - return nil, daemon.imageNotExistToErrcode(err) + return nil, err } repoRefs := daemon.stores[platform].referenceStore.References(imgID.Digest()) @@ -84,8 +84,8 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I // this image would remain "dangling" and since // we really want to avoid that the client must // explicitly force its removal. - err := fmt.Errorf("conflict: unable to remove repository reference %q (must force) - container %s is using its referenced image %s", imageRef, stringid.TruncateID(container.ID), stringid.TruncateID(imgID.String())) - return nil, errors.NewRequestConflictError(err) + err := errors.Errorf("conflict: unable to remove repository reference %q (must force) - container %s is using its referenced image %s", imageRef, stringid.TruncateID(container.ID), stringid.TruncateID(imgID.String())) + return nil, stateConflictError{err} } } @@ -285,6 +285,8 @@ func (idc *imageDeleteConflict) Error() string { return fmt.Sprintf("conflict: unable to delete %s (%s) - %s", stringid.TruncateID(idc.imgID.String()), forceMsg, idc.message) } +func (idc *imageDeleteConflict) Conflict() {} + // imageDeleteHelper attempts to delete the given image from this daemon. If // the image has any hard delete conflicts (child images or running containers // using the image) then it cannot be deleted. If the image has any soft delete diff --git a/daemon/image_pull.go b/daemon/image_pull.go index abc81ec67c..0f59f77711 100644 --- a/daemon/image_pull.go +++ b/daemon/image_pull.go @@ -26,7 +26,7 @@ func (daemon *Daemon) PullImage(ctx context.Context, image, tag, platform string ref, err := reference.ParseNormalizedNamed(image) if err != nil { - return err + return validationError{err} } if tag != "" { @@ -39,7 +39,7 @@ func (daemon *Daemon) PullImage(ctx context.Context, image, tag, platform string ref, err = reference.WithTag(ref, tag) } if err != nil { - return err + return validationError{err} } } @@ -96,7 +96,7 @@ func (daemon *Daemon) GetRepository(ctx context.Context, ref reference.Named, au } // makes sure name is not empty or `scratch` if err := distribution.ValidateRepoName(repoInfo.Name); err != nil { - return nil, false, err + return nil, false, validationError{err} } // get endpoints diff --git a/daemon/images.go b/daemon/images.go index 4baf703715..548c3a5fc8 100644 --- a/daemon/images.go +++ b/daemon/images.go @@ -71,7 +71,7 @@ func (daemon *Daemon) Images(imageFilters filters.Args, all bool, withExtraAttrs if imageFilters.ExactMatch("dangling", "true") { danglingOnly = true } else if !imageFilters.ExactMatch("dangling", "false") { - return nil, fmt.Errorf("Invalid filter 'dangling=%s'", imageFilters.Get("dangling")) + return nil, invalidFilter{"dangling", imageFilters.Get("dangling")} } } if danglingOnly { diff --git a/daemon/import.go b/daemon/import.go index 0409cd6bd6..e58d912de9 100644 --- a/daemon/import.go +++ b/daemon/import.go @@ -42,16 +42,16 @@ func (daemon *Daemon) ImportImage(src string, repository, platform string, tag s var err error newRef, err = reference.ParseNormalizedNamed(repository) if err != nil { - return err + return validationError{err} } if _, isCanonical := newRef.(reference.Canonical); isCanonical { - return errors.New("cannot import digest reference") + return validationError{errors.New("cannot import digest reference")} } if tag != "" { newRef, err = reference.WithTag(newRef, tag) if err != nil { - return err + return validationError{err} } } } @@ -69,7 +69,7 @@ func (daemon *Daemon) ImportImage(src string, repository, platform string, tag s } u, err := url.Parse(src) if err != nil { - return err + return validationError{err} } resp, err = remotecontext.GetWithStatusError(u.String()) diff --git a/daemon/inspect.go b/daemon/inspect.go index fcdeb81ab8..98f291c529 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -11,6 +11,7 @@ import ( "github.com/docker/docker/api/types/versions/v1p20" "github.com/docker/docker/container" "github.com/docker/docker/daemon/network" + volumestore "github.com/docker/docker/volume/store" "github.com/docker/go-connections/nat" ) @@ -187,7 +188,7 @@ func (daemon *Daemon) getInspectData(container *container.Container) (*types.Con // could have been removed, it will cause error if we try to get the metadata, // we can ignore the error if the container is dead. if err != nil && !container.Dead { - return nil, err + return nil, systemError{err} } contJSONBase.GraphDriver.Data = graphDriverData @@ -228,7 +229,10 @@ func (daemon *Daemon) ContainerExecInspect(id string) (*backend.ExecInspect, err func (daemon *Daemon) VolumeInspect(name string) (*types.Volume, error) { v, err := daemon.volumes.Get(name) if err != nil { - return nil, err + if volumestore.IsNotExist(err) { + return nil, volumeNotFound(name) + } + return nil, systemError{err} } apiV := volumeToAPIType(v) apiV.Mountpoint = v.Path() diff --git a/daemon/kill.go b/daemon/kill.go index 586c9661fb..43981513db 100644 --- a/daemon/kill.go +++ b/daemon/kill.go @@ -10,6 +10,7 @@ import ( containerpkg "github.com/docker/docker/container" "github.com/docker/docker/pkg/signal" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -22,6 +23,8 @@ func (e errNoSuchProcess) Error() string { return fmt.Sprintf("Cannot kill process (pid=%d) with signal %d: no such process.", e.pid, e.signal) } +func (errNoSuchProcess) NotFound() {} + // isErrNoSuchProcess returns true if the error // is an instance of errNoSuchProcess. func isErrNoSuchProcess(err error) bool { @@ -61,7 +64,7 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int) defer container.Unlock() if !container.Running { - return errNotRunning{container.ID} + return errNotRunning(container.ID) } var unpause bool @@ -91,8 +94,9 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int) } if err := daemon.kill(container, sig); err != nil { - err = fmt.Errorf("Cannot kill container %s: %s", container.ID, err) + err = errors.Wrapf(err, "Cannot kill container %s", container.ID) // if container or process not exists, ignore the error + // TODO: we shouldn't have to parse error strings from containerd if strings.Contains(err.Error(), "container not found") || strings.Contains(err.Error(), "no such process") { logrus.Warnf("container kill failed because of 'container not found' or 'no such process': %s", err.Error()) @@ -119,7 +123,7 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int) // Kill forcefully terminates a container. func (daemon *Daemon) Kill(container *containerpkg.Container) error { if !container.IsRunning() { - return errNotRunning{container.ID} + return errNotRunning(container.ID) } // 1. Send SIGKILL diff --git a/daemon/list.go b/daemon/list.go index 5906ed3a69..b171ffb1b8 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -1,7 +1,6 @@ package daemon import ( - "errors" "fmt" "sort" "strconv" @@ -13,6 +12,7 @@ import ( "github.com/docker/docker/image" "github.com/docker/docker/volume" "github.com/docker/go-connections/nat" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -265,7 +265,7 @@ func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerLis err = psFilters.WalkValues("status", func(value string) error { if !container.IsValidStateString(value) { - return fmt.Errorf("Unrecognised filter value for status: %s", value) + return invalidFilter{"status", value} } config.All = true @@ -284,13 +284,13 @@ func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerLis taskFilter = true isTask = false } else { - return nil, fmt.Errorf("Invalid filter 'is-task=%s'", psFilters.Get("is-task")) + return nil, invalidFilter{"is-task", psFilters.Get("is-task")} } } err = psFilters.WalkValues("health", func(value string) error { if !container.IsValidHealthString(value) { - return fmt.Errorf("Unrecognised filter value for health: %s", value) + return validationError{errors.Errorf("Unrecognised filter value for health: %s", value)} } return nil @@ -567,7 +567,7 @@ func (daemon *Daemon) refreshImage(s *container.Snapshot, ctx *listContext) (*ty image := s.Image // keep the original ref if still valid (hasn't changed) if image != s.ImageID { id, _, err := daemon.GetImageIDAndPlatform(image) - if _, isDNE := err.(ErrImageDoesNotExist); err != nil && !isDNE { + if _, isDNE := err.(errImageDoesNotExist); err != nil && !isDNE { return nil, err } if err != nil || id.String() != s.ImageID { @@ -653,7 +653,7 @@ func (daemon *Daemon) filterVolumes(vols []volume.Volume, filter filters.Args) ( if filter.ExactMatch("dangling", "true") || filter.ExactMatch("dangling", "1") { danglingOnly = true } else if !filter.ExactMatch("dangling", "false") && !filter.ExactMatch("dangling", "0") { - return nil, fmt.Errorf("Invalid filter 'dangling=%s'", filter.Get("dangling")) + return nil, invalidFilter{"dangling", filter.Get("dangling")} } retVols = daemon.volumes.FilterByUsed(retVols, !danglingOnly) } diff --git a/daemon/logger/logger.go b/daemon/logger/logger.go index 258a5dc5f0..a9d1e7640b 100644 --- a/daemon/logger/logger.go +++ b/daemon/logger/logger.go @@ -8,7 +8,6 @@ package logger import ( - "errors" "sync" "time" @@ -16,8 +15,15 @@ import ( "github.com/docker/docker/pkg/jsonlog" ) -// ErrReadLogsNotSupported is returned when the logger does not support reading logs. -var ErrReadLogsNotSupported = errors.New("configured logging driver does not support reading") +// ErrReadLogsNotSupported is returned when the underlying log driver does not support reading +type ErrReadLogsNotSupported struct{} + +func (ErrReadLogsNotSupported) Error() string { + return "configured logging driver does not support reading" +} + +// NotImplemented makes this error implement the `NotImplemented` interface from api/errdefs +func (ErrReadLogsNotSupported) NotImplemented() {} const ( // TimeFormat is the time format used for timestamps sent to log readers. diff --git a/daemon/logs.go b/daemon/logs.go index 2bd57ee533..68c5e5aa47 100644 --- a/daemon/logs.go +++ b/daemon/logs.go @@ -22,7 +22,7 @@ import ( // // if it returns nil, the config channel will be active and return log // messages until it runs out or the context is canceled. -func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, config *types.ContainerLogsOptions) (<-chan *backend.LogMessage, error) { +func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, config *types.ContainerLogsOptions) (<-chan *backend.LogMessage, bool, error) { lg := logrus.WithFields(logrus.Fields{ "module": "daemon", "method": "(*Daemon).ContainerLogs", @@ -30,24 +30,24 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c }) if !(config.ShowStdout || config.ShowStderr) { - return nil, errors.New("You must choose at least one stream") + return nil, false, validationError{errors.New("You must choose at least one stream")} } container, err := daemon.GetContainer(containerName) if err != nil { - return nil, err + return nil, false, err } if container.RemovalInProgress || container.Dead { - return nil, errors.New("can not get logs from container which is dead or marked for removal") + return nil, false, stateConflictError{errors.New("can not get logs from container which is dead or marked for removal")} } if container.HostConfig.LogConfig.Type == "none" { - return nil, logger.ErrReadLogsNotSupported + return nil, false, logger.ErrReadLogsNotSupported{} } cLog, cLogCreated, err := daemon.getLogger(container) if err != nil { - return nil, err + return nil, false, err } if cLogCreated { defer func() { @@ -59,7 +59,7 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c logReader, ok := cLog.(logger.LogReader) if !ok { - return nil, logger.ErrReadLogsNotSupported + return nil, false, logger.ErrReadLogsNotSupported{} } follow := config.Follow && !cLogCreated @@ -72,7 +72,7 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c if config.Since != "" { s, n, err := timetypes.ParseTimestamps(config.Since, 0) if err != nil { - return nil, err + return nil, false, err } since = time.Unix(s, n) } @@ -137,7 +137,7 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c } } }() - return messageChan, nil + return messageChan, container.Config.Tty, nil } func (daemon *Daemon) getLogger(container *container.Container) (l logger.Logger, created bool, err error) { diff --git a/daemon/names.go b/daemon/names.go index 845c4e9953..e61e94008f 100644 --- a/daemon/names.go +++ b/daemon/names.go @@ -8,6 +8,7 @@ import ( "github.com/docker/docker/container" "github.com/docker/docker/pkg/namesgenerator" "github.com/docker/docker/pkg/stringid" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -55,7 +56,7 @@ func (daemon *Daemon) generateIDAndName(name string) (string, string, error) { func (daemon *Daemon) reserveName(id, name string) (string, error) { if !validContainerNamePattern.MatchString(strings.TrimPrefix(name, "/")) { - return "", fmt.Errorf("Invalid container name (%s), only %s are allowed", name, validContainerNameChars) + return "", validationError{errors.Errorf("Invalid container name (%s), only %s are allowed", name, validContainerNameChars)} } if name[0] != '/' { name = "/" + name @@ -68,9 +69,9 @@ func (daemon *Daemon) reserveName(id, name string) (string, error) { logrus.Errorf("got unexpected error while looking up reserved name: %v", err) return "", err } - return "", fmt.Errorf("Conflict. The container name %q is already in use by container %q. You have to remove (or rename) that container to be able to reuse that name.", name, id) + return "", validationError{errors.Errorf("Conflict. The container name %q is already in use by container %q. You have to remove (or rename) that container to be able to reuse that name.", name, id)} } - return "", fmt.Errorf("error reserving name: %q, error: %v", name, err) + return "", errors.Wrapf(err, "error reserving name: %q", name) } return name, nil } diff --git a/daemon/network.go b/daemon/network.go index 22f94bcb32..2d27e108d2 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -8,7 +8,6 @@ import ( "strings" "sync" - apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/network" clustertypes "github.com/docker/docker/daemon/cluster/provider" @@ -57,10 +56,10 @@ func (daemon *Daemon) GetNetworkByID(partialID string) (libnetwork.Network, erro list := daemon.GetNetworksByID(partialID) if len(list) == 0 { - return nil, libnetwork.ErrNoSuchNetwork(partialID) + return nil, errors.WithStack(networkNotFound(partialID)) } if len(list) > 1 { - return nil, libnetwork.ErrInvalidID(partialID) + return nil, errors.WithStack(invalidIdentifier(partialID)) } return list[0], nil } @@ -287,7 +286,7 @@ func (daemon *Daemon) CreateNetwork(create types.NetworkCreateRequest) (*types.N func (daemon *Daemon) createNetwork(create types.NetworkCreateRequest, id string, agent bool) (*types.NetworkCreateResponse, error) { if runconfig.IsPreDefinedNetwork(create.Name) && !agent { err := fmt.Errorf("%s is a pre-defined network and cannot be created", create.Name) - return nil, apierrors.NewRequestForbiddenError(err) + return nil, notAllowedError{err} } var warning string @@ -504,7 +503,7 @@ func (daemon *Daemon) deleteNetwork(networkID string, dynamic bool) error { if runconfig.IsPreDefinedNetwork(nw.Name()) && !dynamic { err := fmt.Errorf("%s is a pre-defined network and cannot be removed", nw.Name()) - return apierrors.NewRequestForbiddenError(err) + return notAllowedError{err} } if dynamic && !nw.Info().Dynamic() { @@ -514,7 +513,7 @@ func (daemon *Daemon) deleteNetwork(networkID string, dynamic bool) error { return nil } err := fmt.Errorf("%s is not a dynamic network", nw.Name()) - return apierrors.NewRequestForbiddenError(err) + return notAllowedError{err} } if err := nw.Delete(); err != nil { diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index a80de1be31..53b246a2d2 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -518,7 +518,7 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c for _, m := range mounts { for _, cm := range s.Mounts { if cm.Destination == m.Destination { - return fmt.Errorf("Duplicate mount point '%s'", m.Destination) + return duplicateMountPointError(m.Destination) } } diff --git a/daemon/pause.go b/daemon/pause.go index dbfafbc5fd..3fecea59c9 100644 --- a/daemon/pause.go +++ b/daemon/pause.go @@ -28,7 +28,7 @@ func (daemon *Daemon) containerPause(container *container.Container) error { // We cannot Pause the container which is not running if !container.Running { - return errNotRunning{container.ID} + return errNotRunning(container.ID) } // We cannot Pause the container which is already paused diff --git a/daemon/prune.go b/daemon/prune.go index 824e0cd833..7df922d6b5 100644 --- a/daemon/prune.go +++ b/daemon/prune.go @@ -186,7 +186,7 @@ func (daemon *Daemon) ImagesPrune(ctx context.Context, pruneFilters filters.Args if pruneFilters.ExactMatch("dangling", "false") || pruneFilters.ExactMatch("dangling", "0") { danglingOnly = false } else if !pruneFilters.ExactMatch("dangling", "true") && !pruneFilters.ExactMatch("dangling", "1") { - return nil, fmt.Errorf("Invalid filter 'dangling=%s'", pruneFilters.Get("dangling")) + return nil, invalidFilter{"dangling", pruneFilters.Get("dangling")} } } diff --git a/daemon/rename.go b/daemon/rename.go index b714ece7f9..26ee9beaea 100644 --- a/daemon/rename.go +++ b/daemon/rename.go @@ -1,12 +1,11 @@ package daemon import ( - "errors" - "fmt" "strings" dockercontainer "github.com/docker/docker/container" "github.com/docker/libnetwork" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -20,7 +19,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { ) if oldName == "" || newName == "" { - return errors.New("Neither old nor new names may be empty") + return validationError{errors.New("Neither old nor new names may be empty")} } if newName[0] != '/' { @@ -39,19 +38,19 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { oldIsAnonymousEndpoint := container.NetworkSettings.IsAnonymousEndpoint if oldName == newName { - return errors.New("Renaming a container with the same name as its current name") + return validationError{errors.New("Renaming a container with the same name as its current name")} } links := map[string]*dockercontainer.Container{} for k, v := range daemon.linkIndex.children(container) { if !strings.HasPrefix(k, oldName) { - return fmt.Errorf("Linked container %s does not match parent %s", k, oldName) + return validationError{errors.Errorf("Linked container %s does not match parent %s", k, oldName)} } links[strings.TrimPrefix(k, oldName)] = v } if newName, err = daemon.reserveName(container.ID, newName); err != nil { - return fmt.Errorf("Error when allocating new name: %v", err) + return errors.Wrap(err, "Error when allocating new name") } for k, v := range links { diff --git a/daemon/resize.go b/daemon/resize.go index 747353852e..0923d0fe12 100644 --- a/daemon/resize.go +++ b/daemon/resize.go @@ -15,7 +15,7 @@ func (daemon *Daemon) ContainerResize(name string, height, width int) error { } if !container.IsRunning() { - return errNotRunning{container.ID} + return errNotRunning(container.ID) } if err = daemon.containerd.Resize(container.ID, libcontainerd.InitFriendlyName, width, height); err == nil { diff --git a/daemon/search.go b/daemon/search.go index 5d2ac5d222..540e247fe4 100644 --- a/daemon/search.go +++ b/daemon/search.go @@ -1,7 +1,6 @@ package daemon import ( - "fmt" "strconv" "golang.org/x/net/context" @@ -38,14 +37,14 @@ func (daemon *Daemon) SearchRegistryForImages(ctx context.Context, filtersArgs s if searchFilters.UniqueExactMatch("is-automated", "true") { isAutomated = true } else if !searchFilters.UniqueExactMatch("is-automated", "false") { - return nil, fmt.Errorf("Invalid filter 'is-automated=%s'", searchFilters.Get("is-automated")) + return nil, invalidFilter{"is-automated", searchFilters.Get("is-automated")} } } if searchFilters.Include("is-official") { if searchFilters.UniqueExactMatch("is-official", "true") { isOfficial = true } else if !searchFilters.UniqueExactMatch("is-official", "false") { - return nil, fmt.Errorf("Invalid filter 'is-official=%s'", searchFilters.Get("is-official")) + return nil, invalidFilter{"is-official", searchFilters.Get("is-official")} } } if searchFilters.Include("stars") { @@ -53,7 +52,7 @@ func (daemon *Daemon) SearchRegistryForImages(ctx context.Context, filtersArgs s for _, hasStar := range hasStars { iHasStar, err := strconv.Atoi(hasStar) if err != nil { - return nil, fmt.Errorf("Invalid filter 'stars=%s'", hasStar) + return nil, invalidFilter{"stars", hasStar} } if iHasStar > hasStarFilter { hasStarFilter = iHasStar diff --git a/daemon/start.go b/daemon/start.go index 61af1cf9ec..f0522d3931 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -1,26 +1,20 @@ package daemon import ( - "fmt" - "net/http" "runtime" - "strings" - "syscall" "time" - "google.golang.org/grpc" - - apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/container" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) // ContainerStart starts a container. func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.HostConfig, checkpoint string, checkpointDir string) error { if checkpoint != "" && !daemon.HasExperimental() { - return apierrors.NewBadRequestError(fmt.Errorf("checkpoint is only supported in experimental mode")) + return validationError{errors.New("checkpoint is only supported in experimental mode")} } container, err := daemon.GetContainer(name) @@ -28,13 +22,26 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos return err } - if container.IsPaused() { - return fmt.Errorf("Cannot start a paused container, try unpause instead.") + validateState := func() error { + container.Lock() + defer container.Unlock() + + if container.Paused { + return stateConflictError{errors.New("Cannot start a paused container, try unpause instead.")} + } + + if container.Running { + return containerNotModifiedError{running: true} + } + + if container.RemovalInProgress || container.Dead { + return stateConflictError{errors.New("Container is marked for removal and cannot be started.")} + } + return nil } - if container.IsRunning() { - err := fmt.Errorf("Container already started") - return apierrors.NewErrorWithStatusCode(err, http.StatusNotModified) + if err := validateState(); err != nil { + return err } // Windows does not have the backwards compatibility issue here. @@ -45,13 +52,13 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos logrus.Warn("DEPRECATED: Setting host configuration options when the container starts is deprecated and has been removed in Docker 1.12") oldNetworkMode := container.HostConfig.NetworkMode if err := daemon.setSecurityOptions(container, hostConfig); err != nil { - return err + return validationError{err} } if err := daemon.mergeAndVerifyLogConfig(&hostConfig.LogConfig); err != nil { - return err + return validationError{err} } if err := daemon.setHostConfig(container, hostConfig); err != nil { - return err + return validationError{err} } newNetworkMode := container.HostConfig.NetworkMode if string(oldNetworkMode) != string(newNetworkMode) { @@ -59,31 +66,34 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos // old networks. It is a deprecated feature and has been removed in Docker 1.12 container.NetworkSettings.Networks = nil if err := container.CheckpointTo(daemon.containersReplica); err != nil { - return err + return systemError{err} } } container.InitDNSHostConfig() } } else { if hostConfig != nil { - return fmt.Errorf("Supplying a hostconfig on start is not supported. It should be supplied on create") + return validationError{errors.New("Supplying a hostconfig on start is not supported. It should be supplied on create")} } } // check if hostConfig is in line with the current system settings. // It may happen cgroups are umounted or the like. if _, err = daemon.verifyContainerSettings(container.HostConfig, nil, false); err != nil { - return err + return validationError{err} } // Adapt for old containers in case we have updates in this function and // old containers never have chance to call the new function in create stage. if hostConfig != nil { if err := daemon.adaptContainerSettings(container.HostConfig, false); err != nil { - return err + return validationError{err} } } - return daemon.containerStart(container, checkpoint, checkpointDir, true) + if err := daemon.containerStart(container, checkpoint, checkpointDir, true); err != nil { + return err + } + return nil } // containerStart prepares the container to run by setting up everything the @@ -100,7 +110,7 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint } if container.RemovalInProgress || container.Dead { - return fmt.Errorf("Container is marked for removal and cannot be started.") + return stateConflictError{errors.New("Container is marked for removal and cannot be started.")} } // if we encounter an error during start we need to ensure that any other @@ -139,7 +149,7 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint spec, err := daemon.createSpec(container) if err != nil { - return err + return systemError{err} } createOptions, err := daemon.getLibcontainerdCreateOptions(container) @@ -160,32 +170,8 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint } if err := daemon.containerd.Create(container.ID, checkpoint, checkpointDir, *spec, container.InitializeStdio, createOptions...); err != nil { - errDesc := grpc.ErrorDesc(err) - contains := func(s1, s2 string) bool { - return strings.Contains(strings.ToLower(s1), s2) - } - logrus.Errorf("Create container failed with error: %s", errDesc) - // if we receive an internal error from the initial start of a container then lets - // return it instead of entering the restart loop - // set to 127 for container cmd not found/does not exist) - if contains(errDesc, container.Path) && - (contains(errDesc, "executable file not found") || - contains(errDesc, "no such file or directory") || - contains(errDesc, "system cannot find the file specified")) { - container.SetExitCode(127) - } - // set to 126 for container cmd can't be invoked errors - if contains(errDesc, syscall.EACCES.Error()) { - container.SetExitCode(126) - } + return translateContainerdStartErr(container.Path, container.SetExitCode, err) - // attempted to mount a file onto a directory, or a directory onto a file, maybe from user specified bind mounts - if contains(errDesc, syscall.ENOTDIR.Error()) { - errDesc += ": Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified host path exists and is the expected type" - container.SetExitCode(127) - } - - return fmt.Errorf("%s", errDesc) } containerActions.WithValues("start").UpdateSince(start) diff --git a/daemon/start_unix.go b/daemon/start_unix.go index 12ecdab2db..87ab0850c2 100644 --- a/daemon/start_unix.go +++ b/daemon/start_unix.go @@ -3,10 +3,9 @@ package daemon import ( - "fmt" - "github.com/docker/docker/container" "github.com/docker/docker/libcontainerd" + "github.com/pkg/errors" ) // getLibcontainerdCreateOptions callers must hold a lock on the container @@ -21,7 +20,7 @@ func (daemon *Daemon) getLibcontainerdCreateOptions(container *container.Contain rt := daemon.configStore.GetRuntime(container.HostConfig.Runtime) if rt == nil { - return nil, fmt.Errorf("no such runtime '%s'", container.HostConfig.Runtime) + return nil, validationError{errors.Errorf("no such runtime '%s'", container.HostConfig.Runtime)} } if UsingSystemd(daemon.configStore) { rt.Args = append(rt.Args, "--systemd-cgroup=true") diff --git a/daemon/stats/collector.go b/daemon/stats/collector.go index c06d80c172..c930bc756c 100644 --- a/daemon/stats/collector.go +++ b/daemon/stats/collector.go @@ -113,10 +113,10 @@ func (s *Collector) Run() { type notRunningErr interface { error - ContainerIsRunning() bool + Conflict() } type notFoundErr interface { error - ContainerNotFound() bool + NotFound() } diff --git a/daemon/stats_unix.go b/daemon/stats_unix.go index d875607b3a..4831b84ab9 100644 --- a/daemon/stats_unix.go +++ b/daemon/stats_unix.go @@ -3,10 +3,9 @@ package daemon import ( - "fmt" - "github.com/docker/docker/api/types" "github.com/docker/docker/container" + "github.com/pkg/errors" ) // Resolve Network SandboxID in case the container reuse another container's network stack @@ -16,7 +15,7 @@ func (daemon *Daemon) getNetworkSandboxID(c *container.Container) (string, error containerID := curr.HostConfig.NetworkMode.ConnectedContainer() connected, err := daemon.GetContainer(containerID) if err != nil { - return "", fmt.Errorf("Could not get container for %s", containerID) + return "", errors.Wrapf(err, "Could not get container for %s", containerID) } curr = connected } diff --git a/daemon/stop.go b/daemon/stop.go index af3020eaba..c43fbfa73b 100644 --- a/daemon/stop.go +++ b/daemon/stop.go @@ -2,12 +2,10 @@ package daemon import ( "context" - "fmt" - "net/http" "time" - "github.com/docker/docker/api/errors" containerpkg "github.com/docker/docker/container" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -23,15 +21,14 @@ func (daemon *Daemon) ContainerStop(name string, seconds *int) error { return err } if !container.IsRunning() { - err := fmt.Errorf("Container %s is already stopped", name) - return errors.NewErrorWithStatusCode(err, http.StatusNotModified) + return containerNotModifiedError{running: false} } if seconds == nil { stopTimeout := container.StopTimeout() seconds = &stopTimeout } if err := daemon.containerStop(container, *seconds); err != nil { - return fmt.Errorf("Cannot stop container %s: %v", name, err) + return errors.Wrapf(systemError{err}, "cannot stop container: %s", name) } return nil } diff --git a/daemon/top_unix.go b/daemon/top_unix.go index 4d94d4a62f..ec2b1da8b7 100644 --- a/daemon/top_unix.go +++ b/daemon/top_unix.go @@ -130,7 +130,7 @@ func (daemon *Daemon) ContainerTop(name string, psArgs string) (*container.Conta } if !container.IsRunning() { - return nil, errNotRunning{container.ID} + return nil, errNotRunning(container.ID) } if container.IsRestarting() { diff --git a/daemon/update.go b/daemon/update.go index a65cbd51b1..4b133b255b 100644 --- a/daemon/update.go +++ b/daemon/update.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/docker/docker/api/types/container" + "github.com/pkg/errors" ) // ContainerUpdate updates configuration of the container @@ -12,7 +13,7 @@ func (daemon *Daemon) ContainerUpdate(name string, hostConfig *container.HostCon warnings, err := daemon.verifyContainerSettings(hostConfig, nil, true) if err != nil { - return container.ContainerUpdateOKBody{Warnings: warnings}, err + return container.ContainerUpdateOKBody{Warnings: warnings}, validationError{err} } if err := daemon.update(name, hostConfig); err != nil { @@ -72,7 +73,8 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro if container.IsRunning() && !container.IsRestarting() { if err := daemon.containerd.UpdateResources(container.ID, toContainerdResources(hostConfig.Resources)); err != nil { restoreConfig = true - return errCannotUpdate(container.ID, err) + // TODO: it would be nice if containerd responded with better errors here so we can classify this better. + return errCannotUpdate(container.ID, systemError{err}) } } @@ -82,5 +84,5 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro } func errCannotUpdate(containerID string, err error) error { - return fmt.Errorf("Cannot update container %s: %v", containerID, err) + return errors.Wrap(err, "Cannot update container "+containerID) } diff --git a/daemon/volumes.go b/daemon/volumes.go index c2372ef572..b2f17dfab9 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -1,7 +1,6 @@ package daemon import ( - "errors" "fmt" "os" "path/filepath" @@ -9,13 +8,13 @@ import ( "strings" "time" - dockererrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/container" "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -149,7 +148,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo // #10618 _, tmpfsExists := hostConfig.Tmpfs[bind.Destination] if binds[bind.Destination] || tmpfsExists { - return fmt.Errorf("Duplicate mount point '%s'", bind.Destination) + return duplicateMountPointError(bind.Destination) } if bind.Type == mounttypes.TypeVolume { @@ -175,11 +174,11 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo for _, cfg := range hostConfig.Mounts { mp, err := volume.ParseMountSpec(cfg) if err != nil { - return dockererrors.NewBadRequestError(err) + return validationError{err} } if binds[mp.Destination] { - return fmt.Errorf("Duplicate mount point '%s'", cfg.Target) + return duplicateMountPointError(cfg.Target) } if mp.Type == mounttypes.TypeVolume { diff --git a/distribution/errors.go b/distribution/errors.go index 0e4942d6a6..dd6ff0a9aa 100644 --- a/distribution/errors.go +++ b/distribution/errors.go @@ -1,6 +1,7 @@ package distribution import ( + "fmt" "net/url" "strings" "syscall" @@ -12,7 +13,6 @@ import ( "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/auth" "github.com/docker/docker/distribution/xfer" - "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -60,6 +60,45 @@ func shouldV2Fallback(err errcode.Error) bool { return false } +type notFoundError struct { + cause errcode.Error + ref reference.Named +} + +func (e notFoundError) Error() string { + switch e.cause.Code { + case errcode.ErrorCodeDenied: + // ErrorCodeDenied is used when access to the repository was denied + return fmt.Sprintf("pull access denied for %s, repository does not exist or may require 'docker login'", reference.FamiliarName(e.ref)) + case v2.ErrorCodeManifestUnknown: + return fmt.Sprintf("manifest for %s not found", reference.FamiliarString(e.ref)) + case v2.ErrorCodeNameUnknown: + return fmt.Sprintf("repository %s not found", reference.FamiliarName(e.ref)) + } + // Shouldn't get here, but this is better than returning an empty string + return e.cause.Message +} + +func (e notFoundError) NotFound() {} + +func (e notFoundError) Cause() error { + return e.cause +} + +type unknownError struct { + cause error +} + +func (e unknownError) Error() string { + return e.cause.Error() +} + +func (e unknownError) Cause() error { + return e.cause +} + +func (e unknownError) Unknown() {} + // TranslatePullError is used to convert an error from a registry pull // operation to an error representing the entire pull operation. Any error // information which is not used by the returned error gets output to @@ -74,25 +113,15 @@ func TranslatePullError(err error, ref reference.Named) error { return TranslatePullError(v[0], ref) } case errcode.Error: - var newErr error switch v.Code { - case errcode.ErrorCodeDenied: - // ErrorCodeDenied is used when access to the repository was denied - newErr = errors.Errorf("pull access denied for %s, repository does not exist or may require 'docker login'", reference.FamiliarName(ref)) - case v2.ErrorCodeManifestUnknown: - newErr = errors.Errorf("manifest for %s not found", reference.FamiliarString(ref)) - case v2.ErrorCodeNameUnknown: - newErr = errors.Errorf("repository %s not found", reference.FamiliarName(ref)) - } - if newErr != nil { - logrus.Infof("Translating %q to %q", err, newErr) - return newErr + case errcode.ErrorCodeDenied, v2.ErrorCodeManifestUnknown, v2.ErrorCodeNameUnknown: + return notFoundError{v, ref} } case xfer.DoNotRetry: return TranslatePullError(v.Err, ref) } - return err + return unknownError{err} } // continueOnError returns true if we should fallback to the next endpoint @@ -157,3 +186,30 @@ func retryOnError(err error) error { // add them to the switch above. return err } + +type invalidManifestClassError struct { + mediaType string + class string +} + +func (e invalidManifestClassError) Error() string { + return fmt.Sprintf("Encountered remote %q(%s) when fetching", e.mediaType, e.class) +} + +func (e invalidManifestClassError) InvalidParameter() {} + +type invalidManifestFormatError struct{} + +func (invalidManifestFormatError) Error() string { + return "unsupported manifest format" +} + +func (invalidManifestFormatError) InvalidParameter() {} + +type reservedNameError string + +func (e reservedNameError) Error() string { + return "'" + string(e) + "' is a reserved name" +} + +func (e reservedNameError) Forbidden() {} diff --git a/distribution/pull.go b/distribution/pull.go index 690551c842..68144df794 100644 --- a/distribution/pull.go +++ b/distribution/pull.go @@ -10,6 +10,7 @@ import ( refstore "github.com/docker/docker/reference" "github.com/docker/docker/registry" "github.com/opencontainers/go-digest" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -173,7 +174,7 @@ func writeStatus(requestedTag string, out progress.Output, layersDownloaded bool // ValidateRepoName validates the name of a repository. func ValidateRepoName(name reference.Named) error { if reference.FamiliarName(name) == api.NoBaseImageSpecifier { - return fmt.Errorf("'%s' is a reserved name", api.NoBaseImageSpecifier) + return errors.WithStack(reservedNameError(api.NoBaseImageSpecifier)) } return nil } diff --git a/distribution/pull_v1.go b/distribution/pull_v1.go index eb62146422..19a8792a40 100644 --- a/distribution/pull_v1.go +++ b/distribution/pull_v1.go @@ -52,11 +52,7 @@ func (p *v1Puller) Pull(ctx context.Context, ref reference.Named) error { registry.DockerHeaders(dockerversion.DockerUserAgent(ctx), p.config.MetaHeaders)..., ) client := registry.HTTPClient(tr) - v1Endpoint, err := p.endpoint.ToV1Endpoint(dockerversion.DockerUserAgent(ctx), p.config.MetaHeaders) - if err != nil { - logrus.Debugf("Could not get v1 endpoint: %v", err) - return fallbackError{err: err} - } + v1Endpoint := p.endpoint.ToV1Endpoint(dockerversion.DockerUserAgent(ctx), p.config.MetaHeaders) p.session, err = registry.NewSession(client, p.config.AuthConfig, v1Endpoint) if err != nil { // TODO(dmcgowan): Check if should fallback diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index 713e9b7da7..9bc704d12d 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -2,7 +2,6 @@ package distribution import ( "encoding/json" - "errors" "fmt" "io" "io/ioutil" @@ -30,6 +29,7 @@ import ( refstore "github.com/docker/docker/reference" "github.com/docker/docker/registry" "github.com/opencontainers/go-digest" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -368,7 +368,7 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat if configClass == "" { configClass = "unknown" } - return false, fmt.Errorf("Encountered remote %q(%s) when fetching", m.Manifest.Config.MediaType, configClass) + return false, invalidManifestClassError{m.Manifest.Config.MediaType, configClass} } } @@ -404,7 +404,7 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat return false, err } default: - return false, errors.New("unsupported manifest format") + return false, invalidManifestFormatError{} } progress.Message(p.config.ProgressOutput, "", "Digest: "+manifestDigest.String()) diff --git a/distribution/push_v1.go b/distribution/push_v1.go index 4da4d5eac4..6cc4f1588f 100644 --- a/distribution/push_v1.go +++ b/distribution/push_v1.go @@ -41,11 +41,7 @@ func (p *v1Pusher) Push(ctx context.Context) error { registry.DockerHeaders(dockerversion.DockerUserAgent(ctx), p.config.MetaHeaders)..., ) client := registry.HTTPClient(tr) - v1Endpoint, err := p.endpoint.ToV1Endpoint(dockerversion.DockerUserAgent(ctx), p.config.MetaHeaders) - if err != nil { - logrus.Debugf("Could not get v1 endpoint: %v", err) - return fallbackError{err: err} - } + v1Endpoint := p.endpoint.ToV1Endpoint(dockerversion.DockerUserAgent(ctx), p.config.MetaHeaders) p.session, err = registry.NewSession(client, p.config.AuthConfig, v1Endpoint) if err != nil { // TODO(dmcgowan): Check if should fallback diff --git a/image/store.go b/image/store.go index 1a3045f58a..17769f4c3d 100644 --- a/image/store.go +++ b/image/store.go @@ -180,13 +180,21 @@ func (is *store) Create(config []byte) (ID, error) { return imageID, nil } +type imageNotFoundError string + +func (e imageNotFoundError) Error() string { + return "No such image: " + string(e) +} + +func (imageNotFoundError) NotFound() {} + func (is *store) Search(term string) (ID, error) { dgst, err := is.digestSet.Lookup(term) if err != nil { if err == digestset.ErrDigestNotFound { - err = fmt.Errorf("No such image: %s", term) + err = imageNotFoundError(term) } - return "", err + return "", errors.WithStack(err) } return IDFromDigest(dgst), nil } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 25c724425e..72b4807a55 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -508,7 +508,7 @@ func (s *DockerSuite) TestContainerAPIBadPort(c *check.C) { status, body, err := request.SockRequest("POST", "/containers/create", config, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) c.Assert(getErrorMessage(c, body), checker.Equals, `invalid port specification: "aa80"`, check.Commentf("Incorrect error msg: %s", body)) } @@ -537,7 +537,7 @@ func (s *DockerSuite) TestContainerAPICreateEmptyConfig(c *check.C) { status, body, err := request.SockRequest("POST", "/containers/create", config, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) expected := "Config cannot be empty in order to create a container" c.Assert(getErrorMessage(c, body), checker.Equals, expected) @@ -673,13 +673,13 @@ func (s *DockerSuite) TestContainerAPIVerifyHeader(c *check.C) { // Try with no content-type res, body, err := create("") c.Assert(err, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) body.Close() // Try with wrong content-type res, body, err = create("application/xml") c.Assert(err, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) body.Close() // now application/json @@ -705,7 +705,7 @@ func (s *DockerSuite) TestContainerAPIInvalidPortSyntax(c *check.C) { res, body, err := request.Post("/containers/create", request.RawString(config), request.JSON) c.Assert(err, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) b, err := testutil.ReadBody(body) c.Assert(err, checker.IsNil) @@ -725,7 +725,7 @@ func (s *DockerSuite) TestContainerAPIRestartPolicyInvalidPolicyName(c *check.C) res, body, err := request.Post("/containers/create", request.RawString(config), request.JSON) c.Assert(err, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) b, err := testutil.ReadBody(body) c.Assert(err, checker.IsNil) @@ -745,7 +745,7 @@ func (s *DockerSuite) TestContainerAPIRestartPolicyRetryMismatch(c *check.C) { res, body, err := request.Post("/containers/create", request.RawString(config), request.JSON) c.Assert(err, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) b, err := testutil.ReadBody(body) c.Assert(err, checker.IsNil) @@ -765,7 +765,7 @@ func (s *DockerSuite) TestContainerAPIRestartPolicyNegativeRetryCount(c *check.C res, body, err := request.Post("/containers/create", request.RawString(config), request.JSON) c.Assert(err, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) b, err := testutil.ReadBody(body) c.Assert(err, checker.IsNil) @@ -850,7 +850,7 @@ func (s *DockerSuite) TestCreateWithTooLowMemoryLimit(c *check.C) { b, err2 := testutil.ReadBody(body) c.Assert(err2, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) c.Assert(string(b), checker.Contains, "Minimum memory limit allowed is 4MB") } @@ -1005,7 +1005,7 @@ func (s *DockerSuite) TestContainerAPICopyPre124(c *check.C) { c.Assert(found, checker.True) } -func (s *DockerSuite) TestContainerAPICopyResourcePathEmptyPr124(c *check.C) { +func (s *DockerSuite) TestContainerAPICopyResourcePathEmptyPre124(c *check.C) { testRequires(c, DaemonIsLinux) // Windows only supports 1.25 or later name := "test-container-api-copy-resource-empty" dockerCmd(c, "run", "--name", name, "busybox", "touch", "/test.txt") @@ -1016,7 +1016,7 @@ func (s *DockerSuite) TestContainerAPICopyResourcePathEmptyPr124(c *check.C) { status, body, err := request.SockRequest("POST", "/v1.23/containers/"+name+"/copy", postData, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) c.Assert(string(body), checker.Matches, "Path cannot be empty\n") } @@ -1031,7 +1031,7 @@ func (s *DockerSuite) TestContainerAPICopyResourcePathNotFoundPre124(c *check.C) status, body, err := request.SockRequest("POST", "/v1.23/containers/"+name+"/copy", postData, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusNotFound) c.Assert(string(body), checker.Matches, "Could not find the file /notexist in container "+name+"\n") } @@ -1301,7 +1301,7 @@ func (s *DockerSuite) TestPostContainersCreateWithWrongCpusetValues(c *check.C) name := "wrong-cpuset-cpus" status, body, err := request.SockRequest("POST", "/containers/create?name="+name, c1, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) expected := "Invalid value 1-42,, for cpuset cpus" c.Assert(getErrorMessage(c, body), checker.Equals, expected) @@ -1312,7 +1312,7 @@ func (s *DockerSuite) TestPostContainersCreateWithWrongCpusetValues(c *check.C) name = "wrong-cpuset-mems" status, body, err = request.SockRequest("POST", "/containers/create?name="+name, c2, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) expected = "Invalid value 42-3,1-- for cpuset mems" c.Assert(getErrorMessage(c, body), checker.Equals, expected) } @@ -1327,7 +1327,7 @@ func (s *DockerSuite) TestPostContainersCreateShmSizeNegative(c *check.C) { status, body, err := request.SockRequest("POST", "/containers/create", config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) c.Assert(getErrorMessage(c, body), checker.Contains, "SHM size can not be less than 0") } @@ -1463,7 +1463,7 @@ func (s *DockerSuite) TestPostContainersCreateWithOomScoreAdjInvalidRange(c *che name := "oomscoreadj-over" status, b, err := request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) expected := "Invalid value 1001, range for oom score adj is [-1000, 1000]" msg := getErrorMessage(c, b) @@ -1478,7 +1478,7 @@ func (s *DockerSuite) TestPostContainersCreateWithOomScoreAdjInvalidRange(c *che name = "oomscoreadj-low" status, b, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) expected = "Invalid value -1001, range for oom score adj is [-1000, 1000]" msg = getErrorMessage(c, b) if !strings.Contains(msg, expected) { @@ -1488,10 +1488,9 @@ func (s *DockerSuite) TestPostContainersCreateWithOomScoreAdjInvalidRange(c *che // test case for #22210 where an empty container name caused panic. func (s *DockerSuite) TestContainerAPIDeleteWithEmptyName(c *check.C) { - status, out, err := request.SockRequest("DELETE", "/containers/", nil, daemonHost()) + status, _, err := request.SockRequest("DELETE", "/containers/", nil, daemonHost()) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusBadRequest) - c.Assert(string(out), checker.Contains, "No container name or ID supplied") } func (s *DockerSuite) TestContainerAPIStatsWithNetworkDisabled(c *check.C) { diff --git a/integration-cli/docker_api_create_test.go b/integration-cli/docker_api_create_test.go index c2152d32a4..58ae6e95ec 100644 --- a/integration-cli/docker_api_create_test.go +++ b/integration-cli/docker_api_create_test.go @@ -25,7 +25,7 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) { status, body, err := request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) expected := fmt.Sprintf("Interval in Healthcheck cannot be less than %s", container.MinimumDuration) c.Assert(getErrorMessage(c, body), checker.Contains, expected) @@ -41,7 +41,7 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) { } status, body, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) c.Assert(getErrorMessage(c, body), checker.Contains, expected) // test invalid Timeout in Healthcheck: less than 1ms @@ -56,7 +56,7 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) { } status, body, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) expected = fmt.Sprintf("Timeout in Healthcheck cannot be less than %s", container.MinimumDuration) c.Assert(getErrorMessage(c, body), checker.Contains, expected) @@ -72,7 +72,7 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) { } status, body, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) expected = "Retries in Healthcheck cannot be negative" c.Assert(getErrorMessage(c, body), checker.Contains, expected) @@ -89,7 +89,7 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) { } status, body, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) c.Assert(err, check.IsNil) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) expected = fmt.Sprintf("StartPeriod in Healthcheck cannot be less than %s", container.MinimumDuration) c.Assert(getErrorMessage(c, body), checker.Contains, expected) } diff --git a/integration-cli/docker_api_exec_resize_test.go b/integration-cli/docker_api_exec_resize_test.go index f43bc2de0b..169b70a4a7 100644 --- a/integration-cli/docker_api_exec_resize_test.go +++ b/integration-cli/docker_api_exec_resize_test.go @@ -22,7 +22,7 @@ func (s *DockerSuite) TestExecResizeAPIHeightWidthNoInt(c *check.C) { endpoint := "/exec/" + cleanedContainerID + "/resize?h=foo&w=bar" status, _, err := request.SockRequest("POST", endpoint, nil, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) } // Part of #14845 diff --git a/integration-cli/docker_api_exec_test.go b/integration-cli/docker_api_exec_test.go index 25399343a2..8118aa94a7 100644 --- a/integration-cli/docker_api_exec_test.go +++ b/integration-cli/docker_api_exec_test.go @@ -23,7 +23,7 @@ func (s *DockerSuite) TestExecAPICreateNoCmd(c *check.C) { status, body, err := request.SockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), map[string]interface{}{"Cmd": nil}, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) comment := check.Commentf("Expected message when creating exec command with no Cmd specified") c.Assert(getErrorMessage(c, body), checker.Contains, "No exec command specified", comment) @@ -40,7 +40,7 @@ func (s *DockerSuite) TestExecAPICreateNoValidContentType(c *check.C) { res, body, err := request.Post(fmt.Sprintf("/containers/%s/exec", name), request.RawContent(ioutil.NopCloser(jsonData)), request.ContentType("test/plain")) c.Assert(err, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) b, err := testutil.ReadBody(body) c.Assert(err, checker.IsNil) @@ -177,7 +177,7 @@ func (s *DockerSuite) TestExecAPIStartInvalidCommand(c *check.C) { dockerCmd(c, "run", "-d", "-t", "--name", name, "busybox", "/bin/sh") id := createExecCmd(c, name, "invalid") - startExec(c, id, http.StatusNotFound) + startExec(c, id, http.StatusBadRequest) waitForExec(c, id) var inspectJSON struct{ ExecIDs []string } diff --git a/integration-cli/docker_api_resize_test.go b/integration-cli/docker_api_resize_test.go index 4a07fc737b..68ac67f8e7 100644 --- a/integration-cli/docker_api_resize_test.go +++ b/integration-cli/docker_api_resize_test.go @@ -25,7 +25,7 @@ func (s *DockerSuite) TestResizeAPIHeightWidthNoInt(c *check.C) { endpoint := "/containers/" + cleanedContainerID + "/resize?h=foo&w=bar" status, _, err := request.SockRequest("POST", endpoint, nil, daemonHost()) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusBadRequest) c.Assert(err, check.IsNil) } @@ -38,7 +38,7 @@ func (s *DockerSuite) TestResizeAPIResponseWhenContainerNotStarted(c *check.C) { endpoint := "/containers/" + cleanedContainerID + "/resize?h=40&w=40" status, body, err := request.SockRequest("POST", endpoint, nil, daemonHost()) - c.Assert(status, check.Equals, http.StatusInternalServerError) + c.Assert(status, check.Equals, http.StatusConflict) c.Assert(err, check.IsNil) c.Assert(getErrorMessage(c, body), checker.Contains, "is not running", check.Commentf("resize should fail with message 'Container is not running'")) diff --git a/integration-cli/docker_api_test.go b/integration-cli/docker_api_test.go index 1af77ea513..3b8de03360 100644 --- a/integration-cli/docker_api_test.go +++ b/integration-cli/docker_api_test.go @@ -80,7 +80,7 @@ func (s *DockerSuite) TestAPIDockerAPIVersion(c *check.C) { func (s *DockerSuite) TestAPIErrorJSON(c *check.C) { httpResp, body, err := request.Post("/containers/create", request.JSONBody(struct{}{})) c.Assert(err, checker.IsNil) - c.Assert(httpResp.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(httpResp.StatusCode, checker.Equals, http.StatusBadRequest) c.Assert(httpResp.Header.Get("Content-Type"), checker.Equals, "application/json") b, err := testutil.ReadBody(body) c.Assert(err, checker.IsNil) @@ -93,7 +93,7 @@ func (s *DockerSuite) TestAPIErrorPlainText(c *check.C) { testRequires(c, DaemonIsLinux) httpResp, body, err := request.Post("/v1.23/containers/create", request.JSONBody(struct{}{})) c.Assert(err, checker.IsNil) - c.Assert(httpResp.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(httpResp.StatusCode, checker.Equals, http.StatusBadRequest) c.Assert(httpResp.Header.Get("Content-Type"), checker.Contains, "text/plain") b, err := testutil.ReadBody(body) c.Assert(err, checker.IsNil) diff --git a/integration-cli/docker_cli_cp_from_container_test.go b/integration-cli/docker_cli_cp_from_container_test.go index 116f24610e..687aec8b5f 100644 --- a/integration-cli/docker_cli_cp_from_container_test.go +++ b/integration-cli/docker_cli_cp_from_container_test.go @@ -64,19 +64,17 @@ func (s *DockerSuite) TestCpFromErrDstParentNotExists(c *check.C) { // Try with a file source. srcPath := containerCpPath(containerID, "/file1") dstPath := cpPath(tmpDir, "notExists", "file1") + _, dstStatErr := os.Lstat(filepath.Dir(dstPath)) + c.Assert(os.IsNotExist(dstStatErr), checker.True) err := runDockerCp(c, srcPath, dstPath, nil) - c.Assert(err, checker.NotNil) - - c.Assert(isCpNotExist(err), checker.True, check.Commentf("expected IsNotExist error, but got %T: %s", err, err)) + c.Assert(err.Error(), checker.Contains, dstStatErr.Error()) // Try with a directory source. srcPath = containerCpPath(containerID, "/dir1") err = runDockerCp(c, srcPath, dstPath, nil) - c.Assert(err, checker.NotNil) - - c.Assert(isCpNotExist(err), checker.True, check.Commentf("expected IsNotExist error, but got %T: %s", err, err)) + c.Assert(err.Error(), checker.Contains, dstStatErr.Error()) } // Test for error when DST ends in a trailing diff --git a/integration-cli/docker_cli_cp_to_container_test.go b/integration-cli/docker_cli_cp_to_container_test.go index 97e9aa1230..57a850c425 100644 --- a/integration-cli/docker_cli_cp_to_container_test.go +++ b/integration-cli/docker_cli_cp_to_container_test.go @@ -2,6 +2,7 @@ package main import ( "os" + "strings" "github.com/docker/docker/integration-cli/checker" "github.com/go-check/check" @@ -30,11 +31,11 @@ func (s *DockerSuite) TestCpToErrSrcNotExists(c *check.C) { srcPath := cpPath(tmpDir, "file1") dstPath := containerCpPath(containerID, "file1") + _, srcStatErr := os.Stat(srcPath) + c.Assert(os.IsNotExist(srcStatErr), checker.True) err := runDockerCp(c, srcPath, dstPath, nil) - c.Assert(err, checker.NotNil) - - c.Assert(isCpNotExist(err), checker.True, check.Commentf("expected IsNotExist error, but got %T: %s", err, err)) + c.Assert(strings.ToLower(err.Error()), checker.Contains, strings.ToLower(srcStatErr.Error())) } // Test for error when SRC ends in a trailing diff --git a/integration-cli/docker_cli_cp_utils_test.go b/integration-cli/docker_cli_cp_utils_test.go index 48aff90617..e517fc0f37 100644 --- a/integration-cli/docker_cli_cp_utils_test.go +++ b/integration-cli/docker_cli_cp_utils_test.go @@ -231,7 +231,7 @@ func getTestDir(c *check.C, label string) (tmpDir string) { } func isCpNotExist(err error) bool { - return strings.Contains(err.Error(), "no such file or directory") || strings.Contains(err.Error(), "cannot find the file specified") + return strings.Contains(strings.ToLower(err.Error()), "could not find the file") } func isCpDirNotExist(err error) bool { diff --git a/integration-cli/docker_cli_inspect_test.go b/integration-cli/docker_cli_inspect_test.go index 96e2ee4512..7d3509a5d7 100644 --- a/integration-cli/docker_cli_inspect_test.go +++ b/integration-cli/docker_cli_inspect_test.go @@ -463,6 +463,5 @@ func (s *DockerSuite) TestInspectInvalidReference(c *check.C) { // This test should work on both Windows and Linux out, _, err := dockerCmdWithError("inspect", "FooBar") c.Assert(err, checker.NotNil) - c.Assert(out, checker.Contains, "Error: No such object: FooBar") - c.Assert(err.Error(), checker.Contains, "Error: No such object: FooBar") + c.Assert(out, checker.Contains, "no such image: FooBar") } diff --git a/integration-cli/docker_cli_links_test.go b/integration-cli/docker_cli_links_test.go index b43c6d1fba..7c9ce739ce 100644 --- a/integration-cli/docker_cli_links_test.go +++ b/integration-cli/docker_cli_links_test.go @@ -28,7 +28,7 @@ func (s *DockerSuite) TestLinksInvalidContainerTarget(c *check.C) { // an invalid container target should produce an error c.Assert(err, checker.NotNil, check.Commentf("out: %s", out)) // an invalid container target should produce an error - c.Assert(out, checker.Contains, "Could not get container") + c.Assert(out, checker.Contains, "could not get container") } func (s *DockerSuite) TestLinksPingLinkedContainers(c *check.C) { diff --git a/integration-cli/docker_cli_netmode_test.go b/integration-cli/docker_cli_netmode_test.go index deb8f69160..abf1ff2cf7 100644 --- a/integration-cli/docker_cli_netmode_test.go +++ b/integration-cli/docker_cli_netmode_test.go @@ -1,6 +1,8 @@ package main import ( + "strings" + "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/runconfig" "github.com/go-check/check" @@ -47,7 +49,7 @@ func (s *DockerSuite) TestNetHostname(c *check.C) { c.Assert(out, checker.Contains, "Invalid network mode: invalid container format container:") out, _ = dockerCmdWithFail(c, "run", "--net=weird", "busybox", "ps") - c.Assert(out, checker.Contains, "network weird not found") + c.Assert(strings.ToLower(out), checker.Contains, "no such network") } func (s *DockerSuite) TestConflictContainerNetworkAndLinks(c *check.C) { diff --git a/integration-cli/docker_cli_ps_test.go b/integration-cli/docker_cli_ps_test.go index adf0cf48be..e44547f3bb 100644 --- a/integration-cli/docker_cli_ps_test.go +++ b/integration-cli/docker_cli_ps_test.go @@ -208,7 +208,7 @@ func (s *DockerSuite) TestPsListContainersFilterStatus(c *check.C) { result := cli.Docker(cli.Args("ps", "-a", "-q", "--filter=status=rubbish"), cli.WithTimeout(time.Second*60)) c.Assert(result, icmd.Matches, icmd.Expected{ ExitCode: 1, - Err: "Unrecognised filter value for status", + Err: "Invalid filter 'status=rubbish'", }) // Windows doesn't support pausing of containers diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index a0bb7a2283..3e39f47bf2 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -1887,7 +1887,7 @@ func (s *DockerSwarmSuite) TestNetworkInspectWithDuplicateNames(c *check.C) { // Name with duplicates out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) c.Assert(err, checker.NotNil, check.Commentf(out)) - c.Assert(out, checker.Contains, "network foo is ambiguous (2 matches found based on name)") + c.Assert(out, checker.Contains, "2 matches found based on name") out, err = d.Cmd("network", "rm", n2.ID) c.Assert(err, checker.IsNil, check.Commentf(out)) @@ -1912,7 +1912,7 @@ func (s *DockerSwarmSuite) TestNetworkInspectWithDuplicateNames(c *check.C) { // Name with duplicates out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) c.Assert(err, checker.NotNil, check.Commentf(out)) - c.Assert(out, checker.Contains, "network foo is ambiguous (2 matches found based on name)") + c.Assert(out, checker.Contains, "2 matches found based on name") } func (s *DockerSwarmSuite) TestSwarmStopSignal(c *check.C) { diff --git a/integration-cli/docker_deprecated_api_v124_test.go b/integration-cli/docker_deprecated_api_v124_test.go index a3a8fbdb6c..db2ce10e48 100644 --- a/integration-cli/docker_deprecated_api_v124_test.go +++ b/integration-cli/docker_deprecated_api_v124_test.go @@ -23,10 +23,9 @@ func (s *DockerSuite) TestDeprecatedContainerAPIStartHostConfig(c *check.C) { config := map[string]interface{}{ "Binds": []string{"/aa:/bb"}, } - status, body, err := request.SockRequest("POST", "/containers/"+name+"/start", config, daemonHost()) + status, _, err := request.SockRequest("POST", "/containers/"+name+"/start", config, daemonHost()) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusBadRequest) - c.Assert(string(body), checker.Contains, "was deprecated since v1.10") } func (s *DockerSuite) TestDeprecatedContainerAPIStartVolumeBinds(c *check.C) { @@ -81,7 +80,7 @@ func (s *DockerSuite) TestDeprecatedContainerAPIStartDupVolumeBinds(c *check.C) } status, body, err := request.SockRequest("POST", formatV123StartAPIURL("/containers/"+name+"/start"), config, daemonHost()) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) c.Assert(string(body), checker.Contains, "Duplicate mount point", check.Commentf("Expected failure due to duplicate bind mounts to same path, instead got: %q with error: %v", string(body), err)) } @@ -154,7 +153,7 @@ func (s *DockerSuite) TestDeprecatedStartWithTooLowMemoryLimit(c *check.C) { c.Assert(err, checker.IsNil) b, err2 := testutil.ReadBody(body) c.Assert(err2, checker.IsNil) - c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(res.StatusCode, checker.Equals, http.StatusBadRequest) c.Assert(string(b), checker.Contains, "Minimum memory limit allowed is 4MB") } diff --git a/opts/env.go b/opts/env.go index e6ddd73309..4fbd470bcf 100644 --- a/opts/env.go +++ b/opts/env.go @@ -5,6 +5,8 @@ import ( "os" "runtime" "strings" + + "github.com/pkg/errors" ) // ValidateEnv validates an environment variable and returns it. @@ -18,7 +20,7 @@ import ( func ValidateEnv(val string) (string, error) { arr := strings.Split(val, "=") if arr[0] == "" { - return "", fmt.Errorf("invalid environment variable: %s", val) + return "", errors.Errorf("invalid environment variable: %s", val) } if len(arr) > 1 { return val, nil diff --git a/pkg/authorization/authz.go b/pkg/authorization/authz.go index b52446add6..924908af9b 100644 --- a/pkg/authorization/authz.go +++ b/pkg/authorization/authz.go @@ -176,10 +176,7 @@ type authorizationError struct { error } -// HTTPErrorStatusCode returns the authorization error status code (forbidden) -func (e authorizationError) HTTPErrorStatusCode() int { - return http.StatusForbidden -} +func (authorizationError) Forbidden() {} func newAuthorizationError(plugin, msg string) authorizationError { return authorizationError{error: fmt.Errorf("authorization denied by plugin %s: %s", plugin, msg)} diff --git a/plugin/backend_linux.go b/plugin/backend_linux.go index 727a26862a..e70dcdbb06 100644 --- a/plugin/backend_linux.go +++ b/plugin/backend_linux.go @@ -6,7 +6,6 @@ import ( "archive/tar" "compress/gzip" "encoding/json" - "fmt" "io" "io/ioutil" "net/http" @@ -55,7 +54,7 @@ func (pm *Manager) Disable(refOrID string, config *types.PluginDisableConfig) er pm.mu.RUnlock() if !config.ForceDisable && p.GetRefCount() > 0 { - return fmt.Errorf("plugin %s is in use", p.Name()) + return errors.WithStack(inUseError(p.Name())) } for _, typ := range p.GetTypes() { @@ -142,7 +141,7 @@ func (s *tempConfigStore) Put(c []byte) (digest.Digest, error) { func (s *tempConfigStore) Get(d digest.Digest) ([]byte, error) { if d != s.configDigest { - return nil, fmt.Errorf("digest not found") + return nil, errNotFound("digest not found") } return s.config, nil } @@ -151,7 +150,7 @@ func (s *tempConfigStore) RootFSAndPlatformFromConfig(c []byte) (*image.RootFS, return configToRootFS(c) } -func computePrivileges(c types.PluginConfig) (types.PluginPrivileges, error) { +func computePrivileges(c types.PluginConfig) types.PluginPrivileges { var privileges types.PluginPrivileges if c.Network.Type != "null" && c.Network.Type != "bridge" && c.Network.Type != "" { privileges = append(privileges, types.PluginPrivilege{ @@ -207,7 +206,7 @@ func computePrivileges(c types.PluginConfig) (types.PluginPrivileges, error) { }) } - return privileges, nil + return privileges } // Privileges pulls a plugin config and computes the privileges required to install it. @@ -236,21 +235,21 @@ func (pm *Manager) Privileges(ctx context.Context, ref reference.Named, metaHead } var config types.PluginConfig if err := json.Unmarshal(cs.config, &config); err != nil { - return nil, err + return nil, systemError{err} } - return computePrivileges(config) + return computePrivileges(config), nil } // Upgrade upgrades a plugin func (pm *Manager) Upgrade(ctx context.Context, ref reference.Named, name string, metaHeader http.Header, authConfig *types.AuthConfig, privileges types.PluginPrivileges, outStream io.Writer) (err error) { p, err := pm.config.Store.GetV2Plugin(name) if err != nil { - return errors.Wrap(err, "plugin must be installed before upgrading") + return err } if p.IsEnabled() { - return fmt.Errorf("plugin must be disabled before upgrading") + return errors.Wrap(enabledError(p.Name()), "plugin must be disabled before upgrading") } pm.muGC.RLock() @@ -258,12 +257,12 @@ func (pm *Manager) Upgrade(ctx context.Context, ref reference.Named, name string // revalidate because Pull is public if _, err := reference.ParseNormalizedNamed(name); err != nil { - return errors.Wrapf(err, "failed to parse %q", name) + return errors.Wrapf(validationError{err}, "failed to parse %q", name) } tmpRootFSDir, err := ioutil.TempDir(pm.tmpDir(), ".rootfs") if err != nil { - return err + return errors.Wrap(systemError{err}, "error preparing upgrade") } defer os.RemoveAll(tmpRootFSDir) @@ -305,17 +304,17 @@ func (pm *Manager) Pull(ctx context.Context, ref reference.Named, name string, m // revalidate because Pull is public nameref, err := reference.ParseNormalizedNamed(name) if err != nil { - return errors.Wrapf(err, "failed to parse %q", name) + return errors.Wrapf(validationError{err}, "failed to parse %q", name) } name = reference.FamiliarString(reference.TagNameOnly(nameref)) if err := pm.config.Store.validateName(name); err != nil { - return err + return validationError{err} } tmpRootFSDir, err := ioutil.TempDir(pm.tmpDir(), ".rootfs") if err != nil { - return err + return errors.Wrap(systemError{err}, "error preparing pull") } defer os.RemoveAll(tmpRootFSDir) @@ -372,7 +371,7 @@ func (pm *Manager) List(pluginFilters filters.Args) ([]types.Plugin, error) { } else if pluginFilters.ExactMatch("enabled", "false") { disabledOnly = true } else { - return nil, fmt.Errorf("Invalid filter 'enabled=%s'", pluginFilters.Get("enabled")) + return nil, invalidFilter{"enabled", pluginFilters.Get("enabled")} } } @@ -615,10 +614,10 @@ func (pm *Manager) Remove(name string, config *types.PluginRmConfig) error { if !config.ForceRemove { if p.GetRefCount() > 0 { - return fmt.Errorf("plugin %s is in use", p.Name()) + return inUseError(p.Name()) } if p.IsEnabled() { - return fmt.Errorf("plugin %s is enabled", p.Name()) + return enabledError(p.Name()) } } diff --git a/plugin/errors.go b/plugin/errors.go new file mode 100644 index 0000000000..0a101d4dc5 --- /dev/null +++ b/plugin/errors.go @@ -0,0 +1,94 @@ +package plugin + +import "fmt" + +type errNotFound string + +func (name errNotFound) Error() string { + return fmt.Sprintf("plugin %q not found", string(name)) +} + +func (errNotFound) NotFound() {} + +type errAmbiguous string + +func (name errAmbiguous) Error() string { + return fmt.Sprintf("multiple plugins found for %q", string(name)) +} + +func (name errAmbiguous) InvalidParameter() {} + +type errDisabled string + +func (name errDisabled) Error() string { + return fmt.Sprintf("plugin %s found but disabled", string(name)) +} + +func (name errDisabled) Conflict() {} + +type validationError struct { + cause error +} + +func (e validationError) Error() string { + return e.cause.Error() +} + +func (validationError) Conflict() {} + +func (e validationError) Cause() error { + return e.cause +} + +type systemError struct { + cause error +} + +func (e systemError) Error() string { + return e.cause.Error() +} + +func (systemError) SystemError() {} + +func (e systemError) Cause() error { + return e.cause +} + +type invalidFilter struct { + filter string + value []string +} + +func (e invalidFilter) Error() string { + msg := "Invalid filter '" + e.filter + if len(e.value) > 0 { + msg += fmt.Sprintf("=%s", e.value) + } + return msg + "'" +} + +func (invalidFilter) InvalidParameter() {} + +type inUseError string + +func (e inUseError) Error() string { + return "plugin " + string(e) + " is in use" +} + +func (inUseError) Conflict() {} + +type enabledError string + +func (e enabledError) Error() string { + return "plugin " + string(e) + " is enabled" +} + +func (enabledError) Conflict() {} + +type alreadyExistsError string + +func (e alreadyExistsError) Error() string { + return "plugin " + string(e) + " already exists" +} + +func (alreadyExistsError) Conflict() {} diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index 301b12c814..f1364e071a 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -4,7 +4,6 @@ package plugin import ( "encoding/json" - "fmt" "net" "os" "path/filepath" @@ -28,7 +27,7 @@ import ( func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { p.Rootfs = filepath.Join(pm.config.Root, p.PluginObj.ID, "rootfs") if p.IsEnabled() && !force { - return fmt.Errorf("plugin %s is already enabled", p.Name()) + return errors.Wrap(enabledError(p.Name()), "plugin already enabled") } spec, err := p.InitSpec(pm.config.ExecRoot) if err != nil { @@ -171,7 +170,7 @@ func setupRoot(root string) error { func (pm *Manager) disable(p *v2.Plugin, c *controller) error { if !p.IsEnabled() { - return fmt.Errorf("plugin %s is already disabled", p.Name()) + return errors.Wrap(errDisabled(p.Name()), "plugin is already disabled") } c.restart = false @@ -213,12 +212,12 @@ func (pm *Manager) upgradePlugin(p *v2.Plugin, configDigest digest.Digest, blobs // This could happen if the plugin was disabled with `-f` with active mounts. // If there is anything in `orig` is still mounted, this should error out. if err := mount.RecursiveUnmount(orig); err != nil { - return err + return systemError{err} } backup := orig + "-old" if err := os.Rename(orig, backup); err != nil { - return errors.Wrap(err, "error backing up plugin data before upgrade") + return errors.Wrap(systemError{err}, "error backing up plugin data before upgrade") } defer func() { @@ -244,7 +243,7 @@ func (pm *Manager) upgradePlugin(p *v2.Plugin, configDigest digest.Digest, blobs }() if err := os.Rename(tmpRootFSDir, orig); err != nil { - return errors.Wrap(err, "error upgrading") + return errors.Wrap(systemError{err}, "error upgrading") } p.PluginObj.Config = config @@ -268,7 +267,7 @@ func (pm *Manager) setupNewPlugin(configDigest digest.Digest, blobsums []digest. return types.PluginConfig{}, errors.New("invalid config json") } - requiredPrivileges, err := computePrivileges(config) + requiredPrivileges := computePrivileges(config) if err != nil { return types.PluginConfig{}, err } @@ -284,7 +283,7 @@ func (pm *Manager) setupNewPlugin(configDigest digest.Digest, blobsums []digest. // createPlugin creates a new plugin. take lock before calling. func (pm *Manager) createPlugin(name string, configDigest digest.Digest, blobsums []digest.Digest, rootFSDir string, privileges *types.PluginPrivileges, opts ...CreateOpt) (p *v2.Plugin, err error) { if err := pm.config.Store.validateName(name); err != nil { // todo: this check is wrong. remove store - return nil, err + return nil, validationError{err} } config, err := pm.setupNewPlugin(configDigest, blobsums, privileges) diff --git a/plugin/store.go b/plugin/store.go index acb92d2e5a..adc0e26503 100644 --- a/plugin/store.go +++ b/plugin/store.go @@ -24,25 +24,6 @@ const allowV1PluginsFallback bool = true */ const defaultAPIVersion string = "1.0" -// ErrNotFound indicates that a plugin was not found locally. -type ErrNotFound string - -func (name ErrNotFound) Error() string { return fmt.Sprintf("plugin %q not found", string(name)) } - -// ErrAmbiguous indicates that more than one plugin was found -type ErrAmbiguous string - -func (name ErrAmbiguous) Error() string { - return fmt.Sprintf("multiple plugins found for %q", string(name)) -} - -// ErrDisabled indicates that a plugin was found but it is disabled -type ErrDisabled string - -func (name ErrDisabled) Error() string { - return fmt.Sprintf("plugin %s found but disabled", string(name)) -} - // GetV2Plugin retrieves a plugin by name, id or partial ID. func (ps *Store) GetV2Plugin(refOrID string) (*v2.Plugin, error) { ps.RLock() @@ -55,7 +36,7 @@ func (ps *Store) GetV2Plugin(refOrID string) (*v2.Plugin, error) { p, idOk := ps.plugins[id] if !idOk { - return nil, errors.WithStack(ErrNotFound(id)) + return nil, errors.WithStack(errNotFound(id)) } return p, nil @@ -65,7 +46,7 @@ func (ps *Store) GetV2Plugin(refOrID string) (*v2.Plugin, error) { func (ps *Store) validateName(name string) error { for _, p := range ps.plugins { if p.Name() == name { - return errors.Errorf("plugin %q already exists", name) + return alreadyExistsError(name) } } return nil @@ -145,9 +126,9 @@ func (ps *Store) Get(name, capability string, mode int) (plugingetter.CompatPlug } // Plugin was found but it is disabled, so we should not fall back to legacy plugins // but we should error out right away - return nil, ErrDisabled(name) + return nil, errDisabled(name) } - if _, ok := errors.Cause(err).(ErrNotFound); !ok { + if _, ok := errors.Cause(err).(errNotFound); !ok { return nil, err } } @@ -156,7 +137,10 @@ func (ps *Store) Get(name, capability string, mode int) (plugingetter.CompatPlug if allowV1PluginsFallback { p, err := plugins.Get(name, capability) if err != nil { - return nil, fmt.Errorf("legacy plugin: %v", err) + if errors.Cause(err) == plugins.ErrNotFound { + return nil, errNotFound(name) + } + return nil, errors.Wrap(systemError{err}, "legacy plugin") } return p, nil } @@ -189,7 +173,7 @@ func (ps *Store) GetAllByCap(capability string) ([]plugingetter.CompatPlugin, er if allowV1PluginsFallback { pl, err := plugins.GetAll(capability) if err != nil { - return nil, fmt.Errorf("legacy plugin: %v", err) + return nil, errors.Wrap(systemError{err}, "legacy plugin") } for _, p := range pl { result = append(result, p) @@ -239,11 +223,11 @@ func (ps *Store) resolvePluginID(idOrName string) (string, error) { ref, err := reference.ParseNormalizedNamed(idOrName) if err != nil { - return "", errors.WithStack(ErrNotFound(idOrName)) + return "", errors.WithStack(errNotFound(idOrName)) } if _, ok := ref.(reference.Canonical); ok { logrus.Warnf("canonical references cannot be resolved: %v", reference.FamiliarString(ref)) - return "", errors.WithStack(ErrNotFound(idOrName)) + return "", errors.WithStack(errNotFound(idOrName)) } ref = reference.TagNameOnly(ref) @@ -258,13 +242,13 @@ func (ps *Store) resolvePluginID(idOrName string) (string, error) { for id, p := range ps.plugins { // this can be optimized if strings.HasPrefix(id, idOrName) { if found != nil { - return "", errors.WithStack(ErrAmbiguous(idOrName)) + return "", errors.WithStack(errAmbiguous(idOrName)) } found = p } } if found == nil { - return "", errors.WithStack(ErrNotFound(idOrName)) + return "", errors.WithStack(errNotFound(idOrName)) } return found.PluginObj.ID, nil } diff --git a/reference/errors.go b/reference/errors.go new file mode 100644 index 0000000000..a16f124e9f --- /dev/null +++ b/reference/errors.go @@ -0,0 +1,25 @@ +package reference + +type notFoundError string + +func (e notFoundError) Error() string { + return string(e) +} + +func (notFoundError) NotFound() {} + +type invalidTagError string + +func (e invalidTagError) Error() string { + return string(e) +} + +func (invalidTagError) InvalidParameter() {} + +type conflictingTagError string + +func (e conflictingTagError) Error() string { + return string(e) +} + +func (conflictingTagError) Conflict() {} diff --git a/reference/store.go b/reference/store.go index 5b68c437c8..b98f9579db 100644 --- a/reference/store.go +++ b/reference/store.go @@ -2,7 +2,6 @@ package reference import ( "encoding/json" - "errors" "fmt" "os" "path/filepath" @@ -12,12 +11,13 @@ import ( "github.com/docker/distribution/reference" "github.com/docker/docker/pkg/ioutils" "github.com/opencontainers/go-digest" + "github.com/pkg/errors" ) var ( // ErrDoesNotExist is returned if a reference is not found in the // store. - ErrDoesNotExist = errors.New("reference does not exist") + ErrDoesNotExist notFoundError = "reference does not exist" ) // An Association is a tuple associating a reference with an image ID. @@ -100,7 +100,7 @@ func NewReferenceStore(jsonPath, platform string) (Store, error) { // references can be overwritten. This only works for tags, not digests. func (store *store) AddTag(ref reference.Named, id digest.Digest, force bool) error { if _, isCanonical := ref.(reference.Canonical); isCanonical { - return errors.New("refusing to create a tag with a digest reference") + return errors.WithStack(invalidTagError("refusing to create a tag with a digest reference")) } return store.addReference(reference.TagNameOnly(ref), id, force) } @@ -138,7 +138,7 @@ func (store *store) addReference(ref reference.Named, id digest.Digest, force bo refStr := reference.FamiliarString(ref) if refName == string(digest.Canonical) { - return errors.New("refusing to create an ambiguous tag using digest algorithm as name") + return errors.WithStack(invalidTagError("refusing to create an ambiguous tag using digest algorithm as name")) } store.mu.Lock() @@ -155,11 +155,15 @@ func (store *store) addReference(ref reference.Named, id digest.Digest, force bo if exists { // force only works for tags if digested, isDigest := ref.(reference.Canonical); isDigest { - return fmt.Errorf("Cannot overwrite digest %s", digested.Digest().String()) + return errors.WithStack(conflictingTagError("Cannot overwrite digest " + digested.Digest().String())) } if !force { - return fmt.Errorf("Conflict: Tag %s is already set to image %s, if you want to replace it, please use -f option", refStr, oldID.String()) + return errors.WithStack( + conflictingTagError( + fmt.Sprintf("Conflict: Tag %s is already set to image %s, if you want to replace it, please use the force option", refStr, oldID.String()), + ), + ) } if store.referencesByIDCache[oldID] != nil { diff --git a/registry/auth.go b/registry/auth.go index f7a8fdc936..493626bcdc 100644 --- a/registry/auth.go +++ b/registry/auth.go @@ -1,7 +1,6 @@ package registry import ( - "fmt" "io/ioutil" "net/http" "net/url" @@ -13,6 +12,7 @@ import ( "github.com/docker/distribution/registry/client/transport" "github.com/docker/docker/api/types" registrytypes "github.com/docker/docker/api/types/registry" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -23,21 +23,15 @@ const ( // loginV1 tries to register/login to the v1 registry server. func loginV1(authConfig *types.AuthConfig, apiEndpoint APIEndpoint, userAgent string) (string, string, error) { - registryEndpoint, err := apiEndpoint.ToV1Endpoint(userAgent, nil) - if err != nil { - return "", "", err - } - + registryEndpoint := apiEndpoint.ToV1Endpoint(userAgent, nil) serverAddress := registryEndpoint.String() logrus.Debugf("attempting v1 login to registry endpoint %s", serverAddress) if serverAddress == "" { - return "", "", fmt.Errorf("Server Error: Server Address not set.") + return "", "", systemError{errors.New("Server Error: Server Address not set.")} } - loginAgainstOfficialIndex := serverAddress == IndexServer - req, err := http.NewRequest("GET", serverAddress+"users/", nil) if err != nil { return "", "", err @@ -53,27 +47,23 @@ func loginV1(authConfig *types.AuthConfig, apiEndpoint APIEndpoint, userAgent st defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) if err != nil { - return "", "", err + return "", "", systemError{err} } - if resp.StatusCode == http.StatusOK { + + switch resp.StatusCode { + case http.StatusOK: return "Login Succeeded", "", nil - } else if resp.StatusCode == http.StatusUnauthorized { - if loginAgainstOfficialIndex { - return "", "", fmt.Errorf("Wrong login/password, please try again. Haven't got a Docker ID? Create one at https://hub.docker.com") - } - return "", "", fmt.Errorf("Wrong login/password, please try again") - } else if resp.StatusCode == http.StatusForbidden { - if loginAgainstOfficialIndex { - return "", "", fmt.Errorf("Login: Account is not active. Please check your e-mail for a confirmation link.") - } + case http.StatusUnauthorized: + return "", "", unauthorizedError{errors.New("Wrong login/password, please try again")} + case http.StatusForbidden: // *TODO: Use registry configuration to determine what this says, if anything? - return "", "", fmt.Errorf("Login: Account is not active. Please see the documentation of the registry %s for instructions how to activate it.", serverAddress) - } else if resp.StatusCode == http.StatusInternalServerError { // Issue #14326 + return "", "", notActivatedError{errors.Errorf("Login: Account is not active. Please see the documentation of the registry %s for instructions how to activate it.", serverAddress)} + case http.StatusInternalServerError: logrus.Errorf("%s returned status code %d. Response Body :\n%s", req.URL.String(), resp.StatusCode, body) - return "", "", fmt.Errorf("Internal Server Error") + return "", "", systemError{errors.New("Internal Server Error")} } - return "", "", fmt.Errorf("Login: %s (Code: %d; Headers: %s)", body, - resp.StatusCode, resp.Header) + return "", "", systemError{errors.Errorf("Login: %s (Code: %d; Headers: %s)", body, + resp.StatusCode, resp.Header)} } type loginCredentialStore struct { @@ -159,24 +149,25 @@ func loginV2(authConfig *types.AuthConfig, endpoint APIEndpoint, userAgent strin resp, err := loginClient.Do(req) if err != nil { + err = translateV2AuthError(err) if !foundV2 { err = fallbackError{err: err} } + return "", "", err } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - // TODO(dmcgowan): Attempt to further interpret result, status code and error code string - err := fmt.Errorf("login attempt to %s failed with status: %d %s", endpointStr, resp.StatusCode, http.StatusText(resp.StatusCode)) - if !foundV2 { - err = fallbackError{err: err} - } - return "", "", err + if resp.StatusCode == http.StatusOK { + return "Login Succeeded", credentialAuthConfig.IdentityToken, nil } - return "Login Succeeded", credentialAuthConfig.IdentityToken, nil - + // TODO(dmcgowan): Attempt to further interpret result, status code and error code string + err = errors.Errorf("login attempt to %s failed with status: %d %s", endpointStr, resp.StatusCode, http.StatusText(resp.StatusCode)) + if !foundV2 { + err = fallbackError{err: err} + } + return "", "", err } func v2AuthHTTPClient(endpoint *url.URL, authTransport http.RoundTripper, modifiers []transport.RequestModifier, creds auth.CredentialStore, scopes []auth.Scope) (*http.Client, bool, error) { diff --git a/registry/endpoint_v1.go b/registry/endpoint_v1.go index 59fd72e938..d6a51bfaf7 100644 --- a/registry/endpoint_v1.go +++ b/registry/endpoint_v1.go @@ -67,7 +67,7 @@ func validateEndpoint(endpoint *V1Endpoint) error { return nil } -func newV1Endpoint(address url.URL, tlsConfig *tls.Config, userAgent string, metaHeaders http.Header) (*V1Endpoint, error) { +func newV1Endpoint(address url.URL, tlsConfig *tls.Config, userAgent string, metaHeaders http.Header) *V1Endpoint { endpoint := &V1Endpoint{ IsSecure: (tlsConfig == nil || !tlsConfig.InsecureSkipVerify), URL: new(url.URL), @@ -78,7 +78,7 @@ func newV1Endpoint(address url.URL, tlsConfig *tls.Config, userAgent string, met // TODO(tiborvass): make sure a ConnectTimeout transport is used tr := NewTransport(tlsConfig) endpoint.client = HTTPClient(transport.NewTransport(tr, DockerHeaders(userAgent, metaHeaders)...)) - return endpoint, nil + return endpoint } // trimV1Address trims the version off the address and returns the @@ -123,7 +123,7 @@ func newV1EndpointFromStr(address string, tlsConfig *tls.Config, userAgent strin return nil, err } - endpoint, err := newV1Endpoint(*uri, tlsConfig, userAgent, metaHeaders) + endpoint := newV1Endpoint(*uri, tlsConfig, userAgent, metaHeaders) if err != nil { return nil, err } diff --git a/registry/errors.go b/registry/errors.go new file mode 100644 index 0000000000..b388efca73 --- /dev/null +++ b/registry/errors.go @@ -0,0 +1,86 @@ +package registry + +import ( + "net/url" + + "github.com/docker/distribution/registry/api/errcode" +) + +type notFoundError string + +func (e notFoundError) Error() string { + return string(e) +} + +func (notFoundError) NotFound() {} + +type validationError struct { + cause error +} + +func (e validationError) Error() string { + return e.cause.Error() +} + +func (e validationError) InvalidParameter() {} + +func (e validationError) Cause() error { + return e.cause +} + +type unauthorizedError struct { + cause error +} + +func (e unauthorizedError) Error() string { + return e.cause.Error() +} + +func (e unauthorizedError) Unauthorized() {} + +func (e unauthorizedError) Cause() error { + return e.cause +} + +type systemError struct { + cause error +} + +func (e systemError) Error() string { + return e.cause.Error() +} + +func (e systemError) SystemError() {} + +func (e systemError) Cause() error { + return e.cause +} + +type notActivatedError struct { + cause error +} + +func (e notActivatedError) Error() string { + return e.cause.Error() +} + +func (e notActivatedError) Forbidden() {} + +func (e notActivatedError) Cause() error { + return e.cause +} + +func translateV2AuthError(err error) error { + switch e := err.(type) { + case *url.Error: + switch e2 := e.Err.(type) { + case errcode.Error: + switch e2.Code { + case errcode.ErrorCodeUnauthorized: + return unauthorizedError{err} + } + } + } + + return err +} diff --git a/registry/service.go b/registry/service.go index b4f747380a..d36b11f0e1 100644 --- a/registry/service.go +++ b/registry/service.go @@ -2,7 +2,6 @@ package registry import ( "crypto/tls" - "fmt" "net/http" "net/url" "strings" @@ -14,6 +13,7 @@ import ( "github.com/docker/distribution/registry/client/auth" "github.com/docker/docker/api/types" registrytypes "github.com/docker/docker/api/types/registry" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -117,12 +117,12 @@ func (s *DefaultService) Auth(ctx context.Context, authConfig *types.AuthConfig, } u, err := url.Parse(serverAddress) if err != nil { - return "", "", fmt.Errorf("unable to parse server address: %v", err) + return "", "", validationError{errors.Errorf("unable to parse server address: %v", err)} } endpoints, err := s.LookupPushEndpoints(u.Host) if err != nil { - return "", "", err + return "", "", validationError{err} } for _, endpoint := range endpoints { @@ -140,6 +140,7 @@ func (s *DefaultService) Auth(ctx context.Context, authConfig *types.AuthConfig, logrus.Infof("Error logging in to %s endpoint, trying next endpoint: %v", endpoint.Version, err) continue } + return "", "", err } @@ -258,7 +259,7 @@ type APIEndpoint struct { } // ToV1Endpoint returns a V1 API endpoint based on the APIEndpoint -func (e APIEndpoint) ToV1Endpoint(userAgent string, metaHeaders http.Header) (*V1Endpoint, error) { +func (e APIEndpoint) ToV1Endpoint(userAgent string, metaHeaders http.Header) *V1Endpoint { return newV1Endpoint(*e.URL, e.TLSConfig, userAgent, metaHeaders) } diff --git a/registry/session.go b/registry/session.go index bc4a244032..eaa5aec750 100644 --- a/registry/session.go +++ b/registry/session.go @@ -3,7 +3,6 @@ package registry import ( "bytes" "crypto/sha256" - "errors" "sync" // this is required for some certificates _ "crypto/sha512" @@ -27,13 +26,14 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/tarsum" "github.com/docker/docker/registry/resumable" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) var ( // ErrRepoNotFound is returned if the repository didn't exist on the // remote side - ErrRepoNotFound = errors.New("Repository not found") + ErrRepoNotFound notFoundError = "Repository not found" ) // A Session is used to communicate with a V1 registry @@ -734,27 +734,27 @@ func shouldRedirect(response *http.Response) bool { // SearchRepositories performs a search against the remote repository func (r *Session) SearchRepositories(term string, limit int) (*registrytypes.SearchResults, error) { if limit < 1 || limit > 100 { - return nil, fmt.Errorf("Limit %d is outside the range of [1, 100]", limit) + return nil, validationError{errors.Errorf("Limit %d is outside the range of [1, 100]", limit)} } logrus.Debugf("Index server: %s", r.indexEndpoint) u := r.indexEndpoint.String() + "search?q=" + url.QueryEscape(term) + "&n=" + url.QueryEscape(fmt.Sprintf("%d", limit)) req, err := http.NewRequest("GET", u, nil) if err != nil { - return nil, fmt.Errorf("Error while getting from the server: %v", err) + return nil, errors.Wrap(validationError{err}, "Error building request") } // Have the AuthTransport send authentication, when logged in. req.Header.Set("X-Docker-Token", "true") res, err := r.client.Do(req) if err != nil { - return nil, err + return nil, systemError{err} } defer res.Body.Close() if res.StatusCode != 200 { return nil, newJSONError(fmt.Sprintf("Unexpected status code %d", res.StatusCode), res) } result := new(registrytypes.SearchResults) - return result, json.NewDecoder(res.Body).Decode(result) + return result, errors.Wrap(json.NewDecoder(res.Body).Decode(result), "error decoding registry search results") } func isTimeout(err error) bool { diff --git a/runconfig/config.go b/runconfig/config.go index c9dc6e96ea..ebb6d3f6b9 100644 --- a/runconfig/config.go +++ b/runconfig/config.go @@ -2,13 +2,13 @@ package runconfig import ( "encoding/json" - "fmt" "io" "github.com/docker/docker/api/types/container" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/volume" + "github.com/pkg/errors" ) // ContainerDecoder implements httputils.ContainerDecoder @@ -17,19 +17,27 @@ type ContainerDecoder struct{} // DecodeConfig makes ContainerDecoder to implement httputils.ContainerDecoder func (r ContainerDecoder) DecodeConfig(src io.Reader) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig, error) { - return DecodeContainerConfig(src) + c, hc, nc, err := decodeContainerConfig(src) + if err != nil { + return nil, nil, nil, err + } + return c, hc, nc, nil } // DecodeHostConfig makes ContainerDecoder to implement httputils.ContainerDecoder func (r ContainerDecoder) DecodeHostConfig(src io.Reader) (*container.HostConfig, error) { - return DecodeHostConfig(src) + hc, err := decodeHostConfig(src) + if err != nil { + return nil, err + } + return hc, nil } -// DecodeContainerConfig decodes a json encoded config into a ContainerConfigWrapper +// decodeContainerConfig decodes a json encoded config into a ContainerConfigWrapper // struct and returns both a Config and a HostConfig struct // Be aware this function is not checking whether the resulted structs are nil, // it's your business to do so -func DecodeContainerConfig(src io.Reader) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig, error) { +func decodeContainerConfig(src io.Reader) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig, error) { var w ContainerConfigWrapper decoder := json.NewDecoder(src) @@ -95,12 +103,12 @@ func validateMountSettings(c *container.Config, hc *container.HostConfig) error // Ensure all volumes and binds are valid. for spec := range c.Volumes { if _, err := volume.ParseMountRaw(spec, hc.VolumeDriver); err != nil { - return fmt.Errorf("invalid volume spec %q: %v", spec, err) + return errors.Wrapf(err, "invalid volume spec %q", spec) } } for _, spec := range hc.Binds { if _, err := volume.ParseMountRaw(spec, hc.VolumeDriver); err != nil { - return fmt.Errorf("invalid bind mount spec %q: %v", spec, err) + return errors.Wrapf(err, "invalid bind mount spec %q", spec) } } diff --git a/runconfig/config_test.go b/runconfig/config_test.go index 83ec363a0b..231b4509bc 100644 --- a/runconfig/config_test.go +++ b/runconfig/config_test.go @@ -51,7 +51,7 @@ func TestDecodeContainerConfig(t *testing.T) { t.Fatal(err) } - c, h, _, err := DecodeContainerConfig(bytes.NewReader(b)) + c, h, _, err := decodeContainerConfig(bytes.NewReader(b)) if err != nil { t.Fatal(fmt.Errorf("Error parsing %s: %v", f, err)) } @@ -135,5 +135,5 @@ func callDecodeContainerConfigIsolation(isolation string) (*container.Config, *c if b, err = json.Marshal(w); err != nil { return nil, nil, nil, fmt.Errorf("Error on marshal %s", err.Error()) } - return DecodeContainerConfig(bytes.NewReader(b)) + return decodeContainerConfig(bytes.NewReader(b)) } diff --git a/runconfig/errors.go b/runconfig/errors.go index c95a2919e8..78b0a8817c 100644 --- a/runconfig/errors.go +++ b/runconfig/errors.go @@ -1,38 +1,42 @@ package runconfig -import ( - "fmt" +const ( + // ErrConflictContainerNetworkAndLinks conflict between --net=container and links + ErrConflictContainerNetworkAndLinks validationError = "conflicting options: container type network can't be used with links. This would result in undefined behavior" + // ErrConflictSharedNetwork conflict between private and other networks + ErrConflictSharedNetwork validationError = "container sharing network namespace with another container or host cannot be connected to any other network" + // ErrConflictHostNetwork conflict from being disconnected from host network or connected to host network. + ErrConflictHostNetwork validationError = "container cannot be disconnected from host network or connected to host network" + // ErrConflictNoNetwork conflict between private and other networks + ErrConflictNoNetwork validationError = "container cannot be connected to multiple networks with one of the networks in private (none) mode" + // ErrConflictNetworkAndDNS conflict between --dns and the network mode + ErrConflictNetworkAndDNS validationError = "conflicting options: dns and the network mode" + // ErrConflictNetworkHostname conflict between the hostname and the network mode + ErrConflictNetworkHostname validationError = "conflicting options: hostname and the network mode" + // ErrConflictHostNetworkAndLinks conflict between --net=host and links + ErrConflictHostNetworkAndLinks validationError = "conflicting options: host type networking can't be used with links. This would result in undefined behavior" + // ErrConflictContainerNetworkAndMac conflict between the mac address and the network mode + ErrConflictContainerNetworkAndMac validationError = "conflicting options: mac-address and the network mode" + // ErrConflictNetworkHosts conflict between add-host and the network mode + ErrConflictNetworkHosts validationError = "conflicting options: custom host-to-IP mapping and the network mode" + // ErrConflictNetworkPublishPorts conflict between the publish options and the network mode + ErrConflictNetworkPublishPorts validationError = "conflicting options: port publishing and the container type network mode" + // ErrConflictNetworkExposePorts conflict between the expose option and the network mode + ErrConflictNetworkExposePorts validationError = "conflicting options: port exposing and the container type network mode" + // ErrUnsupportedNetworkAndIP conflict between network mode and requested ip address + ErrUnsupportedNetworkAndIP validationError = "user specified IP address is supported on user defined networks only" + // ErrUnsupportedNetworkNoSubnetAndIP conflict between network with no configured subnet and requested ip address + ErrUnsupportedNetworkNoSubnetAndIP validationError = "user specified IP address is supported only when connecting to networks with user configured subnets" + // ErrUnsupportedNetworkAndAlias conflict between network mode and alias + ErrUnsupportedNetworkAndAlias validationError = "network-scoped alias is supported only for containers in user defined networks" + // ErrConflictUTSHostname conflict between the hostname and the UTS mode + ErrConflictUTSHostname validationError = "conflicting options: hostname and the UTS mode" ) -var ( - // ErrConflictContainerNetworkAndLinks conflict between --net=container and links - ErrConflictContainerNetworkAndLinks = fmt.Errorf("conflicting options: container type network can't be used with links. This would result in undefined behavior") - // ErrConflictSharedNetwork conflict between private and other networks - ErrConflictSharedNetwork = fmt.Errorf("container sharing network namespace with another container or host cannot be connected to any other network") - // ErrConflictHostNetwork conflict from being disconnected from host network or connected to host network. - ErrConflictHostNetwork = fmt.Errorf("container cannot be disconnected from host network or connected to host network") - // ErrConflictNoNetwork conflict between private and other networks - ErrConflictNoNetwork = fmt.Errorf("container cannot be connected to multiple networks with one of the networks in private (none) mode") - // ErrConflictNetworkAndDNS conflict between --dns and the network mode - ErrConflictNetworkAndDNS = fmt.Errorf("conflicting options: dns and the network mode") - // ErrConflictNetworkHostname conflict between the hostname and the network mode - ErrConflictNetworkHostname = fmt.Errorf("conflicting options: hostname and the network mode") - // ErrConflictHostNetworkAndLinks conflict between --net=host and links - ErrConflictHostNetworkAndLinks = fmt.Errorf("conflicting options: host type networking can't be used with links. This would result in undefined behavior") - // ErrConflictContainerNetworkAndMac conflict between the mac address and the network mode - ErrConflictContainerNetworkAndMac = fmt.Errorf("conflicting options: mac-address and the network mode") - // ErrConflictNetworkHosts conflict between add-host and the network mode - ErrConflictNetworkHosts = fmt.Errorf("conflicting options: custom host-to-IP mapping and the network mode") - // ErrConflictNetworkPublishPorts conflict between the publish options and the network mode - ErrConflictNetworkPublishPorts = fmt.Errorf("conflicting options: port publishing and the container type network mode") - // ErrConflictNetworkExposePorts conflict between the expose option and the network mode - ErrConflictNetworkExposePorts = fmt.Errorf("conflicting options: port exposing and the container type network mode") - // ErrUnsupportedNetworkAndIP conflict between network mode and requested ip address - ErrUnsupportedNetworkAndIP = fmt.Errorf("user specified IP address is supported on user defined networks only") - // ErrUnsupportedNetworkNoSubnetAndIP conflict between network with no configured subnet and requested ip address - ErrUnsupportedNetworkNoSubnetAndIP = fmt.Errorf("user specified IP address is supported only when connecting to networks with user configured subnets") - // ErrUnsupportedNetworkAndAlias conflict between network mode and alias - ErrUnsupportedNetworkAndAlias = fmt.Errorf("network-scoped alias is supported only for containers in user defined networks") - // ErrConflictUTSHostname conflict between the hostname and the UTS mode - ErrConflictUTSHostname = fmt.Errorf("conflicting options: hostname and the UTS mode") -) +type validationError string + +func (e validationError) Error() string { + return string(e) +} + +func (e validationError) InvalidParameter() {} diff --git a/runconfig/hostconfig.go b/runconfig/hostconfig.go index 24aed1935e..3a90c84498 100644 --- a/runconfig/hostconfig.go +++ b/runconfig/hostconfig.go @@ -2,7 +2,6 @@ package runconfig import ( "encoding/json" - "fmt" "io" "strings" @@ -11,7 +10,7 @@ import ( // DecodeHostConfig creates a HostConfig based on the specified Reader. // It assumes the content of the reader will be JSON, and decodes it. -func DecodeHostConfig(src io.Reader) (*container.HostConfig, error) { +func decodeHostConfig(src io.Reader) (*container.HostConfig, error) { decoder := json.NewDecoder(src) var w ContainerConfigWrapper @@ -45,7 +44,7 @@ func validateNetContainerMode(c *container.Config, hc *container.HostConfig) err parts := strings.Split(string(hc.NetworkMode), ":") if parts[0] == "container" { if len(parts) < 2 || parts[1] == "" { - return fmt.Errorf("Invalid network mode: invalid container format container:") + return validationError("Invalid network mode: invalid container format container:") } } diff --git a/runconfig/hostconfig_test.go b/runconfig/hostconfig_test.go index ec9846ab16..b461c16f0c 100644 --- a/runconfig/hostconfig_test.go +++ b/runconfig/hostconfig_test.go @@ -190,7 +190,7 @@ func TestDecodeHostConfig(t *testing.T) { t.Fatal(err) } - c, err := DecodeHostConfig(bytes.NewReader(b)) + c, err := decodeHostConfig(bytes.NewReader(b)) if err != nil { t.Fatal(fmt.Errorf("Error parsing %s: %v", f, err)) } diff --git a/volume/drivers/extpoint.go b/volume/drivers/extpoint.go index da230dcc76..9c65d899ea 100644 --- a/volume/drivers/extpoint.go +++ b/volume/drivers/extpoint.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/pkg/locker" getter "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/volume" + "github.com/pkg/errors" ) // currently created by hand. generation tool would generate this like: @@ -99,6 +100,14 @@ func Unregister(name string) bool { return true } +type driverNotFoundError string + +func (e driverNotFoundError) Error() string { + return "volume driver not found: " + string(e) +} + +func (driverNotFoundError) NotFound() {} + // lookup returns the driver associated with the given name. If a // driver with the given name has not been registered it checks if // there is a VolumeDriver plugin available with the given name. @@ -115,7 +124,7 @@ func lookup(name string, mode int) (volume.Driver, error) { if drivers.plugingetter != nil { p, err := drivers.plugingetter.Get(name, extName, mode) if err != nil { - return nil, fmt.Errorf("Error looking up volume plugin %s: %v", name, err) + return nil, errors.Wrap(err, "error looking up volume plugin "+name) } d := NewVolumeDriver(p.Name(), p.BasePath(), p.Client()) @@ -130,7 +139,7 @@ func lookup(name string, mode int) (volume.Driver, error) { } return d, nil } - return nil, fmt.Errorf("Error looking up volume plugin %s", name) + return nil, driverNotFoundError(name) } func validateDriver(vd volume.Driver) error { diff --git a/volume/local/local.go b/volume/local/local.go index 329febb4d7..eb78d875a5 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -13,12 +13,11 @@ import ( "strings" "sync" - "github.com/pkg/errors" - "github.com/docker/docker/api" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/volume" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -39,14 +38,6 @@ var ( volumeNameRegex = api.RestrictedNamePattern ) -type validationError struct { - error -} - -func (validationError) IsValidationError() bool { - return true -} - type activeMount struct { count uint64 mounted bool @@ -148,6 +139,30 @@ func (r *Root) Name() string { return volume.DefaultDriverName } +type alreadyExistsError struct { + path string +} + +func (e alreadyExistsError) Error() string { + return "local volume already exists under " + e.path +} + +func (e alreadyExistsError) Conflict() {} + +type systemError struct { + err error +} + +func (e systemError) Error() string { + return e.err.Error() +} + +func (e systemError) SystemError() {} + +func (e systemError) Cause() error { + return e.err +} + // Create creates a new volume.Volume with the provided name, creating // the underlying directory tree required for this volume in the // process. @@ -167,9 +182,9 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error path := r.DataPath(name) if err := idtools.MkdirAllAndChown(path, 0755, r.rootIDs); err != nil { if os.IsExist(err) { - return nil, fmt.Errorf("volume already exists under %s", filepath.Dir(path)) + return nil, alreadyExistsError{filepath.Dir(path)} } - return nil, errors.Wrapf(err, "error while creating volume path '%s'", path) + return nil, errors.Wrapf(systemError{err}, "error while creating volume path '%s'", path) } var err error @@ -195,7 +210,7 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error return nil, err } if err = ioutil.WriteFile(filepath.Join(filepath.Dir(path), "opts.json"), b, 600); err != nil { - return nil, errors.Wrap(err, "error while persisting volume options") + return nil, errors.Wrap(systemError{err}, "error while persisting volume options") } } @@ -213,11 +228,11 @@ func (r *Root) Remove(v volume.Volume) error { lv, ok := v.(*localVolume) if !ok { - return fmt.Errorf("unknown volume type %T", v) + return systemError{errors.Errorf("unknown volume type %T", v)} } if lv.active.count > 0 { - return fmt.Errorf("volume has active mounts") + return systemError{errors.Errorf("volume has active mounts")} } if err := lv.unmount(); err != nil { @@ -233,7 +248,7 @@ func (r *Root) Remove(v volume.Volume) error { } if !r.scopedPath(realPath) { - return fmt.Errorf("Unable to remove a directory of out the Docker root %s: %s", r.scope, realPath) + return systemError{errors.Errorf("Unable to remove a directory of out the Docker root %s: %s", r.scope, realPath)} } if err := removePath(realPath); err != nil { @@ -249,7 +264,7 @@ func removePath(path string) error { if os.IsNotExist(err) { return nil } - return errors.Wrapf(err, "error removing volume path '%s'", path) + return errors.Wrapf(systemError{err}, "error removing volume path '%s'", path) } return nil } @@ -270,12 +285,20 @@ func (r *Root) Scope() string { return volume.LocalScope } +type validationError string + +func (e validationError) Error() string { + return string(e) +} + +func (e validationError) InvalidParameter() {} + func (r *Root) validateName(name string) error { if len(name) == 1 { - return validationError{fmt.Errorf("volume name is too short, names should be at least two alphanumeric characters")} + return validationError("volume name is too short, names should be at least two alphanumeric characters") } if !volumeNameRegex.MatchString(name) { - return validationError{fmt.Errorf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, api.RestrictedNameChars)} + return validationError(fmt.Sprintf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, api.RestrictedNameChars)) } return nil } @@ -319,7 +342,7 @@ func (v *localVolume) Mount(id string) (string, error) { if v.opts != nil { if !v.active.mounted { if err := v.mount(); err != nil { - return "", err + return "", systemError{err} } v.active.mounted = true } @@ -353,7 +376,7 @@ func (v *localVolume) unmount() error { if v.opts != nil { if err := mount.Unmount(v.path); err != nil { if mounted, mErr := mount.Mounted(v.path); mounted || mErr != nil { - return errors.Wrapf(err, "error while unmounting volume path '%s'", v.path) + return errors.Wrapf(systemError{err}, "error while unmounting volume path '%s'", v.path) } } v.active.mounted = false @@ -364,7 +387,7 @@ func (v *localVolume) unmount() error { func validateOpts(opts map[string]string) error { for opt := range opts { if !validOpts[opt] { - return validationError{fmt.Errorf("invalid option key: %q", opt)} + return validationError(fmt.Sprintf("invalid option key: %q", opt)) } } return nil diff --git a/volume/store/errors.go b/volume/store/errors.go index 980175f29c..13c7765070 100644 --- a/volume/store/errors.go +++ b/volume/store/errors.go @@ -2,21 +2,41 @@ package store import ( "strings" - - "github.com/pkg/errors" ) -var ( +const ( // errVolumeInUse is a typed error returned when trying to remove a volume that is currently in use by a container - errVolumeInUse = errors.New("volume is in use") + errVolumeInUse conflictError = "volume is in use" // errNoSuchVolume is a typed error returned if the requested volume doesn't exist in the volume store - errNoSuchVolume = errors.New("no such volume") + errNoSuchVolume notFoundError = "no such volume" // errInvalidName is a typed error returned when creating a volume with a name that is not valid on the platform - errInvalidName = errors.New("volume name is not valid on this platform") + errInvalidName invalidName = "volume name is not valid on this platform" // errNameConflict is a typed error returned on create when a volume exists with the given name, but for a different driver - errNameConflict = errors.New("volume name must be unique") + errNameConflict conflictError = "volume name must be unique" ) +type conflictError string + +func (e conflictError) Error() string { + return string(e) +} +func (conflictError) Conflict() {} + +type notFoundError string + +func (e notFoundError) Error() string { + return string(e) +} + +func (notFoundError) NotFound() {} + +type invalidName string + +func (e invalidName) Error() string { + return string(e) +} +func (invalidName) InvalidParameter() {} + // OpErr is the error type returned by functions in the store package. It describes // the operation, volume name, and error. type OpErr struct { @@ -47,6 +67,11 @@ func (e *OpErr) Error() string { return s } +// Cause returns the error the caused this error +func (e *OpErr) Cause() error { + return e.Err +} + // IsInUse returns a boolean indicating whether the error indicates that a // volume is in use func IsInUse(err error) bool { @@ -64,13 +89,16 @@ func IsNameConflict(err error) bool { return isErr(err, errNameConflict) } +type causal interface { + Cause() error +} + func isErr(err error, expected error) bool { - err = errors.Cause(err) switch pe := err.(type) { case nil: return false - case *OpErr: - err = errors.Cause(pe.Err) + case causal: + return isErr(pe.Cause(), expected) } return err == expected } diff --git a/volume/validate.go b/volume/validate.go index 5de46198f6..04883f35c5 100644 --- a/volume/validate.go +++ b/volume/validate.go @@ -1,12 +1,12 @@ package volume import ( - "errors" "fmt" "os" "runtime" "github.com/docker/docker/api/types/mount" + "github.com/pkg/errors" ) var errBindNotExist = errors.New("bind source path does not exist") @@ -135,10 +135,10 @@ func (e *errMountConfig) Error() string { } func errExtraField(name string) error { - return fmt.Errorf("field %s must not be specified", name) + return errors.Errorf("field %s must not be specified", name) } func errMissingField(name string) error { - return fmt.Errorf("field %s must not be empty", name) + return errors.Errorf("field %s must not be empty", name) } func validateAbsolute(p string) error { @@ -146,7 +146,7 @@ func validateAbsolute(p string) error { if isAbsPath(p) { return nil } - return fmt.Errorf("invalid mount path: '%s' mount path must be absolute", p) + return errors.Errorf("invalid mount path: '%s' mount path must be absolute", p) } // ValidateTmpfsMountDestination validates the destination of tmpfs mount. @@ -160,3 +160,17 @@ func ValidateTmpfsMountDestination(dest string) error { } return validateAbsolute(dest) } + +type validationError struct { + err error +} + +func (e validationError) Error() string { + return e.err.Error() +} + +func (e validationError) InvalidParameter() {} + +func (e validationError) Cause() error { + return e.err +} diff --git a/volume/volume.go b/volume/volume.go index 7e8d16cc68..7f962a981e 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -310,7 +310,7 @@ func ParseMountRaw(raw, volumeDriver string) (*MountPoint, error) { mp.Mode = mode } if err != nil { - err = fmt.Errorf("%v: %v", errInvalidSpec(raw), err) + err = errors.Wrap(err, errInvalidSpec(raw).Error()) } return mp, err } @@ -318,7 +318,7 @@ func ParseMountRaw(raw, volumeDriver string) (*MountPoint, error) { // ParseMountSpec reads a mount config, validates it, and configures a mountpoint from it. func ParseMountSpec(cfg mounttypes.Mount, options ...func(*validateOpts)) (*MountPoint, error) { if err := validateMountConfig(&cfg, options...); err != nil { - return nil, err + return nil, validationError{err} } mp := &MountPoint{ RW: !cfg.ReadOnly, @@ -360,9 +360,9 @@ func ParseMountSpec(cfg mounttypes.Mount, options ...func(*validateOpts)) (*Moun } func errInvalidMode(mode string) error { - return fmt.Errorf("invalid mode: %v", mode) + return validationError{errors.Errorf("invalid mode: %v", mode)} } func errInvalidSpec(spec string) error { - return fmt.Errorf("invalid volume specification: '%s'", spec) + return validationError{errors.Errorf("invalid volume specification: '%s'", spec)} }