You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2015/11/10 16:53:24 UTC
[jclouds-labs] add docker NetworkAPI (#221)
- bump api version to 1.21
- use `alpine/3.2` image for liveTests
- use `kwart/alpine-ext:3.2-ssh` image as ssh-able image
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/221
-- Commit Summary --
* add docker NetworkAPI
-- File Changes --
M docker/README.md (4)
M docker/pom.xml (2)
M docker/src/main/java/org/jclouds/docker/DockerApi.java (8)
M docker/src/main/java/org/jclouds/docker/DockerApiMetadata.java (18)
M docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java (1)
A docker/src/main/java/org/jclouds/docker/domain/Network.java (105)
M docker/src/main/java/org/jclouds/docker/domain/NetworkSettings.java (175)
M docker/src/main/java/org/jclouds/docker/domain/State.java (17)
A docker/src/main/java/org/jclouds/docker/features/NetworkApi.java (100)
M docker/src/test/java/org/jclouds/docker/compute/BaseDockerApiLiveTest.java (5)
M docker/src/test/java/org/jclouds/docker/compute/DockerComputeServiceAdapterLiveTest.java (7)
M docker/src/test/java/org/jclouds/docker/compute/functions/ContainerToNodeMetadataTest.java (6)
M docker/src/test/java/org/jclouds/docker/config/DockerParserModuleTest.java (69)
M docker/src/test/java/org/jclouds/docker/features/ContainerApiLiveTest.java (9)
M docker/src/test/java/org/jclouds/docker/features/ContainerApiMockTest.java (2)
M docker/src/test/java/org/jclouds/docker/features/ImageApiLiveTest.java (3)
M docker/src/test/java/org/jclouds/docker/features/MiscApiLiveTest.java (12)
A docker/src/test/java/org/jclouds/docker/features/NetworkApiLiveTest.java (123)
A docker/src/test/java/org/jclouds/docker/features/NetworkApiMockTest.java (155)
M docker/src/test/java/org/jclouds/docker/parse/ContainerParseTest.java (45)
A docker/src/test/java/org/jclouds/docker/parse/NetworkParseTest.java (71)
A docker/src/test/java/org/jclouds/docker/parse/NetworksParseTest.java (97)
M docker/src/test/resources/container.json (270)
A docker/src/test/resources/network-creation.json (4)
A docker/src/test/resources/network.json (30)
A docker/src/test/resources/networks.json (56)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/221.patch
https://github.com/jclouds/jclouds-labs/pull/221.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> +package org.jclouds.docker.domain;
> +
> +import static org.jclouds.docker.internal.NullSafeCopies.copyOf;
> +import java.util.List;
> +import java.util.Map;
> +
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +
> +@AutoValue
> +public abstract class Network {
> +
> + @AutoValue
> + public abstract static class IPAM {
[minor] Fix indentation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44466130
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
Thanks @nacx I think I'm done now, I hope :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#issuecomment-155814158
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
Closed #221.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#event-463732998
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> }
>
> public Builder fromNetworkSettings(NetworkSettings in) {
> return this.ipAddress(in.ipAddress()).ipPrefixLen(in.ipPrefixLen()).gateway(in.gateway()).bridge(in.bridge())
> - .portMapping(in.portMapping()).ports(in.ports());
> + .portMapping(in.portMapping()).ports(in.ports()).sandboxId(in.sandboxId()).hairpinMode(in.hairpinMode()).linkLocalIPv6Address(in
> + .linkLocalIPv6Address()).linkLocalIPv6PrefixLen(in.linkLocalIPv6PrefixLen()).sandboxKey(in.sandboxKey()).secondaryIPAddresses(in
> + .secondaryIPAddresses()).secondaryIPv6Addresses(in.secondaryIPv6Addresses()).endpointId(in.endpointId()).globalIPv6Address(in
> + .globalIPv6Address()).globalIPv6PrefixLen(in.globalIPv6PrefixLen()).ipv6Gateway(in.ipv6Gateway()).macAddress(in.macAddress())
> + .networks(in.networks());
Not for this PR, but we should consider moving all builders to google auto builders.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44467077
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + try {
> + api.removeNetwork(networkId);
> + assertSent(server, "DELETE", "/networks/" + networkId);
> +
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + public void testConnectContainerToNetwork() throws Exception {
> + MockWebServer server = mockWebServer(new MockResponse().setResponseCode(200));
> + NetworkApi api = api(DockerApi.class, server.getUrl("/").toString()).getNetworkApi();
> + try {
> + api.connectContainerToNetwork("1", "containerName");
> + assertSent(server, "POST", "/networks/1/connect");
What I mean, is that the call generates a body; not only a requestLine. The assertSent should also verify that it is sending the right payload. Take as an example the [GCE mock tests](https://github.com/jclouds/jclouds/blob/master/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiMockTest.java#L85-L117).
This should be applied to all mock test classes, but let's fix just the network mock in this PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44520654
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> +
> + @Test(dependsOnMethods = "testGetNetwork")
> + public void testAttachContainerToNetwork() {
> + api().connectContainerToNetwork(network.id(), container.id());
> + Container foundContainer = api.getContainerApi().inspectContainer(container.id());
> + assertNotNull(Iterables.tryFind(foundContainer.networkSettings().networks().keySet(), Predicates.equalTo(network.name())).orNull());
> + }
> +
> + @Test(dependsOnMethods = "testAttachContainerToNetwork")
> + public void testDisconnectContainerFromNetwork() {
> + api().disconnectContainerFromNetwork(network.id(), container.id());
> + // TODO assertion
> + }
> +
> + @Test(dependsOnMethods = "testDisconnectContainerFromNetwork")
Make it depend on the `testCreateNetwork` to maximise the chances it is not skipped due to a previous test failure.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44468269
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
> +
> + try {
> + api.removeNetwork(networkId);
> + assertSent(server, "DELETE", "/networks/" + networkId);
> +
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + public void testConnectContainerToNetwork() throws Exception {
> + MockWebServer server = mockWebServer(new MockResponse().setResponseCode(200));
> + NetworkApi api = api(DockerApi.class, server.getUrl("/").toString()).getNetworkApi();
> + try {
> + api.connectContainerToNetwork("1", "containerName");
> + assertSent(server, "POST", "/networks/1/connect");
I think I was using `@Produces` annotation in a wrong way as I had to use `@Headers` for `Content-Type` so I think it is now better. Sorry for the confusion!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44519204
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
> + container = api.getContainerApi().inspectContainer(container.id());
> + }
> +
> + @AfterClass
> + protected void tearDown() {
> + if (container != null) {
> + api.getContainerApi().stopContainer(container.id());
> + api.getContainerApi().removeContainer(container.id());
> + }
> + if (network != null) {
> + api().removeNetwork(network.id());
> + }
> + }
> +
> + public void testCreateNetwork() throws IOException, InterruptedException {
> + network = api().createNetwork(Network.create(NETWORK_NAME, null, null, null, null, ImmutableMap.<String, Network.Details> of(), ImmutableMap.<String, String> of()));
Not sure, maybe when I'll add auto value builders things will be easier?
I think there are a bunch of options:
- multiple Network.create()
- `NetworkCreate`
- Network.builder(). ... .build() using AutoValue builders
but maybe we should have some sort of guidelines.
wdyt?
Happy to solve it here or in a subsequent PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44516823
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
Nice job updating the model to the latest version @andreaturli. Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#issuecomment-155574738
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
Just one missing assertion in the mock test!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#issuecomment-156247260
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + try {
> + api.removeNetwork(networkId);
> + assertSent(server, "DELETE", "/networks/" + networkId);
> +
> + } finally {
> + server.shutdown();
> + }
> + }
> +
> + public void testConnectContainerToNetwork() throws Exception {
> + MockWebServer server = mockWebServer(new MockResponse().setResponseCode(200));
> + NetworkApi api = api(DockerApi.class, server.getUrl("/").toString()).getNetworkApi();
> + try {
> + api.connectContainerToNetwork("1", "containerName");
> + assertSent(server, "POST", "/networks/1/connect");
I know that it is not done in any of the mock tests, but when testing methods that produce a body, we should also verify the sent body is the expected one. Let's add these checks to this test and fix the other mock tests in upcoming PRs.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44468806
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @andreaturli! Let's squash and merge the PR!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#issuecomment-156463798
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
> + assertNotNull(network);
> + assertNotNull(network.id());
> +
> + }
> +
> + @Test(dependsOnMethods = "testCreateNetwork")
> + public void testGetNetwork() {
> + network = api().inspectNetwork(network.id());
> + assertNotNull(network);
> + }
> +
> + @Test(dependsOnMethods = "testGetNetwork")
> + public void testAttachContainerToNetwork() {
> + api().connectContainerToNetwork(network.id(), container.id());
> + Container foundContainer = api.getContainerApi().inspectContainer(container.id());
> + assertNotNull(Iterables.tryFind(foundContainer.networkSettings().networks().keySet(), Predicates.equalTo(network.name())).orNull());
+1
thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44516836
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + container = api.getContainerApi().inspectContainer(container.id());
> + }
> +
> + @AfterClass
> + protected void tearDown() {
> + if (container != null) {
> + api.getContainerApi().stopContainer(container.id());
> + api.getContainerApi().removeContainer(container.id());
> + }
> + if (network != null) {
> + api().removeNetwork(network.id());
> + }
> + }
> +
> + public void testCreateNetwork() throws IOException, InterruptedException {
> + network = api().createNetwork(Network.create(NETWORK_NAME, null, null, null, null, ImmutableMap.<String, Network.Details> of(), ImmutableMap.<String, String> of()));
It seems convenient to add a NetworkCreate object to simplify network creation, allowing users to only pass the relevant parameters. WDYT?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44467867
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + assertNotNull(network);
> + assertNotNull(network.id());
> +
> + }
> +
> + @Test(dependsOnMethods = "testCreateNetwork")
> + public void testGetNetwork() {
> + network = api().inspectNetwork(network.id());
> + assertNotNull(network);
> + }
> +
> + @Test(dependsOnMethods = "testGetNetwork")
> + public void testAttachContainerToNetwork() {
> + api().connectContainerToNetwork(network.id(), container.id());
> + Container foundContainer = api.getContainerApi().inspectContainer(container.id());
> + assertNotNull(Iterables.tryFind(foundContainer.networkSettings().networks().keySet(), Predicates.equalTo(network.name())).orNull());
Would it be more readable as `assertTrue(Iterables.any(...)` ?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44468123
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + ),
> + ImmutableMap.of("39b69226f9d79f5634485fb236a23b2fe4e96a0a94128390a7fbbcc167065867",
> + Network.Details.create(
> + "ed2419a97c1d9954d05b46e462e7002ea552f216e9b136b80a7db8d98b442eda", //endpointId
> + "02:42:ac:11:00:02", // MAC
> + "172.17.0.2/16", // ipv4address
> + "" // ipv6address
> + )
> + ),
> + options);
> +
> + try {
> + Network created = api.createNetwork(network);
> + assertNotNull(created);
> + assertThat(created.id()).isEqualTo("22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30");
> + assertSent(server, "POST", "/networks/create");
Verify the body here too
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44720072
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
> + public void testGetNetwork() {
> + network = api().inspectNetwork(network.id());
> + assertNotNull(network);
> + }
> +
> + @Test(dependsOnMethods = "testGetNetwork")
> + public void testAttachContainerToNetwork() {
> + api().connectContainerToNetwork(network.id(), container.id());
> + Container foundContainer = api.getContainerApi().inspectContainer(container.id());
> + assertNotNull(Iterables.tryFind(foundContainer.networkSettings().networks().keySet(), Predicates.equalTo(network.name())).orNull());
> + }
> +
> + @Test(dependsOnMethods = "testAttachContainerToNetwork")
> + public void testDisconnectContainerFromNetwork() {
> + api().disconnectContainerFromNetwork(network.id(), container.id());
> + // TODO assertion
ops :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44516620
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
Using the following versions
```
$ docker version
Client:
Version: 1.9.0
API version: 1.21
Go version: go1.4.3
Git commit: 76d6bc9
Built: Tue Nov 3 19:20:09 UTC 2015
OS/Arch: darwin/amd64
Server:
Version: 1.9.0
API version: 1.21
Go version: go1.4.3
Git commit: 76d6bc9
Built: Tue Nov 3 19:20:09 UTC 2015
OS/Arch: linux/amd64
```
and starting from a clean docker host, this is the result of the liveTests
```
Results :
Tests run: 32, Failures: 0, Errors: 0, Skipped: 0
...
[INFO] BUILD SUCCESS
```
in ~1 minute.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#issuecomment-155462673
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
unfortunately the failure is not related to this PR but to https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/927/org.apache.jclouds.labs$jdbc/
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#issuecomment-155475255
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
> }
>
> public Builder fromNetworkSettings(NetworkSettings in) {
> return this.ipAddress(in.ipAddress()).ipPrefixLen(in.ipPrefixLen()).gateway(in.gateway()).bridge(in.bridge())
> - .portMapping(in.portMapping()).ports(in.ports());
> + .portMapping(in.portMapping()).ports(in.ports()).sandboxId(in.sandboxId()).hairpinMode(in.hairpinMode()).linkLocalIPv6Address(in
> + .linkLocalIPv6Address()).linkLocalIPv6PrefixLen(in.linkLocalIPv6PrefixLen()).sandboxKey(in.sandboxKey()).secondaryIPAddresses(in
> + .secondaryIPAddresses()).secondaryIPv6Addresses(in.secondaryIPv6Addresses()).endpointId(in.endpointId()).globalIPv6Address(in
> + .globalIPv6Address()).globalIPv6PrefixLen(in.globalIPv6PrefixLen()).ipv6Gateway(in.ipv6Gateway()).macAddress(in.macAddress())
> + .networks(in.networks());
completely agree! Next PRs will use the auto builders
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44516331
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + CreateImageOptions options = CreateImageOptions.Builder.fromImage(ALPINE_IMAGE_TAG);
> + InputStream createImageStream = api.getImageApi().createImage(options);
> + consumeStream(createImageStream);
> + }
> + image = api.getImageApi().inspectImage(ALPINE_IMAGE_TAG);
> + assertNotNull(image);
> +
> + Config containerConfig = Config.builder().image(image.id())
> + .cmd(ImmutableList.of("sh", "-c", "touch hello; while true; do echo hello world; sleep 1; done"))
> + .build();
> + container = api.getContainerApi().createContainer("jclouds-test-network", containerConfig);
> + api.getContainerApi().startContainer(container.id());
> + container = api.getContainerApi().inspectContainer(container.id());
> + }
> +
> + @AfterClass
Use `alwaysRun = true`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44467736
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + @Named("networks:list")
> + @GET
> + @Fallback(EmptyListOnNotFoundOr404.class)
> + List<Network> listNetworks();
> +
> + /**
> + * @param network the network’s configuration (@see BindToJsonPayload)
> + * @return a new network
> + */
> + @Named("network:create")
> + @POST
> + @Path("/create")
> + Network createNetwork(@BinderParam(BindToJsonPayload.class) Network network);
> +
> + /**
> + * Return low-level information on the container id
s/container/network/
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44467420
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Andrea Turli <no...@github.com>.
Thanks @nacx for the patience!
Merged at [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/c57ac931)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221#issuecomment-156470828
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + container = api.getContainerApi().inspectContainer(container.id());
> + }
> +
> + @AfterClass
> + protected void tearDown() {
> + if (container != null) {
> + api.getContainerApi().stopContainer(container.id());
> + api.getContainerApi().removeContainer(container.id());
> + }
> + if (network != null) {
> + api().removeNetwork(network.id());
> + }
> + }
> +
> + public void testCreateNetwork() throws IOException, InterruptedException {
> + network = api().createNetwork(Network.create(NETWORK_NAME, null, null, null, null, ImmutableMap.<String, Network.Details> of(), ImmutableMap.<String, String> of()));
+1 to the builders :) let's do that when they are implemented then.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44520373
Re: [jclouds-labs] add docker NetworkAPI (#221)
Posted by Ignasi Barrera <no...@github.com>.
> + public void testGetNetwork() {
> + network = api().inspectNetwork(network.id());
> + assertNotNull(network);
> + }
> +
> + @Test(dependsOnMethods = "testGetNetwork")
> + public void testAttachContainerToNetwork() {
> + api().connectContainerToNetwork(network.id(), container.id());
> + Container foundContainer = api.getContainerApi().inspectContainer(container.id());
> + assertNotNull(Iterables.tryFind(foundContainer.networkSettings().networks().keySet(), Predicates.equalTo(network.name())).orNull());
> + }
> +
> + @Test(dependsOnMethods = "testAttachContainerToNetwork")
> + public void testDisconnectContainerFromNetwork() {
> + api().disconnectContainerFromNetwork(network.id(), container.id());
> + // TODO assertion
Add it? :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/221/files#r44468014