New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Docker Remote API Client #7358

Closed
mcculloughsean opened this Issue Jul 31, 2014 · 5 comments

Comments

Projects
None yet
6 participants
@mcculloughsean

mcculloughsean commented Jul 31, 2014

In addition to work in #5893 and related issues.

Refactoring Docker Remote API Client

What's wrong

The docker remote api isn't easy to use because:

  • There's no official client to the API
  • Much of the validation the CLI provides is in the CLI code, not the
    API layer
  • The documentation doesn't always show a proper mapping of CLI commands
    to API calls (e.g. docker run involves multiple steps)
  • The CLI code is cluttered with many concerns and makes understanding
    how to use the API properly by example difficult
  • Marshaling/Unmarshaling in Go should use the same structures as the
    internal code for easier code sharing (i.e. dont require consumers to
    implement a type Containers struct for unmarshalling the output of
    docker ps)

How to fix

Propose:

  • Creating an official API client for use in the CLI tool
  • Exposing proper validation from the API layer or in the API client
  • Simplifying the CLI code and breaking commands down into smaller files
    so they're easier to understand.
  • Leverage pre-existing types when (un)serializing calls from the API

Proposed implementation

See https://gist.github.com/mcculloughsean/5c1dd3cc674e6fd583f5#file-proposal-go for example code.

Creating an official client

Composes 'Call' with commands to create a fetcher per endpoint in the API with proper unmarshalling.

Reimplement CLI in terms of API client

Use existing types for marshaling/unmarshaling JSON data for use in Go
code.

Move formatting to methods for CLI stringification.

Create helper methods to generate valid query strings.

Push validations down to API as much as possible

Remove all checks from the CLI level that can easily live in the API
layer.

Create a common pattern for passing validation errors back up through
the API pipeline (maybe by switching unmarshaling strategy based on http
status code)

Ensure that validation failure messages are verbose. e.g.:

$curl -v -H "Content-Type: application/json" -d "{\"Hostname\":\"\",\"User\":\"\",\"Memory\":0,\"MemorySwap\":0,\"AttachStdin\":false,\"AttachStdout\":true,\"AttachStderr\":true,\"PortSpecs\":null,\"Tty\":false,\"OpenStdin\":false,\"StdinOnce\":false,\"Env\":null,\"Cmd\":[\"date\"],\"Image\":\"repository.snc1/candyland/echo-fedora\",\"Volumes\":{\"/tmp\":{}},\"WorkingDir\":\"\",\"DisableNetwork\":false,\"ExposedPorts\":{\"22/tcp\":{}}}" http://localhost:12345/containers/create?name=foo
* About to connect() to localhost port 12345 (#0)
*   Trying ::1... Connection refused
*   Trying 127.0.0.1... connected
* Connected to localhost (127.0.0.1) port 12345 (#0)
> POST /containers/create?name=foo HTTP/1.1
> User-Agent: curl/7.19.7 (x86_64-redhat-linux-gnu) libcurl/7.19.7 NSS/3.15.3 zlib/1.2.3 libidn/1.18 libssh2/1.4.2
> Host: localhost:12345
> Accept: */*
> Content-Type: application/json
> Content-Length: 340
>
< HTTP/1.1 201 Created
< Content-Type: application/json
< Date: Thu, 31 Jul 2014 21:54:33 GMT
< Content-Length: 90
<
{"Id":"2d3a1faaa6d83680ddb4164dc39755ef18fab37b4bf66e88f62bd00168547faa","Warnings":null}
* Connection #0 to host localhost left intact
* Closing connection #0

$curl -v -H "Content-Type: application/json" -d "{\"Hostname\":\"\",\"User\":\"\",\"Memory\":0,\"MemorySwap\":0,\"AttachStdin\":false,\"AttachStdout\":true,\"AttachStderr\":true,\"PortSpecs\":null,\"Tty\":false,\"OpenStdin\":false,\"StdinOnce\":false,\"Env\":null,\"Cmd\":[\"date\"],\"Image\":\"repository.snc1/candyland/echo-fedora\",\"Volumes\":{\"/tmp\":{}},\"WorkingDir\":\"\",\"DisableNetwork\":false,\"ExposedPorts\":{\"22/tcp\":{}}}" http://localhost:12345/containers/create?name=foo
* About to connect() to localhost port 12345 (#0)
*   Trying ::1... Connection refused
*   Trying 127.0.0.1... connected
* Connected to localhost (127.0.0.1) port 12345 (#0)
> POST /containers/create?name=foo HTTP/1.1
> User-Agent: curl/7.19.7 (x86_64-redhat-linux-gnu) libcurl/7.19.7 NSS/3.15.3 zlib/1.2.3 libidn/1.18 libssh2/1.4.2
> Host: localhost:12345
> Accept: */*
> Content-Type: application/json
> Content-Length: 340
>
< HTTP/1.1 500 Internal Server Error
< Content-Type: text/plain; charset=utf-8
< Date: Thu, 31 Jul 2014 21:54:35 GMT
< Content-Length: 53
<
Abort due to constraint violation: constraint failed

The second error message isn't useful. The dockerd logs themselves
aren't useful either:

[debug] server.go:999 Calling POST /containers/create
2014/07/31 21:54:33 POST /containers/create?name=foo
[ead17ee3] +job create(foo)
[error] mount.go:11 [warning]: couldn't run auplink before unmount:
exec: "auplink": executable file not found in $PATH
[ead17ee3] -job create(foo) = OK (0)
[debug] server.go:999 Calling POST /containers/create
2014/07/31 21:54:35 POST /containers/create?name=foo
[ead17ee3] +job create(foo)
Abort due to constraint violation: constraint failed
[ead17ee3] -job create(foo) = ERR (1)
[error] server.go:1025 Error: Abort due to constraint violation:
constraint failed
[error] server.go:90 HTTP Error: statusCode=500 Abort due to constraint
violation: constraint failed
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 31, 2014

Member

👍

Member

thaJeztah commented Jul 31, 2014

👍

@fsouza

This comment has been minimized.

Show comment
Hide comment
@fsouza

fsouza Aug 1, 2014

Contributor

Cool! I'd love to delete fsouza/go-dockerclient in favor of an official client! :)

Contributor

fsouza commented Aug 1, 2014

Cool! I'd love to delete fsouza/go-dockerclient in favor of an official client! :)

@bfirsh

This comment has been minimized.

Show comment
Hide comment
@bfirsh

bfirsh Aug 5, 2014

Contributor

👍 Would be great to dogfood a Go client inside Docker.

Contributor

bfirsh commented Aug 5, 2014

👍 Would be great to dogfood a Go client inside Docker.

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Feb 25, 2015

Contributor

can we combine these two issues somehow? #5893 and this? or @mcculloughsean and @saden1 are you both set on different things. But yes I think we all want this :D

Contributor

jessfraz commented Feb 25, 2015

can we combine these two issues somehow? #5893 and this? or @mcculloughsean and @saden1 are you both set on different things. But yes I think we all want this :D

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Mar 3, 2016

Contributor

We already do this, see https://github.com/docker/engine-api.

Closing as fixed.

Contributor

calavera commented Mar 3, 2016

We already do this, see https://github.com/docker/engine-api.

Closing as fixed.

@calavera calavera closed this Mar 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment