Upgrade docker-client dep from 5.0.2 to 7.0.0-SNAPSHOT

helios-client/pom.xml

@@ -49,7 +49,6 @@
     <dependency>
       <groupId>org.apache.httpcomponents</groupId>
       <artifactId>httpclient</artifactId>
-      <version>4.5</version>
     </dependency>
     <dependency>
       <groupId>org.bouncycastle</groupId>

helios-services/pom.xml

@@ -196,7 +196,6 @@
     <dependency>
       <groupId>org.apache.httpcomponents</groupId>
       <artifactId>httpclient</artifactId>
-      <version>4.5</version>
     </dependency>
     <dependency>
       <groupId>joda-time</groupId>
@@ -296,7 +295,6 @@
     <dependency>
       <groupId>com.spotify</groupId>
       <artifactId>docker-client</artifactId>
-      <version>5.0.2</version>
       <classifier>shaded</classifier>
     </dependency>
     <dependency>
@@ -319,6 +317,12 @@
       <version>0.8</version>
     </dependency>
 
+    <!-- versions that we have to manage to avoid requireUpperBoundDeps violations -->
+    <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>httpcore</artifactId>
+    </dependency>
+
     <!--test deps-->
     <dependency>
       <groupId>com.spotify</groupId>

helios-services/src/main/java/com/spotify/helios/agent/BindVolumeContainerDecorator.java

@@ -67,16 +67,18 @@ public static boolean isValidBind(final String bind) {
 
   @Override
   public void decorateHostConfig(Job job, Optional<String> dockerVersion,
-                                 HostConfig.Builder hostConfig) {
+                                 HostConfig.Builder hostConfigBuilder) {
     final List<String> b = Lists.newArrayList();
 
+    final HostConfig hostConfig = hostConfigBuilder.build();
+
     if (hostConfig.binds() != null) {
       b.addAll(hostConfig.binds());
     }
 
     b.addAll(this.binds);
 
-    hostConfig.binds(b);
+    hostConfigBuilder.binds(b);
   }
 
   @Override

helios-services/src/main/java/com/spotify/helios/agent/HealthCheckerFactory.java

@@ -30,10 +30,13 @@
 import com.spotify.docker.client.DockerHost;
 import com.spotify.docker.client.LogStream;
 import com.spotify.docker.client.exceptions.DockerException;
+import com.spotify.docker.client.messages.ExecCreation;
 import com.spotify.helios.common.descriptors.ExecHealthCheck;
 import com.spotify.helios.common.descriptors.HealthCheck;
 import com.spotify.helios.common.descriptors.HttpHealthCheck;
 import com.spotify.helios.common.descriptors.TcpHealthCheck;
+
+import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.InetSocketAddress;
 import java.net.MalformedURLException;
@@ -88,13 +91,17 @@ public boolean check(final String containerId) {
 
       try {
         final List<String> cmd = healthCheck.getCommand();
-        final String execId = docker.execCreate(containerId, cmd.toArray(new String[cmd.size()]),
-                                                DockerClient.ExecCreateParam.attachStdout(),
-                                                DockerClient.ExecCreateParam.attachStderr());
+        final ExecCreation execCreation = docker.execCreate(
+            containerId, cmd.toArray(new String[cmd.size()]),
+            DockerClient.ExecCreateParam.attachStdout(),
+            DockerClient.ExecCreateParam.attachStderr());
+        final String execId = execCreation.id();
 
-        final String output;
+        String output = "";
         try (LogStream stream = docker.execStart(execId)) {
           output = stream.readFully();
+        } catch (IOException e) {
+          // thrown from implicit call to close(). Ignore.
         }
 
         final int exitCode = docker.execInspect(execId).exitCode();

helios-services/src/main/java/com/spotify/helios/agent/PollingDockerClient.java

@@ -58,7 +58,7 @@ public ContainerExit waitContainer(final String containerId)
     while (true) {
       final ContainerInfo info = inspectContainer(containerId);
       if (!info.state().running()) {
-        return new ContainerExit(info.state().exitCode());
+        return ContainerExit.create(info.state().exitCode());
       }
       Thread.sleep(WAIT_INSPECT_INTERVAL_MILLIS);
     }

helios-services/src/main/java/com/spotify/helios/agent/SyslogRedirectingContainerDecorator.java

@@ -29,7 +29,10 @@
 import com.spotify.docker.client.messages.ImageInfo;
 import com.spotify.docker.client.messages.LogConfig;
 import com.spotify.helios.common.descriptors.Job;
+
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -46,14 +49,15 @@ public SyslogRedirectingContainerDecorator(final String syslogHostPort) {
 
   @Override
   public void decorateHostConfig(Job job, Optional<String> dockerVersion,
-                                 HostConfig.Builder hostConfig) {
+                                 HostConfig.Builder hostConfigBuilder) {
+    final HostConfig hostConfig = hostConfigBuilder.build();
     if (useSyslogRedirector(dockerVersion)) {
       final List<String> binds = Lists.newArrayList();
       if (hostConfig.binds() != null) {
         binds.addAll(hostConfig.binds());
       }
       binds.add("/usr/lib/helios:/helios:ro");
-      hostConfig.binds(binds);
+      hostConfigBuilder.binds(binds);
     } else {
       final ImmutableMap.Builder<String, String> logOpts = ImmutableMap.builder();
 
@@ -62,13 +66,13 @@ public void decorateHostConfig(Job job, Optional<String> dockerVersion,
 
       logOpts.put("tag", job.getId().toString());
 
-      hostConfig.logConfig(LogConfig.create("syslog", logOpts.build()));
+      hostConfigBuilder.logConfig(LogConfig.create("syslog", logOpts.build()));
     }
   }
 
   @Override
   public void decorateContainerConfig(Job job, ImageInfo imageInfo, Optional<String> dockerVersion,
-                                      ContainerConfig.Builder containerConfig) {
+                                      ContainerConfig.Builder containerConfigBuilder) {
     if (!useSyslogRedirector(dockerVersion)) {
       return;
     }
@@ -86,21 +90,23 @@ public void decorateContainerConfig(Job job, ImageInfo imageInfo, Optional<Strin
     if (imageConfig.entrypoint() != null) {
       entrypoint.addAll(imageConfig.entrypoint());
     }
-    containerConfig.entrypoint(entrypoint);
+    containerConfigBuilder.entrypoint(entrypoint);
+
+    final ContainerConfig containerConfig = containerConfigBuilder.build();
 
     // If there's no explicit container cmd specified, copy over the one from the image.
     // Only setting the entrypoint causes dockerd to not use the image cmd.
     if ((containerConfig.cmd() == null || containerConfig.cmd().isEmpty())
         && imageConfig.cmd() != null) {
-      containerConfig.cmd(imageConfig.cmd());
+      containerConfigBuilder.cmd(imageConfig.cmd());
     }
 
-    final Set<String> volumes = Sets.newHashSet();
+    final ImmutableMap.Builder<String, Map> volumesBuilder = ImmutableMap.builder();
     if (containerConfig.volumes() != null) {
-      volumes.addAll(containerConfig.volumes());
+      volumesBuilder.putAll(containerConfig.volumes());
     }
-    volumes.add("/helios");
-    containerConfig.volumes(volumes);
+    volumesBuilder.put("/helios", new HashMap<>());
+    containerConfigBuilder.volumes(volumesBuilder.build());
   }
 
   private boolean useSyslogRedirector(Optional<String> dockerVersion) {

helios-services/src/main/java/com/spotify/helios/agent/TaskConfig.java

@@ -260,12 +260,12 @@ private Map<String, List<PortBinding>> portBindings() {
     final Map<String, List<PortBinding>> bindings = Maps.newHashMap();
     for (final Map.Entry<String, PortMapping> e : job.getPorts().entrySet()) {
       final PortMapping mapping = e.getValue();
-      final PortBinding binding = new PortBinding();
+      final PortBinding binding;
       final Integer externalPort = mapping.getExternalPort();
       if (externalPort == null) {
-        binding.hostPort(ports.get(e.getKey()).toString());
+        binding = PortBinding.of(null, ports.get(e.getKey()).toString());
       } else {
-        binding.hostPort(externalPort.toString());
+        binding = PortBinding.of(null, externalPort.toString());
       }
       final String entry = containerPort(mapping.getInternalPort(), mapping.getProtocol());
       bindings.put(entry, Collections.singletonList(binding));

helios-services/src/test/java/com/spotify/helios/agent/ExecHealthCheckerTest.java

@@ -20,6 +20,7 @@
 
 package com.spotify.helios.agent;
 
+import static java.util.Collections.emptyList;
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Matchers.any;
@@ -31,6 +32,7 @@
 import com.spotify.docker.client.DockerClient;
 import com.spotify.docker.client.DockerClient.ExecStartParameter;
 import com.spotify.docker.client.LogStream;
+import com.spotify.docker.client.messages.ExecCreation;
 import com.spotify.docker.client.messages.ExecState;
 import com.spotify.docker.client.messages.Info;
 import com.spotify.docker.client.messages.Version;
@@ -73,7 +75,7 @@ public void setUp() throws Exception {
     when(docker.version()).thenReturn(version);
     when(docker.execCreate(eq(CONTAINER_ID), any(String[].class),
                            (DockerClient.ExecCreateParam) anyVararg()))
-        .thenReturn(EXEC_ID);
+        .thenReturn(ExecCreation.create(EXEC_ID, emptyList()));
     when(docker.execStart(eq(EXEC_ID), (ExecStartParameter) anyVararg())).thenReturn(log);
     when(docker.execInspect(EXEC_ID)).thenReturn(execState);
 

helios-services/src/test/java/com/spotify/helios/agent/GracePeriodTest.java

@@ -35,6 +35,7 @@
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -51,7 +52,6 @@
 import com.spotify.docker.client.messages.ContainerState;
 import com.spotify.docker.client.messages.ImageInfo;
 import com.spotify.docker.client.messages.NetworkSettings;
-import com.spotify.docker.client.messages.PortBinding;
 import com.spotify.helios.TemporaryPorts;
 import com.spotify.helios.common.descriptors.Job;
 import com.spotify.helios.common.descriptors.JobId;
@@ -120,30 +120,9 @@ public class GracePeriodTest {
   static final Map<String, String> ENV = ImmutableMap.of("foo", "17", "bar", "4711");
   static final Set<String> EXPECTED_CONTAINER_ENV = ImmutableSet.of("foo=17", "bar=4711");
 
-  public static final ContainerInfo RUNNING_RESPONSE = new ContainerInfo() {
-    @Override
-    public ContainerState state() {
-      final ContainerState state = Mockito.mock(ContainerState.class);
-      when(state.running()).thenReturn(true);
-      return state;
-    }
+  public final ContainerInfo runningResponse = mock(ContainerInfo.class);
 
-    @Override
-    public NetworkSettings networkSettings() {
-      return NetworkSettings.builder()
-          .ports(Collections.<String, List<PortBinding>>emptyMap())
-          .build();
-    }
-  };
-
-  public static final ContainerInfo STOPPED_RESPONSE = new ContainerInfo() {
-    @Override
-    public ContainerState state() {
-      final ContainerState state = Mockito.mock(ContainerState.class);
-      when(state.running()).thenReturn(false);
-      return state;
-    }
-  };
+  public final ContainerInfo stoppedResponse = mock(ContainerInfo.class);
 
   @Mock public AgentModel model;
   @Mock public DockerClient docker;
@@ -159,6 +138,17 @@ public ContainerState state() {
 
   @Before
   public void setup() throws Exception {
+    final ContainerState runningState = Mockito.mock(ContainerState.class);
+    when(runningState.running()).thenReturn(true);
+    when(runningResponse.state()).thenReturn(runningState);
+    when(runningResponse.networkSettings()).thenReturn(NetworkSettings.builder()
+        .ports(Collections.emptyMap())
+        .build());
+
+    final ContainerState stoppedState = Mockito.mock(ContainerState.class);
+    when(stoppedState.running()).thenReturn(false);
+    when(stoppedResponse.state()).thenReturn(stoppedState);
+
     when(retryPolicy.delay(any(ThrottleState.class))).thenReturn(10L);
     when(registrar.register(any(ServiceRegistration.class)))
         .thenReturn(new NopServiceRegistrationHandle());
@@ -230,7 +220,7 @@ public void teardown() throws Exception {
   public void verifySupervisorStartsAndStopsDockerContainer() throws Exception {
     final String containerId = "deadbeef";
 
-    final ContainerCreation createResponse = new ContainerCreation(containerId);
+    final ContainerCreation createResponse = ContainerCreation.builder().id(containerId).build();
 
     final SettableFuture<ContainerCreation> createFuture = SettableFuture.create();
     when(docker.createContainer(any(ContainerConfig.class),
@@ -240,7 +230,7 @@ public void verifySupervisorStartsAndStopsDockerContainer() throws Exception {
     doAnswer(futureAnswer(startFuture))
         .when(docker).startContainer(eq(containerId));
 
-    final ImageInfo imageInfo = new ImageInfo();
+    final ImageInfo imageInfo = mock(ImageInfo.class);
     when(docker.inspectImage(IMAGE)).thenReturn(imageInfo);
 
     final SettableFuture<ContainerExit> waitFuture = SettableFuture.create();
@@ -294,7 +284,7 @@ public void verifySupervisorStartsAndStopsDockerContainer() throws Exception {
                                                        .setEnv(ENV)
                                                        .build())
     );
-    when(docker.inspectContainer(eq(containerId))).thenReturn(RUNNING_RESPONSE);
+    when(docker.inspectContainer(eq(containerId))).thenReturn(runningResponse);
     startFuture.set(null);
 
     verify(docker, timeout(30000)).waitContainer(containerId);
@@ -329,7 +319,7 @@ public Void call() throws Exception {
     verify(sleeper).sleep(GRACE_PERIOD_MILLIS);
 
     // Change docker container state to stopped when it's killed
-    when(docker.inspectContainer(eq(containerId))).thenReturn(STOPPED_RESPONSE);
+    when(docker.inspectContainer(eq(containerId))).thenReturn(stoppedResponse);
     killFuture.set(null);
 
     // Verify that the stopping state is signalled