Fixed tests

6 files changed, 18 insertions, 62 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 e1e5f955e6f..3f9f0686d82 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
@@ -26,8 +26,6 @@ public interface DockerOperations {
 
     void removeContainer(ContainerNodeSpec nodeSpec, Container existingContainer);
 
-    ProcessResult executeCommandInContainer(ContainerName containerName, String[] command);
-
     ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String[] command);
 
     void executeCommandInNetworkNamespace(ContainerName containerName, String[] command);
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 905f0e436a8..aa4e2b892be 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
@@ -141,10 +141,6 @@ public class DockerOperationsImpl implements DockerOperations {
         return docker.getContainer(hostname);
     }
 
-    String[] programExistsCommand(String programPath) {
-        return new String[]{ "/usr/bin/env", "test", "-x", programPath };
-    }
-
     /**
      * Try to suspend node. Suspending a node means the node should be taken offline,
      * such that maintenance can be done of the node (upgrading, rebooting, etc),
@@ -266,11 +262,10 @@ public class DockerOperationsImpl implements DockerOperations {
         numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
     }
 
-    @Override
-    public ProcessResult executeCommandInContainer(ContainerName containerName, String[] command) {
+    ProcessResult executeCommandInContainer(ContainerName containerName, String[] command) {
         ProcessResult result = docker.executeInContainer(containerName, command);
 
-        if (result.isSuccess()) {
+        if (! result.isSuccess()) {
             throw new RuntimeException("Container " + containerName.asString() +
                     ": command " + Arrays.toString(command) + " failed: " + result);
         }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
index 2b68c869008..cf660cb51c9 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
@@ -25,7 +25,6 @@ import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class DockerOperationsImplTest {
@@ -35,64 +34,37 @@ public class DockerOperationsImplTest {
             new MetricReceiverWrapper(MetricReceiver.nullImplementation));
 
     @Test
-    public void absenceOfNodeProgramIsSuccess() throws Exception {
+    public void processResultFromNodeProgramWhenSuccess() throws Exception {
         final ContainerName containerName = new ContainerName("container-name");
+        final ProcessResult actualResult = new ProcessResult(0, "output", "errors");
         final String programPath = "/bin/command";
+        final String[] command = new String[] {programPath, "arg"};
 
-        when(docker.executeInContainer(any(), anyVararg())).thenReturn(new ProcessResult(3, "output", "errors"));
-
-        Optional<ProcessResult> result = dockerOperations.executeOptionalProgramInContainer(
-                containerName,
-                programPath,
-                "arg1",
-                "arg2");
+        when(docker.executeInContainer(any(), anyVararg()))
+                .thenReturn(actualResult); // output from node program
 
-        String[] nodeProgramExistsCommand = dockerOperations.programExistsCommand(programPath);
-        assertThat(nodeProgramExistsCommand.length, is(4));
+        ProcessResult result = dockerOperations.executeCommandInContainer(containerName, command);
 
-        verify(docker, times(1)).executeInContainer(
+        final InOrder inOrder = inOrder(docker);
+        inOrder.verify(docker, times(1)).executeInContainer(
                 eq(containerName),
-                // Mockito fails if we put the array here instead...
-                eq(nodeProgramExistsCommand[0]),
-                eq(nodeProgramExistsCommand[1]),
-                eq(nodeProgramExistsCommand[2]),
-                eq(nodeProgramExistsCommand[3]));
-        assertThat(result.isPresent(), is(false));
+                eq(command[0]),
+                eq(command[1]));
+
+        assertThat(result, is(actualResult));
     }
 
-    @Test
-    public void processResultFromNodeProgramWhenPresent() throws Exception {
+    @Test(expected=RuntimeException.class)
+    public void processResultFromNodeProgramWhenNonZeroExitCode() throws Exception {
         final ContainerName containerName = new ContainerName("container-name");
         final ProcessResult actualResult = new ProcessResult(3, "output", "errors");
         final String programPath = "/bin/command";
         final String[] command = new String[] {programPath, "arg"};
 
         when(docker.executeInContainer(any(), anyVararg()))
-                .thenReturn(new ProcessResult(0, "", "")) // node program exists
                 .thenReturn(actualResult); // output from node program
 
-        Optional<ProcessResult> result = dockerOperations.executeOptionalProgramInContainer(
-                containerName,
-                command);
-
-        String[] nodeProgramExistsCommand = dockerOperations.programExistsCommand(programPath);
-        assertThat(nodeProgramExistsCommand.length, is(4));
-
-        final InOrder inOrder = inOrder(docker);
-        inOrder.verify(docker, times(1)).executeInContainer(
-                eq(containerName),
-                // Mockito fails if we put the array here instead...
-                eq(nodeProgramExistsCommand[0]),
-                eq(nodeProgramExistsCommand[1]),
-                eq(nodeProgramExistsCommand[2]),
-                eq(nodeProgramExistsCommand[3]));
-        inOrder.verify(docker, times(1)).executeInContainer(
-                eq(containerName),
-                eq(command[0]),
-                eq(command[1]));
-
-        assertThat(result.isPresent(), is(true));
-        assertThat(result.get(), is(actualResult));
+        dockerOperations.executeCommandInContainer(containerName, command);
     }
 
     @Test
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java
index 9ad61baf585..a9e811e68e4 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerFailTest.java
@@ -36,7 +36,6 @@ public class DockerFailTest {
             CallOrderVerifier callOrderVerifier = dockerTester.getCallOrderVerifier();
             callOrderVerifier.assertInOrder(
                     "createContainerCommand with DockerImage { imageId=dockerImage }, HostName: hostName, ContainerName { name=container }",
-                    "executeInContainer with ContainerName { name=container }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                     "executeInContainer with ContainerName { name=container }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]");
 
             dockerTester.deleteContainer(containerNodeSpec.containerName);
@@ -44,7 +43,6 @@ public class DockerFailTest {
             callOrderVerifier.assertInOrder(
                     "deleteContainer with ContainerName { name=container }",
                     "createContainerCommand with DockerImage { imageId=dockerImage }, HostName: hostName, ContainerName { name=container }",
-                    "executeInContainer with ContainerName { name=container }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                     "executeInContainer with ContainerName { name=container }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]");
         }
     }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java
index 2f88efd8620..c75cd526baf 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java
@@ -38,18 +38,15 @@ public class MultiDockerTest {
             CallOrderVerifier callOrderVerifier = dockerTester.getCallOrderVerifier();
             callOrderVerifier.assertInOrder(
                     "createContainerCommand with DockerImage { imageId=image1 }, HostName: host1, ContainerName { name=container1 }",
-                    "executeInContainer with ContainerName { name=container1 }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                     "executeInContainer with ContainerName { name=container1 }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]",
 
                     "createContainerCommand with DockerImage { imageId=image2 }, HostName: host2, ContainerName { name=container2 }",
-                    "executeInContainer with ContainerName { name=container2 }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                     "executeInContainer with ContainerName { name=container2 }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]",
 
                     "stopContainer with ContainerName { name=container2 }",
                     "deleteContainer with ContainerName { name=container2 }",
 
                     "createContainerCommand with DockerImage { imageId=image1 }, HostName: host3, ContainerName { name=container3 }",
-                    "executeInContainer with ContainerName { name=container3 }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                     "executeInContainer with ContainerName { name=container3 }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]");
 
             callOrderVerifier.assertInOrderWithAssertMessage("Maintainer did not receive call to delete application storage",
@@ -85,7 +82,6 @@ public class MultiDockerTest {
 
         tester.getCallOrderVerifier().assertInOrder(
                 "createContainerCommand with " + dockerImage + ", HostName: " + hostName + ", " + containerName,
-                "executeInContainer with " + containerName + ", args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                 "executeInContainer with " + containerName + ", args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]");
 
         return containerNodeSpec;
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeStateTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeStateTest.java
index 53d3422e8d5..f4d10abe3a4 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeStateTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeStateTest.java
@@ -37,7 +37,6 @@ public class NodeStateTest {
 
         tester.getCallOrderVerifier().assertInOrder(
                 "createContainerCommand with DockerImage { imageId=dockerImage }, HostName: host1, ContainerName { name=container }",
-                "executeInContainer with ContainerName { name=container }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                 "executeInContainer with ContainerName { name=container }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]");
     }
 
@@ -58,8 +57,7 @@ public class NodeStateTest {
             }
 
             dockerTester.getCallOrderVerifier()
-                        .assertInOrder("executeInContainer with ContainerName { name=container }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
-                                       "executeInContainer with ContainerName { name=container }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", stop]",
+                        .assertInOrder("executeInContainer with ContainerName { name=container }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", stop]",
                                        "stopContainer with ContainerName { name=container }",
                                        "deleteContainer with ContainerName { name=container }");
         }
@@ -95,7 +93,6 @@ public class NodeStateTest {
             callOrderVerifier.assertInOrderWithAssertMessage("Node not started again after being put to active state",
                                                              "deleteContainer with ContainerName { name=container }",
                                                              "createContainerCommand with DockerImage { imageId=newDockerImage }, HostName: host1, ContainerName { name=container }",
-                                                             "executeInContainer with ContainerName { name=container }, args: [/usr/bin/env, test, -x, " + DockerOperationsImpl.NODE_PROGRAM + "]",
                                                              "executeInContainer with ContainerName { name=container }, args: [" + DockerOperationsImpl.NODE_PROGRAM + ", resume]");
         }
     }