Merge pull request #5227 from vespa-engine/freva/run-containers-in-privileged-from-host-admin

Run containers in privileged from host-admin

7 files changed, 108 insertions, 30 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 485de99082b..260e2da7c59 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
@@ -21,6 +21,7 @@ import java.util.Random;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
+import java.util.stream.Stream;
 
 class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
     private final DockerClient docker;
@@ -32,13 +33,14 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
     private final List<String> environmentAssignments = new ArrayList<>();
     private final List<String> volumeBindSpecs = new ArrayList<>();
     private final List<Ulimit> ulimits = new ArrayList<>();
+    private final Set<Capability> addCapabilities = new HashSet<>();
+    private final Set<Capability> dropCapabilities = new HashSet<>();
 
     private Optional<String> networkMode = Optional.empty();
     private Optional<String> ipv4Address = Optional.empty();
     private Optional<String> ipv6Address = Optional.empty();
     private Optional<String[]> entrypoint = Optional.empty();
-    private Set<Capability> addCapabilities = new HashSet<>();
-    private Set<Capability> dropCapabilities = new HashSet<>();
+    private boolean privileged = false;
 
     CreateContainerCommandImpl(DockerClient docker,
                                DockerImage dockerImage,
@@ -60,8 +62,7 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
     }
 
     public Docker.CreateContainerCommand withManagedBy(String manager) {
-        labels.put(DockerImpl.LABEL_NAME_MANAGEDBY, manager);
-        return this;
+        return withLabel(DockerImpl.LABEL_NAME_MANAGEDBY, manager);
     }
 
     @Override
@@ -77,6 +78,12 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
     }
 
     @Override
+    public Docker.CreateContainerCommand withPrivileged(boolean privileged) {
+        this.privileged = privileged;
+        return this;
+    }
+
+    @Override
     public Docker.CreateContainerCommand withUlimit(String name, int softLimit, int hardLimit) {
         ulimits.add(new Ulimit(name, softLimit, hardLimit));
         return this;
@@ -84,6 +91,7 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
 
     @Override
     public Docker.CreateContainerCommand withEntrypoint(String... entrypoint) {
+        if (entrypoint.length < 1) throw new IllegalArgumentException("Entrypoint must contain at least 1 element");
         this.entrypoint = Optional.of(entrypoint);
         return this;
     }
@@ -142,7 +150,8 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
                 .withBinds(volumeBinds)
                 .withUlimits(ulimits)
                 .withCapAdd(new ArrayList<>(addCapabilities))
-                .withCapDrop(new ArrayList<>(dropCapabilities));
+                .withCapDrop(new ArrayList<>(dropCapabilities))
+                .withPrivileged(privileged);
 
         networkMode
                 .filter(mode -> ! mode.toLowerCase().equals("host"))
@@ -156,15 +165,19 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
         return containerCmd;
     }
 
-    /** Maps ("--env", {"A", "B", "C"}) to "--env A --env B --env C ". */
+    /** Maps ("--env", {"A", "B", "C"}) to "--env A --env B --env C" */
     private String toRepeatedOption(String option, List<String> optionValues) {
-        StringBuilder builder = new StringBuilder();
-        optionValues.forEach(optionValue -> builder.append(option).append(" ").append(optionValue).append(" "));
-        return builder.toString();
+        return optionValues.stream()
+                .map(optionValue -> option + " " + optionValue)
+                .collect(Collectors.joining(" "));
+    }
+
+    private String toOptionalOption(String option, Optional<String> value) {
+        return value.map(o -> option + " " + o).orElse("");
     }
 
-    private String toOptionalOption(String option, Optional<?> value) {
-        return value.isPresent() ? option + " " + value.get() + " " : "";
+    private String toFlagOption(String option, boolean value) {
+        return value ? option : "";
     }
 
     /** Make toString() print the equivalent arguments to 'docker run' */
@@ -175,24 +188,31 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
         List<String> ulimitList = ulimits.stream()
                 .map(ulimit -> ulimit.getName() + "=" + ulimit.getSoft() + ":" + ulimit.getHard())
                 .collect(Collectors.toList());
-        List<String> addCapabilitiesList = addCapabilities.stream().map(Enum<Capability>::toString).collect(Collectors.toList());
-        List<String> dropCapabilitiesList = dropCapabilities.stream().map(Enum<Capability>::toString).collect(Collectors.toList());
-
-        return "--name " + containerName.asString() + " "
-                + "--hostname " + hostName + " "
-                + "--cpu-shares " + containerResources.cpuShares + " "
-                + "--memory " + containerResources.memoryBytes + " "
-                + toRepeatedOption("--label", labelList)
-                + toRepeatedOption("--ulimit", ulimitList)
-                + toRepeatedOption("--env", environmentAssignments)
-                + toRepeatedOption("--volume", volumeBindSpecs)
-                + toRepeatedOption("--cap-add", addCapabilitiesList)
-                + toRepeatedOption("--cap-drop", dropCapabilitiesList)
-                + toOptionalOption("--net", networkMode)
-                + toOptionalOption("--ip", ipv4Address)
-                + toOptionalOption("--ip6", ipv6Address)
-                + toOptionalOption("--entrypoint", entrypoint)
-                + dockerImage.asString();
+        List<String> addCapabilitiesList = addCapabilities.stream().map(Enum<Capability>::toString).sorted().collect(Collectors.toList());
+        List<String> dropCapabilitiesList = dropCapabilities.stream().map(Enum<Capability>::toString).sorted().collect(Collectors.toList());
+        Optional<String> entrypointExecuteable = entrypoint.map(args -> args[0]);
+        String entrypointArgs = entrypoint.map(Stream::of).orElseGet(Stream::empty)
+                .skip(1)
+                .collect(Collectors.joining(" "));
+
+        return String.join(" ",
+                "--name " + containerName.asString(),
+                "--hostname " + hostName,
+                "--cpu-shares " + containerResources.cpuShares,
+                "--memory " + containerResources.memoryBytes,
+                toRepeatedOption("--label", labelList),
+                toRepeatedOption("--ulimit", ulimitList),
+                toRepeatedOption("--env", environmentAssignments),
+                toRepeatedOption("--volume", volumeBindSpecs),
+                toRepeatedOption("--cap-add", addCapabilitiesList),
+                toRepeatedOption("--cap-drop", dropCapabilitiesList),
+                toOptionalOption("--net", networkMode),
+                toOptionalOption("--ip", ipv4Address),
+                toOptionalOption("--ip6", ipv6Address),
+                toOptionalOption("--entrypoint", entrypointExecuteable),
+                toFlagOption("--privileged", privileged),
+                dockerImage.asString(),
+                entrypointArgs);
     }
 
     /**
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 2039d0adfc9..36fc1446bea 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
@@ -27,6 +27,7 @@ public interface Docker {
         CreateContainerCommand withManagedBy(String manager);
         CreateContainerCommand withAddCapability(String capabilityName);
         CreateContainerCommand withDropCapability(String capabilityName);
+        CreateContainerCommand withPrivileged(boolean privileged);
 
         void create();
     }
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 f6588512e2d..805b1e69d45 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
@@ -200,7 +200,8 @@ public class DockerImpl implements Docker {
     @Override
     public CreateContainerCommand createContainerCommand(DockerImage image, ContainerResources containerResources,
                                                          ContainerName name, String hostName) {
-        return new CreateContainerCommandImpl(dockerClient, image, containerResources, name, hostName);
+        return new CreateContainerCommandImpl(dockerClient, image, containerResources, name, hostName)
+                .withPrivileged(config.runContainersInPrivileged());
     }
 
     @Override
diff --git a/docker-api/src/main/resources/configdefinitions/docker.def b/docker-api/src/main/resources/configdefinitions/docker.def
index 83fee05dff6..7be8d85e0a9 100644
--- a/docker-api/src/main/resources/configdefinitions/docker.def
+++ b/docker-api/src/main/resources/configdefinitions/docker.def
@@ -13,3 +13,5 @@ isRunningLocally bool default = false
 imageGCMinTimeToLiveMinutes int default = 45
 
 networkNATed bool default = false
+
+runContainersInPrivileged bool default = false
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 aa455cfc0f2..0d8701ac43c 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
@@ -4,6 +4,8 @@ package com.yahoo.vespa.hosted.dockerapi;
 
 import org.junit.Test;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.Optional;
 import java.util.stream.Stream;
 
@@ -12,6 +14,52 @@ import static org.junit.Assert.assertEquals;
 public class CreateContainerCommandImplTest {
 
     @Test
+    public void testToString() throws UnknownHostException {
+        DockerImage dockerImage = new DockerImage("docker.registry.domain.tld/my/image:1.2.3");
+        ContainerResources containerResources = new ContainerResources(100, 1024);
+        String hostname = "docker-1.region.domain.tld";
+        ContainerName containerName = ContainerName.fromHostname(hostname);
+
+        Docker.CreateContainerCommand createContainerCommand = new CreateContainerCommandImpl(
+                null, dockerImage, containerResources, containerName, hostname)
+                .withLabel("my-label", "test-label")
+                .withUlimit("nofile", 1, 2)
+                .withUlimit("nproc", 10, 20)
+                .withEnvironment("env1", "val1")
+                .withEnvironment("env2", "val2")
+                .withVolume("vol1", "/host/vol1")
+                .withAddCapability("SYS_PTRACE")
+                .withAddCapability("SYS_ADMIN")
+                .withDropCapability("NET_ADMIN")
+                .withNetworkMode("bridge")
+                .withIpAddress(InetAddress.getByName("10.0.0.1"))
+                .withIpAddress(InetAddress.getByName("::1"))
+                .withEntrypoint("/path/to/program", "arg1", "arg2")
+                .withPrivileged(true);
+
+        assertEquals("--name docker-1 " +
+                "--hostname docker-1.region.domain.tld " +
+                "--cpu-shares 100 " +
+                "--memory 1024 " +
+                "--label my-label=test-label " +
+                "--ulimit nofile=1:2 " +
+                "--ulimit nproc=10:20 " +
+                "--env env1=val1 " +
+                "--env env2=val2 " +
+                "--volume vol1:/host/vol1 " +
+                "--cap-add SYS_ADMIN " +
+                "--cap-add SYS_PTRACE " +
+                "--cap-drop NET_ADMIN " +
+                "--net bridge " +
+                "--ip 10.0.0.1 " +
+                "--ip6 0:0:0:0:0:0:0:1 " +
+                "--entrypoint /path/to/program " +
+                "--privileged docker.registry.domain.tld/my/image:1.2.3 " +
+                "arg1 " +
+                "arg2", createContainerCommand.toString());
+    }
+
+    @Test
     public void generateMacAddressTest() {
         String[][] addresses = {
                 {"test123.host.yahoo.com",  null,       "abcd:1234::1", "ee:ae:a9:de:ad:c2"},
diff --git a/node-admin/src/main/application/services.xml b/node-admin/src/main/application/services.xml
index f2f9e46c5b8..afe2888c5ef 100644
--- a/node-admin/src/main/application/services.xml
+++ b/node-admin/src/main/application/services.xml
@@ -13,6 +13,7 @@
 
     <config name="vespa.hosted.dockerapi.docker">
       <uri>unix:///var/run/docker.sock</uri>
+      <runContainersInPrivileged>true</runContainersInPrivileged>
     </config>
 
     <preprocess:include file="variant.xml" required="false"/>
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
index 6c9df440826..c183a7ba306 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
@@ -221,6 +221,11 @@ public class DockerMock implements Docker {
         }
 
         @Override
+        public CreateContainerCommand withPrivileged(boolean privileged) {
+            return this;
+        }
+
+        @Override
         public void create() {
 
         }