Re-introduce exec in docker container with timeout method

5 files changed, 75 insertions, 18 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
index 80c8f148cbf..aa7285ec17c 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
@@ -25,9 +25,11 @@ public interface DockerOperations {
 
     void scheduleDownloadOfImage(ContainerName containerName, ContainerNodeSpec nodeSpec, Runnable callback);
 
-    ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String[] command);
+    ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String... command);
 
-    void executeCommandInNetworkNamespace(ContainerName containerName, String[] command);
+    ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... command);
+
+    void executeCommandInNetworkNamespace(ContainerName containerName, String... command);
 
     void resumeNode(ContainerName containerName);
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index 0ae807f7f04..7dc29aaa200 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -274,10 +274,10 @@ public class DockerOperationsImpl implements DockerOperations {
         });
     }
 
-    ProcessResult executeCommandInContainer(ContainerName containerName, String[] command) {
+    ProcessResult executeCommandInContainer(ContainerName containerName, String... command) {
         ProcessResult result = docker.executeInContainerAsRoot(containerName, command);
 
-        if (! result.isSuccess()) {
+        if (!result.isSuccess()) {
             throw new RuntimeException("Container " + containerName.asString() +
                     ": command " + Arrays.toString(command) + " failed: " + result);
         }
@@ -285,12 +285,17 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
-    public ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String[] command) {
+    public ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... command) {
+        return docker.executeInContainerAsRoot(containerName, timeoutSeconds, command);
+    }
+
+    @Override
+    public ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String... command) {
         return docker.executeInContainerAsRoot(containerName, command);
     }
 
     @Override
-    public void executeCommandInNetworkNamespace(ContainerName containerName, String[] command) {
+    public void executeCommandInNetworkNamespace(ContainerName containerName, String... command) {
         final PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
         final Integer containerPid = docker.getContainer(containerName)
                 .filter(container -> container.state.isRunning())
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 2ff10560fc1..8e7892652e3 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
+import com.yahoo.vespa.hosted.dockerapi.DockerExecTimeoutException;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
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 bc3ce6ce5bb..053ad921fef 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
@@ -159,6 +159,14 @@ public class DockerMock implements Docker {
         return new ProcessResult(0, null, "");
     }
 
+    @Override
+    public ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... args) {
+        synchronized (monitor) {
+            callOrderVerifier.add("executeInContainerAsRoot with " + containerName + ", args: " + Arrays.toString(args));
+        }
+        return new ProcessResult(0, null, "");
+    }
+
 
     public static class StartContainerCommandMock implements CreateContainerCommand {
         @Override
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
index eea72619032..f3cc352a3b8 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
@@ -16,7 +16,6 @@ import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
-import static org.mockito.AdditionalMatchers.aryEq;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doThrow;
@@ -102,7 +101,9 @@ public class AclMaintainerTest {
 
         verify(dockerOperations).executeCommandInNetworkNamespace(
                 eq(container.name),
-                aryEq(new String[]{"ip6tables", "-P", "INPUT", "ACCEPT"})
+                eq("ip6tables"),
+                eq("-F"),
+                eq("INPUT")
         );
     }
 
@@ -114,40 +115,80 @@ public class AclMaintainerTest {
                                    VerificationMode verificationMode) {
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-F", "INPUT"})
+                eq("ip6tables"),
+                eq("-F"),
+                eq("INPUT")
         );
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-P", "INPUT", "DROP"})
+                eq("ip6tables"),
+                eq("-P"),
+                eq("INPUT"),
+                eq("DROP")
         );
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-P", "FORWARD", "DROP"})
+                eq("ip6tables"),
+                eq("-P"),
+                eq("FORWARD"),
+                eq("DROP")
         );
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-P", "OUTPUT", "ACCEPT"})
+                eq("ip6tables"),
+                eq("-P"),
+                eq("OUTPUT"),
+                eq("ACCEPT")
         );
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-A", "INPUT", "-m", "state", "--state", "RELATED,ESTABLISHED", "-j",
-                        "ACCEPT"})
+                eq("ip6tables"),
+                eq("-A"),
+                eq("INPUT"),
+                eq("-m"),
+                eq("state"),
+                eq("--state"),
+                eq("RELATED,ESTABLISHED"),
+                eq("-j"),
+                eq("ACCEPT")
         );
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-A", "INPUT", "-i", "lo", "-j", "ACCEPT"})
+                eq("ip6tables"),
+                eq("-A"),
+                eq("INPUT"),
+                eq("-i"),
+                eq("lo"),
+                eq("-j"),
+                eq("ACCEPT")
         );
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-A", "INPUT", "-p", "ipv6-icmp", "-j", "ACCEPT"})
+                eq("ip6tables"),
+                eq("-A"),
+                eq("INPUT"),
+                eq("-p"),
+                eq("ipv6-icmp"),
+                eq("-j"),
+                eq("ACCEPT")
         );
         containerAclSpecs.forEach(aclSpec -> verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-A", "INPUT", "-s", aclSpec.ipAddress() + "/128", "-j", "ACCEPT"})
+                eq("ip6tables"),
+                eq("-A"),
+                eq("INPUT"),
+                eq("-s"),
+                eq(aclSpec.ipAddress() + "/128"),
+                eq("-j"),
+                eq("ACCEPT")
         ));
         verify(dockerOperations, verificationMode).executeCommandInNetworkNamespace(
                 eq(containerName),
-                aryEq(new String[]{"ip6tables", "-A", "INPUT", "-j", "REJECT"})
+                eq("ip6tables"),
+                eq("-A"),
+                eq("INPUT"),
+                eq("-j"),
+                eq("REJECT")
         );
     }