Simplify Docker API

5 files changed, 21 insertions, 55 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
index cf168df4634..04aad562711 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
 
 import java.net.Inet6Address;
 import java.net.InetAddress;
+import java.nio.file.Path;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.util.ArrayList;
@@ -107,15 +108,13 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
     }
 
     @Override
-    public Docker.CreateContainerCommand withVolume(String path, String volumePath) {
-        assert path.indexOf(':') == -1;
+    public Docker.CreateContainerCommand withVolume(Path path, Path volumePath) {
         volumeBindSpecs.add(path + ":" + volumePath + ":Z");
         return this;
     }
 
     @Override
-    public Docker.CreateContainerCommand withSharedVolume(String path, String volumePath) {
-        assert path.indexOf(':') == -1;
+    public Docker.CreateContainerCommand withSharedVolume(Path path, Path volumePath) {
         volumeBindSpecs.add(path + ":" + volumePath + ":z");
         return this;
     }
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
index bc2c9514709..1713b4570b8 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
@@ -2,9 +2,11 @@
 package com.yahoo.vespa.hosted.dockerapi;
 
 import java.net.InetAddress;
+import java.nio.file.Path;
 import java.time.Duration;
 import java.util.List;
 import java.util.Optional;
+import java.util.OptionalLong;
 
 /**
  * API to simplify the com.github.dockerjava API for clients,
@@ -26,19 +28,19 @@ public interface Docker {
          * /foo's content visible/readable/writable only inside the container which was last
          * started and on the host. All the other containers will get "Permission denied".
          *
-         * <p>Use {@link #withSharedVolume(String, String)} to mount a given host directory
+         * <p>Use {@link #withSharedVolume(Path, Path)} to mount a given host directory
          * into multiple containers.
          */
-        CreateContainerCommand withVolume(String path, String volumePath);
+        CreateContainerCommand withVolume(Path path, Path volumePath);
 
         /**
          * Mounts a directory on host inside the docker container.
          *
          * <p>The bind mount content will be <b>shared</b> among multiple containers.
          *
-         * @see #withVolume(String, String)
+         * @see #withVolume(Path, Path)
          */
-        CreateContainerCommand withSharedVolume(String path, String volumePath);
+        CreateContainerCommand withSharedVolume(Path path, Path volumePath);
         CreateContainerCommand withNetworkMode(String mode);
         CreateContainerCommand withIpAddress(InetAddress address);
         CreateContainerCommand withUlimit(String name, int softLimit, int hardLimit);
@@ -84,34 +86,12 @@ public interface Docker {
     boolean deleteUnusedDockerImages(List<DockerImage> excludes, Duration minImageAgeToDelete);
 
     /**
-     * Execute a command in docker container as $VESPA_USER. Will block until the command is finished.
-     *
-     * @param containerName The name of the container
-     * @param command The command with arguments to run.
-     *
-     * @return exitcodes, stdout and stderr in the ProcessResult
-     */
-    ProcessResult executeInContainer(ContainerName containerName, String... command);
-
-    /**
-     * Execute a command in docker container as "root". Will block until the command is finished.
-     *
-     * @param containerName The name of the container
-     * @param command The command with arguments to run.
-     *
-     * @return exitcodes, stdout and stderr in the ProcessResult
-     */
-    ProcessResult executeInContainerAsRoot(ContainerName containerName, String... command);
-
-    /**
-     * Execute a command in docker container as "root"
-     * The timeout will not kill the process spawned.
-     *
      * @param containerName The name of the container
+     * @param user can be "username", "username:group", "uid" or "uid:gid"
      * @param timeoutSeconds Timeout for the process to finish in seconds or without timeout if empty
-     * @param command The command with arguments to run.
+     * @param command The command with arguments to run
      *
      * @return exitcodes, stdout and stderr in the ProcessResult
      */
-    ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... command);
+    ProcessResult executeInContainerAsUser(ContainerName containerName, String user, OptionalLong timeoutSeconds, String... command);
 }
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
index 22c865f6d9e..2ac68be2b2e 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
@@ -34,6 +34,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.OptionalLong;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -41,8 +42,6 @@ import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static com.yahoo.vespa.defaults.Defaults.getDefaults;
-
 public class DockerImpl implements Docker {
     private static final Logger logger = Logger.getLogger(DockerImpl.class.getName());
 
@@ -120,25 +119,9 @@ public class DockerImpl implements Docker {
         return new CreateContainerCommandImpl(dockerClient, image, containerResources, name, hostName);
     }
 
-    @Override
-    public ProcessResult executeInContainer(ContainerName containerName, String... args) {
-        return executeInContainerAsUser(containerName, getDefaults().vespaUser(), Optional.empty(), args);
-    }
-
-    @Override
-    public ProcessResult executeInContainerAsRoot(ContainerName containerName, String... args) {
-        return executeInContainerAsUser(containerName, "root", Optional.empty(), args);
-    }
 
     @Override
-    public ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... args) {
-        return executeInContainerAsUser(containerName, "root", Optional.of(timeoutSeconds), args);
-    }
-
-    /**
-     * Execute command in container as user, "user" can be "username", "username:group", "uid" or "uid:gid"
-     */
-    private ProcessResult executeInContainerAsUser(ContainerName containerName, String user, Optional<Long> timeoutSeconds, String... command) {
+    public ProcessResult executeInContainerAsUser(ContainerName containerName, String user, OptionalLong timeoutSeconds, String... command) {
         try {
             ExecCreateCmdResponse response = execCreateCmd(containerName, user, command);
 
@@ -148,7 +131,7 @@ public class DockerImpl implements Docker {
                     .exec(new ExecStartResultCallback(output, errors));
 
             if (timeoutSeconds.isPresent()) {
-                if (!callback.awaitCompletion(timeoutSeconds.get(), TimeUnit.SECONDS))
+                if (!callback.awaitCompletion(timeoutSeconds.getAsLong(), TimeUnit.SECONDS))
                     throw new DockerExecTimeoutException(String.format(
                             "Command '%s' did not finish within %s seconds.", command[0], timeoutSeconds));
             } else {
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java
index 5ce8c6b093c..dc041b159b5 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImplTest.java
@@ -6,6 +6,7 @@ import org.junit.Test;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.nio.file.Paths;
 import java.util.Optional;
 import java.util.stream.Stream;
 
@@ -27,7 +28,7 @@ public class CreateContainerCommandImplTest {
                 .withUlimit("nproc", 10, 20)
                 .withEnvironment("env1", "val1")
                 .withEnvironment("env2", "val2")
-                .withVolume("vol1", "/host/vol1")
+                .withVolume(Paths.get("vol1"), Paths.get("/host/vol1"))
                 .withAddCapability("SYS_PTRACE")
                 .withAddCapability("SYS_ADMIN")
                 .withDropCapability("NET_ADMIN")
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java
index f1a8d4ef65e..8d9ea61c1d9 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java
@@ -19,6 +19,8 @@ import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Matchers;
 
+import java.util.OptionalLong;
+
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
@@ -66,7 +68,8 @@ public class DockerImplTest {
         when(state.isRunning()).thenReturn(false);
         when(state.getExitCode()).thenReturn(exitCode);
 
-        final ProcessResult result = docker.executeInContainer(new ContainerName(containerId), command);
+        final ProcessResult result = docker.executeInContainerAsUser(
+                new ContainerName(containerId), "root", OptionalLong.empty(), command);
         assertThat(result.getExitStatus(), is(exitCode));
     }