From 0346e9163c66a7e432b7b3a45f78b275d2adc59d Mon Sep 17 00:00:00 2001 From: Henrique Gontijo Date: Mon, 24 Jul 2017 16:33:24 -0700 Subject: [PATCH 1/2] Add the boolean parameter 'Force' to DockerClient#disconnectFromNetwork as it's defined at [Docker Networks](https://docs.docker.com/engine/api/v1.30/#operation/NetworkDisconnect). Refactored accordingly due to the introduction of the new parameter. Fix issue at DockerClient#disconnectFromNetwork and DockerClient#connectToNetwork that led to an request error (HTTP 4xx, 5xx) to be swalled and no exception has been thrown. This was happening since the request was using Response.class and JerseyInvocation#translate would just report the error via Response.class whenever this class is used to carry the response data. If a different class other then Response.class is given, then Jersey will thrown an exception. --- .../docker/client/DefaultDockerClient.java | 19 ++++--- .../spotify/docker/client/DockerClient.java | 13 +++++ .../client/DefaultDockerClientTest.java | 56 +++++++++++++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/spotify/docker/client/DefaultDockerClient.java b/src/main/java/com/spotify/docker/client/DefaultDockerClient.java index c342306c3..8dcab7b1b 100644 --- a/src/main/java/com/spotify/docker/client/DefaultDockerClient.java +++ b/src/main/java/com/spotify/docker/client/DefaultDockerClient.java @@ -2230,7 +2230,7 @@ public void removeNetwork(String networkId) throws DockerException, InterruptedE @Override public void connectToNetwork(String containerId, String networkId) throws DockerException, InterruptedException { - manageNetworkConnection(containerId, "connect", networkId); + connectToNetwork(networkId, NetworkConnection.builder().containerId(containerId).build()); } @Override @@ -2239,7 +2239,7 @@ public void connectToNetwork(String networkId, NetworkConnection networkConnecti final WebTarget resource = resource().path("networks").path(networkId).path("connect"); try { - request(POST, Response.class, resource, resource.request(APPLICATION_JSON_TYPE), + request(POST, String.class, resource, resource.request(APPLICATION_JSON_TYPE), Entity.json(networkConnection)); } catch (DockerRequestException e) { switch (e.status()) { @@ -2256,18 +2256,20 @@ public void connectToNetwork(String networkId, NetworkConnection networkConnecti @Override public void disconnectFromNetwork(String containerId, String networkId) throws DockerException, InterruptedException { - manageNetworkConnection(containerId, "disconnect", networkId); + disconnectFromNetwork(containerId, networkId, false); } - private void manageNetworkConnection(String containerId, String methodname, String networkId) - throws DockerException, InterruptedException { - final WebTarget resource = resource().path("networks").path(networkId).path(methodname); + @Override + public void disconnectFromNetwork(String containerId, String networkId, boolean force) + throws DockerException, InterruptedException { + final WebTarget resource = resource().path("networks").path(networkId).path("disconnect"); - final Map request = new HashMap<>(); + final Map request = new HashMap<>(); request.put("Container", containerId); + request.put("Force", force); try { - request(POST, Response.class, resource, resource.request(APPLICATION_JSON_TYPE), + request(POST, String.class, resource, resource.request(APPLICATION_JSON_TYPE), Entity.json(request)); } catch (DockerRequestException e) { switch (e.status()) { @@ -2844,5 +2846,4 @@ public DefaultDockerClient build() { return new DefaultDockerClient(this); } } - } diff --git a/src/main/java/com/spotify/docker/client/DockerClient.java b/src/main/java/com/spotify/docker/client/DockerClient.java index c83859149..f9ed267a0 100644 --- a/src/main/java/com/spotify/docker/client/DockerClient.java +++ b/src/main/java/com/spotify/docker/client/DockerClient.java @@ -1947,6 +1947,19 @@ void connectToNetwork(String networkId, NetworkConnection networkConnection) void disconnectFromNetwork(String containerId, String networkId) throws DockerException, InterruptedException; + /** + * Disconnects a docker container to a network. + * + * @param containerId The id of the container to disconnect. + * @param networkId The id of the network to disconnect. + * @param force Force the container to disconnect from the network. + * @throws NotFoundException if either container or network is not found (404) + * @throws DockerException if a server error occurred (500) + * @throws InterruptedException If the thread is interrupted + */ + void disconnectFromNetwork(String containerId, String networkId, boolean force) + throws DockerException, InterruptedException; + /** * Closes any and all underlying connections to docker, and release resources. */ diff --git a/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java b/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java index da9709c88..f04abb6fe 100644 --- a/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java +++ b/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java @@ -4061,6 +4061,62 @@ public void testNetworksConnectContainer() throws Exception { } + @Test + public void testNetworksConnectContainerWithoutForcefulDisconnectNetwork() throws Exception { + requireDockerApiVersionAtLeast("1.21", "createNetwork and listNetworks"); + assumeFalse(CIRCLECI); + + sut.pull(BUSYBOX_LATEST); + final String networkName = randomName(); + final String containerName = randomName(); + final NetworkCreation networkCreation = + sut.createNetwork(NetworkConfig.builder().name(networkName).build()); + assertThat(networkCreation.id(), is(notNullValue())); + final ContainerConfig containerConfig = + ContainerConfig.builder().image(BUSYBOX_LATEST).cmd("sh", "-c", "while :; do sleep 1; done") + .build(); + final ContainerCreation containerCreation = sut.createContainer(containerConfig, containerName); + assertThat(containerCreation.id(), is(notNullValue())); + sut.startContainer(containerCreation.id()); + sut.connectToNetwork(containerCreation.id(), networkCreation.id()); + + Network network = sut.inspectNetwork(networkCreation.id()); + assertThat(network.containers().size(), equalTo(1)); + final Network.Container container = network.containers().get(containerCreation.id()); + assertThat(container, notNullValue()); + if (dockerApiVersionAtLeast("1.22")) { + assertThat(container.name(), notNullValue()); + } + assertThat(container.macAddress(), notNullValue()); + assertThat(container.ipv4Address(), notNullValue()); + assertThat(container.ipv6Address(), notNullValue()); + + final ContainerInfo containerInfo = sut.inspectContainer(containerCreation.id()); + assertThat(containerInfo.networkSettings().networks().size(), is(2)); + final AttachedNetwork attachedNetwork = + containerInfo.networkSettings().networks().get(networkName); + assertThat(attachedNetwork, is(notNullValue())); + if (dockerApiVersionAtLeast("1.22")) { + assertThat(attachedNetwork.networkId(), is(notNullValue())); + } + assertThat(attachedNetwork.endpointId(), is(notNullValue())); + assertThat(attachedNetwork.gateway(), is(notNullValue())); + assertThat(attachedNetwork.ipAddress(), is(notNullValue())); + assertThat(attachedNetwork.ipPrefixLen(), is(notNullValue())); + assertThat(attachedNetwork.macAddress(), is(notNullValue())); + assertThat(attachedNetwork.ipv6Gateway(), is(notNullValue())); + assertThat(attachedNetwork.globalIPv6Address(), is(notNullValue())); + assertThat(attachedNetwork.globalIPv6PrefixLen(), greaterThanOrEqualTo(0)); + sut.disconnectFromNetwork(containerCreation.id(), networkCreation.id(), true); + network = sut.inspectNetwork(networkCreation.id()); + assertThat(network.containers().size(), equalTo(0)); + + sut.stopContainer(containerCreation.id(), 1); + sut.removeContainer(containerCreation.id()); + sut.removeNetwork(networkCreation.id()); + + } + @Test public void testNetworksConnectContainerWithEndpointConfig() throws Exception { requireDockerApiVersionAtLeast("1.22", "createNetwork and listNetworks"); From 573fc6a58bdc7a0f71f5f8ec4173aa902eb8c5af Mon Sep 17 00:00:00 2001 From: Henrique Gontijo Date: Sat, 29 Jul 2017 15:14:47 -0700 Subject: [PATCH 2/2] Remove unnecessary unit test. --- .../client/DefaultDockerClientTest.java | 56 ------------------- 1 file changed, 56 deletions(-) diff --git a/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java b/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java index f04abb6fe..da9709c88 100644 --- a/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java +++ b/src/test/java/com/spotify/docker/client/DefaultDockerClientTest.java @@ -4061,62 +4061,6 @@ public void testNetworksConnectContainer() throws Exception { } - @Test - public void testNetworksConnectContainerWithoutForcefulDisconnectNetwork() throws Exception { - requireDockerApiVersionAtLeast("1.21", "createNetwork and listNetworks"); - assumeFalse(CIRCLECI); - - sut.pull(BUSYBOX_LATEST); - final String networkName = randomName(); - final String containerName = randomName(); - final NetworkCreation networkCreation = - sut.createNetwork(NetworkConfig.builder().name(networkName).build()); - assertThat(networkCreation.id(), is(notNullValue())); - final ContainerConfig containerConfig = - ContainerConfig.builder().image(BUSYBOX_LATEST).cmd("sh", "-c", "while :; do sleep 1; done") - .build(); - final ContainerCreation containerCreation = sut.createContainer(containerConfig, containerName); - assertThat(containerCreation.id(), is(notNullValue())); - sut.startContainer(containerCreation.id()); - sut.connectToNetwork(containerCreation.id(), networkCreation.id()); - - Network network = sut.inspectNetwork(networkCreation.id()); - assertThat(network.containers().size(), equalTo(1)); - final Network.Container container = network.containers().get(containerCreation.id()); - assertThat(container, notNullValue()); - if (dockerApiVersionAtLeast("1.22")) { - assertThat(container.name(), notNullValue()); - } - assertThat(container.macAddress(), notNullValue()); - assertThat(container.ipv4Address(), notNullValue()); - assertThat(container.ipv6Address(), notNullValue()); - - final ContainerInfo containerInfo = sut.inspectContainer(containerCreation.id()); - assertThat(containerInfo.networkSettings().networks().size(), is(2)); - final AttachedNetwork attachedNetwork = - containerInfo.networkSettings().networks().get(networkName); - assertThat(attachedNetwork, is(notNullValue())); - if (dockerApiVersionAtLeast("1.22")) { - assertThat(attachedNetwork.networkId(), is(notNullValue())); - } - assertThat(attachedNetwork.endpointId(), is(notNullValue())); - assertThat(attachedNetwork.gateway(), is(notNullValue())); - assertThat(attachedNetwork.ipAddress(), is(notNullValue())); - assertThat(attachedNetwork.ipPrefixLen(), is(notNullValue())); - assertThat(attachedNetwork.macAddress(), is(notNullValue())); - assertThat(attachedNetwork.ipv6Gateway(), is(notNullValue())); - assertThat(attachedNetwork.globalIPv6Address(), is(notNullValue())); - assertThat(attachedNetwork.globalIPv6PrefixLen(), greaterThanOrEqualTo(0)); - sut.disconnectFromNetwork(containerCreation.id(), networkCreation.id(), true); - network = sut.inspectNetwork(networkCreation.id()); - assertThat(network.containers().size(), equalTo(0)); - - sut.stopContainer(containerCreation.id(), 1); - sut.removeContainer(containerCreation.id()); - sut.removeNetwork(networkCreation.id()); - - } - @Test public void testNetworksConnectContainerWithEndpointConfig() throws Exception { requireDockerApiVersionAtLeast("1.22", "createNetwork and listNetworks");