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
Most of them were found and fixed by codespell.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Upstream-commit: 2eee613326fb59fd168849618d14a9054a40f9f5
Component: engine
also fix wrong function comment
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
Upstream-commit: d266142230bd041c8299eef329cf79a17f8f7478
Component: engine
This feels like it's where it belongs and it makes it exported
again (which is needed for libcompose that was using it before 1.10).
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Upstream-commit: fc6122a947f9eb9fc2f54fb8ba3b9da4531a6b99
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
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
Merge was used by builder and daemon. With this commit, the builder
call has been inlined and the function moved to the daemon package,
which is the only other caller.
Signed-off-by: Anusha Ragunathan <anusha@docker.com>
Upstream-commit: eb4ae8e28aa0baf28d6cde1079a5f9c618d475b2
Component: engine
dockerfile.Config is almost redundant with ImageBuildOptions.
Unify the two so that the latter can be removed. This also
helps build's API endpoint code to be less dependent on package
dockerfile.
Signed-off-by: Anusha Ragunathan <anusha@docker.com>
Upstream-commit: 5190794f1d85d5406611eb69c270df62ac1cdc7f
Component: engine
The parse.go file is used almost exclusively in the client. The few small
functions that are used outside of the client could easily be copied out
when the client is extracted, allowing this runconfig/opts package to
move to the client.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Upstream-commit: 2b7ad47bd2649c3f164e8b57b31fae313045c8f4
Component: engine
Move NoBaseImageSpecifier to a common spot and then use it instead of
"scratch" in a couple of places.
Signed-off-by: Doug Davis <dug@us.ibm.com>
Upstream-commit: e6806223e81c916c9b24580b19207271f1a36965
Component: engine
By using a container.Config directly.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Upstream-commit: 6dba0b5d89bfa262757493679788b171f23b891c
Component: engine
- Make the API client library completely standalone.
- Move windows partition isolation detection to the client, so the
driver doesn't use external types.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: 7ac4232e70fe7cf7318333cd0890db7f95663079
Component: engine
This is a very docker concept that nobody elses need.
We only maintain it to keep the API backwards compatible.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: f9b857a200696b07b67e6a7f94ede32487f5649d
Component: engine
Right now, the quiet (-q, --quiet) flag ignores the output
generated from within the container.
However, it ought to be quiet in a way that all kind
of diagnostic output should be ignored, unless the build
process fails.
This patch makes the quiet flag behave in the following way:
1. If the build process succeeds, stdout contains the image ID
and stderr is empty.
2. If the build process fails, stdout is empty and stderr
has the error message and the diagnostic output of that process.
If the quiet flag is not set, then everything goes to stdout
and error messages, if there are any, go to stderr.
Signed-off-by: Boaz Shuster <ripcurld.github@gmail.com>
Upstream-commit: 60b4db7eb17f4eb509be4a4968364ada2075d60c
Component: engine
This commit adds a transfer manager which deduplicates and schedules
transfers, and also an upload manager and download manager that build on
top of the transfer manager to provide high-level interfaces for uploads
and downloads. The push and pull code is modified to use these building
blocks.
Some benefits of the changes:
- Simplification of push/pull code
- Pushes can upload layers concurrently
- Failed downloads and uploads are retried after backoff delays
- Cancellation is supported, but individual transfers will only be
cancelled if all pushes or pulls using them are cancelled.
- The distribution code is decoupled from Docker Engine packages and API
conventions (i.e. streamformatter), which will make it easier to split
out.
This commit also includes unit tests for the new distribution/xfer
package. The tests cover 87.8% of the statements in the package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Upstream-commit: 572ce802306a4e919802e5b77cbeca94acda7c0a
Component: engine
- Moved the following config structs to api/types
- ContainerRmConfig
- ContainerCommitConfig
Signed-off-by: Morgan Bauer <mbauer@us.ibm.com>
Upstream-commit: 63fb931a0b7298c6281898bcc5f53ab0655ad1a6
Component: engine
So other packages don't need to import the daemon package when they
want to use this struct.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
Upstream-commit: 6bb0d1816acd8d4f7a542a6aac047da2b874f476
Component: engine
Add distribution package for managing pulls and pushes. This is based on
the old code in the graph package, with major changes to work with the
new image/layer model.
Add v1 migration code.
Update registry, api/*, and daemon packages to use the reference
package's types where applicable.
Update daemon package to use image/layer/tag stores instead of the graph
package
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Upstream-commit: 4352da7803d182a6013a5238ce20a7c749db979a
Component: engine
