Proposal: Roadmap to replace samalba/dockerclient with engine-api/client

[nishanttotla]

Problem

Swarm has been using samalba/dockerclient to send requests to Engines in the cluster so far. This has been a limiting factor for supporting new features of the Engine API, since dockerclient is outdated and does not fully support the current Engine API.

Solution

engine-api/client contains the officially supported Docker API, along with the relevant type structs and client. We need to be using that instead.

Challenge

All Swarm changes currently go through dockerclient, all of which need to be moved over to use engine-api, along with new using new type structs defined in engine-api/types and updating unit and integration tests. This is a large change that will take time to fully accomplish and test for correctness.

Proposal

The simplest way to start is to add a new engine-api/client object in the Engine struct in Swarm, which uses the same underlying HTTP connection that is used by dockerclient. This means that the two clients will co-exist in Swarm while using the same underlying connection.

Once this is done, we should start moving over individual API endpoints to use the new client. We must make sure that this does not affect or cause any issues to existing Swarm users, except for the addition of new features that engine-api allows us to implement.

Roadmap

The proposal will be implemented in the following steps:

• Create an engine-api client in Swarm, and use it to replace the Version() function with ServerVersion(). This will involve adding/updating Godeps, creating mock and nop clients, and updating tests. This part is handled in Introducing engine-api client into Swarm #1880.
• Replace the Info() function to use its equivalent in engine-api. This is another small change, with several more changes likely needed, because of type mismatches between the Info structs in the two client libraries. For simplicity, we should perform type conversions (mostly between int and int64 as needed) - this is a BAD idea in general, but only a temporary solution until some of the next few changes take effect. This part is handled in Updating info function to use engine-api #1938.
• Change the ContainerConfig struct in the cluster package to use ContainerConfig from engine-api/types. This should mostly take care of the messy type conversions required in the previous step. We will also need to change all functions that create/modify ContainerConfig. This change will span across a large part of the codebase, and move over complete container management to engine-api. This part is handled in Updating ContainerConfig to use engine-api #1983, Moving start container to engine-api #2127, Move container rename and remove to engine-api #2128, use engine-api to replace dockerclient #2194.
• Starting in file handlers.go, focus on individual API endpoints to move them to the new client. The endpoints can be roughly grouped into the following categories
  • Network modification operations (use apiClient for various things #1982, use engine-api for network operations #2068)
  • Image modification operations (use apiClient for various things #1982, Moving Image handling to engine-api #2056, Moving last few image operations to engine-api #2132)
  • Volume modification operations (use apiClient for various things #1982)
  • Events handling (use engine-api for events #2086)
  • Drop support for HostConfig on /start (Stop allowing the passing of HostConfig on Container Start #2756)
• The final step will be to create the HTTP client independent of dockerclient (as opposed to what we do right now). This might need some additional pipelining since Swarm needs some extra features, such as timeout. (Creating http client in swarm #2206, Use http client created in Swarm #2757)

Important considerations

1. Is there a way to ensure that users can roll back to use the old client if they want, especially if things are failing? Unfortunately, it's not as simple as abstracting the client object and then choosing which client to use based on the API endpoint, because the arguments and return types are all different between dockerclient and engine-api (Thanks to @dongluochen for bringing this up)
2. Backward compatibility will break, because engine-api and 'dockerclient` have different types, and this affects the JSON returned by the Swarm API. (Thanks to @amitshukla for bringing this up)

[nishanttotla]

cc @docker/core-swarm-maintainers

[MHBauer]

same as #1560?

[nishanttotla]

@MHBauer thanks, I'd missed that issue. We can start a fresh discussion here, and close that issue, or continue there if others prefer that.

Looking over the comments there, I can see that engine-api has been updated significantly and added a mock client, but not a nop client, though we can push for it if it's necessary.

[MHBauer]

I think this effort is a great testbed for how we address the needs of people outside the docker cli itself. There isn't a nopClient, but I think it should be able to be rigged up internally for now, giving it a mock type client. Definitely something we can get into engine-api if it's a pain to configure.

[nishanttotla]

Here are some good comments regarding the nopclient, from an issue I opened in engine-api: docker/engine-api#117

For now, I'm going to create our own nopclient, but we could consider moving away from it in the future.

[MHBauer]

What I see left. I think it would be 'okay' to leave the tests, if only to have some diversity.

λ ag samalba/dockerclient --ignore vendor --ignore Godeps
api/handlers.go
27: "github.com/samalba/dockerclient"

cluster/cluster.go
7:  "github.com/samalba/dockerclient"

cluster/engine.go
28: "github.com/samalba/dockerclient"
29: "github.com/samalba/dockerclient/nopclient"

cluster/engine_test.go
20: "github.com/samalba/dockerclient"
21: "github.com/samalba/dockerclient/mockclient"
22: "github.com/samalba/dockerclient/nopclient"

cluster/mesos/cluster.go
27: "github.com/samalba/dockerclient"

cluster/swarm/cluster_test.go
15: "github.com/samalba/dockerclient/mockclient"

cluster/swarm/cluster.go
25: "github.com/samalba/dockerclient"

[nishanttotla]

@MHBauer thanks. Based on the checklist above, all of the items you listed should fall under the following two categories

• Passing HostConfig on /start which needs dockerclient
• Creating the http client independent of dockerclient, which I'm currently working on, and which should allow us to get rid of most of those things

There may still be minor things left, but the major part of the work should be done.