From f5d2b9e927b34feba634de1ba8a37f2cbed57129 Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Tue, 9 Dec 2014 17:32:27 +0100 Subject: [PATCH 1/2] Tests for starting a Container with existing configuration This illustrates an issue with current docker-java: you cannot send a StartContainerCmd without payload. --- .../command/StartContainerCmdImplTest.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/test/java/com/github/dockerjava/core/command/StartContainerCmdImplTest.java b/src/test/java/com/github/dockerjava/core/command/StartContainerCmdImplTest.java index fedbc5fa1..98704fd33 100644 --- a/src/test/java/com/github/dockerjava/core/command/StartContainerCmdImplTest.java +++ b/src/test/java/com/github/dockerjava/core/command/StartContainerCmdImplTest.java @@ -2,6 +2,7 @@ import static com.github.dockerjava.api.model.AccessMode.ro; import static com.github.dockerjava.api.model.Capability.*; +import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; @@ -386,4 +387,55 @@ public void startContainerWithRestartPolicy() throws DockerException { is(equalTo(restartPolicy))); } + @Test + public void existingHostConfigIsPreservedByBlankStartCmd() throws DockerException { + + String dnsServer = "8.8.8.8"; + + // prepare a container with custom DNS + CreateContainerResponse container = dockerClient + .createContainerCmd("busybox") + .withDns(dnsServer) + .withCmd("true").exec(); + + LOG.info("Created container {}", container.toString()); + + assertThat(container.getId(), not(isEmptyString())); + + // start container _without_any_customization_ (important!) + dockerClient.startContainerCmd(container.getId()).exec(); + + InspectContainerResponse inspectContainerResponse = dockerClient.inspectContainerCmd(container + .getId()).exec(); + + // The DNS setting survived. + assertThat(inspectContainerResponse.getHostConfig().getDns(), is(notNullValue())); + assertThat(Arrays.asList(inspectContainerResponse.getHostConfig().getDns()), + contains(dnsServer)); + } + + @Test + public void existingHostConfigIsResetByConfiguredStartCmd() throws DockerException { + + String dnsServer = "8.8.8.8"; + + // prepare a container with custom DNS + CreateContainerResponse container = dockerClient + .createContainerCmd("busybox") + .withDns(dnsServer) + .withCmd("true").exec(); + + LOG.info("Created container {}", container.toString()); + + assertThat(container.getId(), not(isEmptyString())); + + // modify another setting in start command. Leave DNS unchanged. + dockerClient.startContainerCmd(container.getId()).withPublishAllPorts(true).exec(); + + InspectContainerResponse inspectContainerResponse = dockerClient.inspectContainerCmd(container + .getId()).exec(); + + // although start did not modify DNS Settings, they were reset to their default. + assertThat(inspectContainerResponse.getHostConfig().getDns(), is(nullValue(String[].class))); + } } From 72d90218926c76c68817ef432ac1bc90cb23119a Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Tue, 9 Dec 2014 18:37:58 +0100 Subject: [PATCH 2/2] Do not serialize unconfigured values in StartContainerCmd Any payload sent with the StartContainerCmd will cause the target container to lose its customization. So in order to just start a preconfigured container you must be able to send an empty "{}" message to the /containers/{id}/start endpoint. This is accomplished by all configuration defaulting to null in combination with @JsonInclude(NON_EMPTY). --- .../api/command/StartContainerCmd.java | 8 +++--- .../core/command/StartContainerCmdImpl.java | 27 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java b/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java index 59aab1ed3..ef96267f7 100644 --- a/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java +++ b/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java @@ -24,9 +24,9 @@ public interface StartContainerCmd extends DockerCmd { public Ports getPortBindings(); - public boolean isPublishAllPorts(); + public Boolean isPublishAllPorts(); - public boolean isPrivileged(); + public Boolean isPrivileged(); public String[] getDns(); @@ -70,9 +70,9 @@ public interface StartContainerCmd extends DockerCmd { */ public StartContainerCmd withPortBindings(PortBinding... portBindings); - public StartContainerCmd withPrivileged(boolean privileged); + public StartContainerCmd withPrivileged(Boolean privileged); - public StartContainerCmd withPublishAllPorts(boolean publishAllPorts); + public StartContainerCmd withPublishAllPorts(Boolean publishAllPorts); /** * Set custom DNS servers diff --git a/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java b/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java index 60d728f0f..63ff4ff0e 100644 --- a/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java +++ b/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java @@ -1,8 +1,11 @@ package com.github.dockerjava.core.command; +import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_EMPTY; + import org.apache.commons.lang.builder.ToStringBuilder; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.github.dockerjava.api.NotFoundException; import com.github.dockerjava.api.NotModifiedException; @@ -22,15 +25,17 @@ /** * Start a container */ +@JsonInclude(NON_EMPTY) public class StartContainerCmdImpl extends AbstrDockerCmd implements StartContainerCmd { + @JsonIgnore private String containerId; @JsonProperty("Binds") - private Binds binds = new Binds(); + private Binds binds; @JsonProperty("Links") - private Links links = new Links(); + private Links links; @JsonProperty("LxcConf") private LxcConf[] lxcConf; @@ -39,10 +44,10 @@ public class StartContainerCmdImpl extends AbstrDockerCmd