search

Transmode / gradle-docker

A Gradle plugin to build Docker images from the build script.
Apache License 2.0
647 stars 142 forks source link

Allow use of the Docker API as an alternative to the command line client #10

Closed sfitts closed 10 years ago

sfitts commented 10 years ago

Added the ability to interact with the Docker server via the REST API (using docker-java) instead of the Docker command line client. Useful for platforms where the Docker client is unavailable (aka Windows).

Use of the API is enabled through the useApi convention which defaults to FALSE (thus preserving the existing behavior). This allows use in environments where the Docker server is not reachable via the API (the docker-java code doesn't support UD sockets yet).

sfitts commented 10 years ago

I saw that this was on your roadmap, so feel free to take this or not as you see fit. I needed it because we have Windows developers so the command line client approach didn't work.

bjornmagnusson commented 10 years ago

Thanks for your contribution, looks promising. Do you mind adding some tests for this new feature, and also some description in readme file how to configure and use it?

sfitts commented 10 years ago

Thanks -- be glad to add those. Based on the existing tests I can easily see how to add tests for the new contextDir directive and for another change I'm about to make (allowing addFile to put files somewhere other than /). I'm less clear on how to approach testing of the useApi directive.

Normally my preference for testing something like this would be to mock the API client and then confirm that we're issuing the commands we expect to (as well as confirming that we use the API and not the command line). However, I don't see any similar style tests. Would it be OK for me to introduce a mocking framework and if so do you have one that you prefer?

sfitts commented 10 years ago

One question -- slightly off the topic of this pull -- what would you think about aligning the names of the various DSL methods (such as addFile and exposePort) with their Dockerfile counterpart? I find it hard to remember which ones match and which ones don't and I think it would make the build scripts easier to read by someone familiar with the Dockerfile syntax.

If you wouldn't mind that I think I'll look at making that change (in a different pull).

sfitts commented 10 years ago

Just a quick note to let you know that I do still plan on making the suggested changes here. Got some stuff going on at work, but should get some time this weekend.

mattgruter commented 10 years ago

@sfitts thank you for the work you've done.

I just merged your changes into our dev branch. We'll have to wait for a merge to master and release until docker-java 0.9.1 is released. I didn't see why we need 0.9.1. What change since 0.9.0 do we rely on?

Now that your changes are merged we can work on it from both ends (I gave you push permissions to the gradle-docker repo).

A couple of tests would be nice as @bjornmagnusson mentioned above. We usually use mockito for mocking. Feel free to add it if you need it.

Also I'd like to document the new useApi property in the README.md. If you have some spare time to add a couple of words would be nice. Otherwise I'll do it as soon as I have some spare time.

mattgruter commented 10 years ago

I'm planning to release a new version of the plugin (1.2) today or tomorrow. We'll hold off with your useApi contribution for that release since docker-java hasn't released 0.9.1 yet. Or could we release with 0.9?

Anyway once 0.9.1 is released we can quickly release a new version of the plugin.

mattgruter commented 10 years ago

About the question regarding the DSL. I agree we could (and should) align our DSL to the Dockerfile DSL. The most flexible and future-proof way would be by using groovy's NodeBuilder. Using the builder pattern we wouldn't have to duplicate the Dockerfile DSL.

sfitts commented 10 years ago

We could use 0,9.0 - the one thing I wanted to get from 0.9.1 was a change I submitted to make build work like it does for the command line -- namely default to the --rm option. I also added code to allow the commands objects to be printed to show what options they are using. But neither of these changes is strictly speaking required.

sfitts commented 10 years ago

Writing the usage documentation now.

sfitts commented 10 years ago

One other reason to use 0.9.1 is that it uses the current Docker default port of 2375 instead of the old value of 4243. This can be worked around by configuring the port, but you'd want to document that.

mattgruter commented 10 years ago

Ok, I think it's best we'll wait with the release of 1.2 until next week so that we can include your contribution (using docker-java 0.9.0). I'll also try to finally fix the addFile issue until then.

mattgruter commented 10 years ago

I extracted the logic around native-client vs java-client into an interface with 2 implementations: NativeDockerClient and JavaDockerClient. This helps to unclutter the overly long DockerTask class.

sfitts commented 10 years ago

Moved all of this over to forked dev branch and issued a pull there for the README (and a change to use the new Docker port so we can use 0.9.0).

Next up, some new tests.

BTW -- the client refactoring looks great.

This pull can probably be closed at this point (since all the changes are now over in dev).

mattgruter commented 10 years ago

All changes merged into dev branch. Closing.