Moving all strings to the errors package wasn't a good idea after all.
Our custom implementation of Go errors predates everything that's nice
and good about working with errors in Go. Take as an example what we
have to do to get an error message:
```go
func GetErrorMessage(err error) string {
switch err.(type) {
case errcode.Error:
e, _ := err.(errcode.Error)
return e.Message
case errcode.ErrorCode:
ec, _ := err.(errcode.ErrorCode)
return ec.Message()
default:
return err.Error()
}
}
```
This goes against every good practice for Go development. The language already provides a simple, intuitive and standard way to get error messages, that is calling the `Error()` method from an error. Reinventing the error interface is a mistake.
Our custom implementation also makes very hard to reason about errors, another nice thing about Go. I found several (>10) error declarations that we don't use anywhere. This is a clear sign about how little we know about the errors we return. I also found several error usages where the number of arguments was different than the parameters declared in the error, another clear example of how difficult is to reason about errors.
Moreover, our custom implementation didn't really make easier for people to return custom HTTP status code depending on the errors. Again, it's hard to reason about when to set custom codes and how. Take an example what we have to do to extract the message and status code from an error before returning a response from the API:
```go
switch err.(type) {
case errcode.ErrorCode:
daError, _ := err.(errcode.ErrorCode)
statusCode = daError.Descriptor().HTTPStatusCode
errMsg = daError.Message()
case errcode.Error:
// For reference, if you're looking for a particular error
// then you can do something like :
// import ( derr "github.com/docker/docker/errors" )
// if daError.ErrorCode() == derr.ErrorCodeNoSuchContainer { ... }
daError, _ := err.(errcode.Error)
statusCode = daError.ErrorCode().Descriptor().HTTPStatusCode
errMsg = daError.Message
default:
// This part of will be removed once we've
// converted everything over to use the errcode package
// FIXME: this is brittle and should not be necessary.
// If we need to differentiate between different possible error types,
// we should create appropriate error types with clearly defined meaning
errStr := strings.ToLower(err.Error())
for keyword, status := range map[string]int{
"not found": http.StatusNotFound,
"no such": http.StatusNotFound,
"bad parameter": http.StatusBadRequest,
"conflict": http.StatusConflict,
"impossible": http.StatusNotAcceptable,
"wrong login/password": http.StatusUnauthorized,
"hasn't been activated": http.StatusForbidden,
} {
if strings.Contains(errStr, keyword) {
statusCode = status
break
}
}
}
```
You can notice two things in that code:
1. We have to explain how errors work, because our implementation goes against how easy to use Go errors are.
2. At no moment we arrived to remove that `switch` statement that was the original reason to use our custom implementation.
This change removes all our status errors from the errors package and puts them back in their specific contexts.
IT puts the messages back with their contexts. That way, we know right away when errors used and how to generate their messages.
It uses custom interfaces to reason about errors. Errors that need to response with a custom status code MUST implementent this simple interface:
```go
type errorWithStatus interface {
HTTPErrorStatusCode() int
}
```
This interface is very straightforward to implement. It also preserves Go errors real behavior, getting the message is as simple as using the `Error()` method.
I included helper functions to generate errors that use custom status code in `errors/errors.go`.
By doing this, we remove the hard dependency we have eeverywhere to our custom errors package. Yes, you can use it as a helper to generate error, but it's still very easy to generate errors without it.
Please, read this fantastic blog post about errors in Go: http://dave.cheney.net/2014/12/24/inspecting-errors
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: a793564b2591035aec5412fbcbcccf220c773a4c
Component: engine
Removing direct dependencies from the server configuration.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: 1ba44a832f6aae811dfc6235287dd5b99e8aa94c
Component: engine
Add `--restart` flag for `update` command, so we can change restart
policy for a container no matter it's running or stopped.
Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
Upstream-commit: ff3ea4c90f2ede5cccc6b49c4d2aad7201c91a4c
Component: engine
It should be explicitly told whether to enable the profiler or not.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: e8f569b3246b3ce4e765b0aafe53b6d70d12a2d6
Component: engine
Ideally I would love to just remove this check entirely because its
seems pretty useless. An old client talking to a new server isn't
an error condition, nor is it something to even worry about - its a normal
part of life. Flooding my screen (and logs) with a warning that isn't
something I (as an admin) need to be concerned about is silly and a
distraction when I need to look for real issues. If anything this should
be printed on the cli not the daemon since its the cli that needs to be
concerned, not the daemon.
However, since when you debug an issue it might be interesting to know the
client is old I decided to pull back a little and just change it from
a Warning to a Debug logrus call instead.
If others want it removed I still do that though :-)
Signed-off-by: Doug Davis <dug@us.ibm.com>
Upstream-commit: 059ad5d0a975ab4970fe0be45a79ffa0ef35e366
Component: engine
In Docker 1.10 and earlier, "docker build" can do a build FROM a private
repository that hasn't yet been pulled. This doesn't work on master. I
bisected this to https://github.com/docker/docker/pull/19414.
AuthConfigs is deserialized from the HTTP request, but not included in
the builder options.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Upstream-commit: 6fed46aeb97943315aed12f2dc62565f7bcc53dc
Component: engine
- Remove duplicated structs that we already have in engine-api.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: 06d8f504f7b1883f490b5deda5a30ef9acd99f95
Component: engine
This is done by moving the following types to api/types/config.go:
- ContainersConfig
- ContainerAttachWithLogsConfig
- ContainerWsAttachWithLogsConfig
- ContainerLogsConfig
- ContainerStatsConfig
Remove dependency on "version" package from types.ContainerStatsConfig.
Decouple the "container" router from the "daemon/exec" implementation.
* This is done by making daemon.ContainerExecInspect() return an interface{}
value. The same trick is already used by daemon.ContainerInspect().
Improve documentation for router packages.
Extract localRoute and router into separate files.
Move local.router to image.imageRouter.
Changes:
- Move local/image.go to image/image_routes.go.
- Move local/local.go to image/image.go
- Rename router to imageRouter.
- Simplify imports for image/image.go (remove alias for router package).
Merge router/local package into router package.
Decouple the "image" router from the actual daemon implementation.
Add Daemon.GetNetworkByID and Daemon.GetNetworkByName.
Decouple the "network" router from the actual daemon implementation.
This is done by replacing the daemon.NetworkByName constant with
an explicit GetNetworkByName method.
Remove the unused Daemon.GetNetwork method and the associated constants NetworkByID and NetworkByName.
Signed-off-by: Lukas Waslowski <cr7pt0gr4ph7@gmail.com>
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: dd93571c69cc5284f695a21d5504fb57b1a4891a
Component: engine
This adds an npipe protocol option for Windows hosts, akin to unix
sockets for Linux hosts. This should become the default transport
for Windows, but this change does not yet do that.
It also does not add support for the client side yet since that
code is in engine-api, which will have to be revendored separately.
Signed-off-by: John Starks <jostarks@microsoft.com>
Upstream-commit: 0906195fbbd6f379c163b80f23e4c5a60bcfc5f0
Component: engine
Currently, daemonbuilder package (part of daemon) implemented the
builder backend. However, it was a very thin wrapper around daemon
methods and caused an implementation dependency for api/server build
endpoint. api/server buildrouter should only know about the backend
implementing the /build API endpoint.
Removing daemonbuilder involved moving build specific methods to
respective files in the daemon, where they fit naturally.
Signed-off-by: Anusha Ragunathan <anusha@docker.com>
Upstream-commit: 9c332b164f1aefa2407706adf59d50495d6e02cb
Component: engine
Warning should be printed with real `logrus.Warn`
Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
Upstream-commit: 98f74f8383241d2f0a5ed8b17f7016e24af899fa
Component: engine
This is happening now due to improvements in net/http:
99fb19194c
To test, change the go version in the Dockerfile:
-ENV GO_VERSION 1.5.3
+ENV GO_VERSION 1.6beta2
More info here: https://github.com/golang/go/issues/14001
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com>
Upstream-commit: 2df5dafdaf12296fa2bd92ced62ba80da41af744
Component: engine
Currently builder.Backend is implemented by daemonbuilder.Docker{} for
the daemon. This registration happens in the API/server code. However,
this is too implementation specific. Ideally we should be able to specify
that docker daemon (or any other) is implementing the Backend and abstract
the implementation details. So we should remove package daemonbuilder
dependency in build_routes.go
With this change, daemonbuilder.Docker is nothing more than the daemon.
A follow on change will remove the daemonbuilder package and move relevant
methods under daemon, so that API only knows about the backend.
Also cleanup code in api/client/build.go. docker cli always performs build
context tar download for remoteURLs and sends an empty remoteContext. So
remove relevant dead code.
Signed-off-by: Anusha Ragunathan <anusha@docker.com>
Upstream-commit: 14215ed5a1900a88a3b17dd7cd566def50bfcbc9
Component: engine
* If user doesn't specify the subnets to create a network, it will pick
subnets from inside preferred pool. This PR aims to inspect these subnets info
* Add integration tests for docker inspect the subnets.
* docker-py project is already synchronized.
* jenkins checks depend on https://github.com/docker/docker-py/pull/888
Fixes issue #18626
Signed-off-by: Wen Cheng Ma <wenchma@cn.ibm.com>
Upstream-commit: 5cc672b0061f3df41073cb7b4ff962998a13a09c
Component: engine
Read configuration after flags making this the priority:
1- Apply configuration from file.
2- Apply configuration from flags.
Reload configuration when a signal is received, USR2 in Linux:
- Reload router if the debug configuration changes.
- Reload daemon labels.
- Reload cluster discovery.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: 677a6b3506107468ed8c00331991afd9176fa0b9
Component: engine
docker's network disconnect api now supports `Force` option which can be
used to force cleanup an endpoint from any host in the cluster.
Signed-off-by: Madhu Venugopal <madhu@docker.com>
Upstream-commit: b464f1d78cdfa2a4124e083b8f7b0f2353f12de3
Component: engine
Fixes an issue where a client can send a large body but specifiy
application/json as the content-type, and cause Docker to consume lots
of RAM while trying to buffer the body so it can be dumped to the debug
log.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 89af3835d483fe758f301aa8f3b1062a0ab712fb
Component: engine
This allows to define clearly what is mutable or not in a container
and remove the use of the internal HostConfig struct to be used.
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Upstream-commit: a4f6920731c6af27a7e89c3da8d0e6fd309de90a
Component: engine
