Docker always returns pid

7 files changed, 23 insertions, 27 deletions

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 f6cfbe938c0..be9f8174982 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
@@ -282,7 +282,8 @@ public class DockerOperationsImpl implements DockerOperations {
     public void executeCommandInNetworkNamespace(ContainerName containerName, String[] command) {
         final PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
         final Integer containerPid = getContainer(containerName.asString())
-                .flatMap(container -> container.pid)
+                .filter(container -> container.isRunning)
+                .map(container -> container.pid)
                 .orElseThrow(() -> new RuntimeException("PID not found for container: " + containerName.asString()));
 
         final List<String> wrappedCommand = new LinkedList<>();
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
index 836a68eb49c..f716d4157f2 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
@@ -96,11 +96,11 @@ public class AclMaintainer implements Runnable {
                 // Container belongs to this Docker host, but is currently unallocated
                 continue;
             }
-            if (!container.get().pid.isPresent()) {
+            if (!container.get().isRunning) {
                 log.info(String.format("PID for container %s not found (not running?)", container.get().name.asString()));
                 continue;
             }
-            applyAcl(container.get().name, new Acl(container.get().pid.get(), entry.getValue()));
+            applyAcl(container.get().name, new Acl(container.get().pid, entry.getValue()));
         }
     }
 
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 507fd218ba7..9a643bd2ce3 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
@@ -125,7 +125,7 @@ public class DockerOperationsImplTest {
 
     private Container makeContainer(String hostname, int pid) {
         final Container container = new Container(hostname, new DockerImage("mock"),
-                new ContainerName(hostname), true, Optional.of(pid));
+                new ContainerName(hostname), pid);
         when(dockerOperations.getContainer(eq(hostname))).thenReturn(Optional.of(container));
         return container;
     }
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 2be286eca7b..a10e9bdfe47 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
@@ -38,7 +38,7 @@ public class DockerMock implements Docker {
         synchronized (monitor) {
             callOrderVerifier.add("createContainerCommand with " + dockerImage +
                     ", HostName: " + hostName + ", " + containerName);
-            containers.add(new Container(hostName, dockerImage, containerName, true));
+            containers.add(new Container(hostName, dockerImage, containerName, 2));
         }
 
         return new StartContainerCommandMock();
@@ -64,11 +64,6 @@ public class DockerMock implements Docker {
     }
 
     @Override
-    public Optional<ContainerInfo> inspectContainer(ContainerName containerName) {
-        return Optional.of(() -> Optional.of(2));
-    }
-
-    @Override
     public Optional<ContainerStats> getContainerStats(ContainerName containerName) {
         return Optional.empty();
     }
@@ -86,7 +81,7 @@ public class DockerMock implements Docker {
             callOrderVerifier.add("stopContainer with " + containerName);
             containers = containers.stream()
                     .map(container -> container.name.equals(containerName) ?
-                            new Container(container.hostname, container.image, container.name, false) : container)
+                            new Container(container.hostname, container.image, container.name, 0) : container)
                     .collect(Collectors.toList());
         }
     }
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 64fa757eb5d..063789dc928 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
@@ -63,7 +63,7 @@ public class AclMaintainerTest {
 
     @Test
     public void reconfigures_acl_when_container_pid_changes() {
-        Container container = makeContainer("container-1", true, Optional.of(42));
+        Container container = makeContainer("container-1", 42);
         List<ContainerAclSpec> aclSpecs = makeAclSpecs(3, container.name);
         nodeRepository.addContainerAclSpecs(NODE_ADMIN_HOSTNAME, aclSpecs);
 
@@ -71,7 +71,7 @@ public class AclMaintainerTest {
         assertAclsApplied(container.name, aclSpecs);
 
         // Container is restarted and PID changes
-        makeContainer(container.name.asString(), true, Optional.of(43));
+        makeContainer(container.name.asString(), 43);
         aclMaintainer.run();
 
         assertAclsApplied(container.name, aclSpecs, times(2));
@@ -79,7 +79,7 @@ public class AclMaintainerTest {
 
     @Test
     public void does_not_configure_acl_for_stopped_container() {
-        Container stoppedContainer = makeContainer("container-1", false, Optional.empty());
+        Container stoppedContainer = makeContainer("container-1", 0);
         List<ContainerAclSpec> aclSpecs = makeAclSpecs(1, stoppedContainer.name);
         nodeRepository.addContainerAclSpecs(NODE_ADMIN_HOSTNAME, aclSpecs);
         aclMaintainer.run();
@@ -142,12 +142,12 @@ public class AclMaintainerTest {
     }
 
     private Container makeContainer(String hostname) {
-        return makeContainer(hostname, true, Optional.of(42));
+        return makeContainer(hostname, 42);
     }
 
-    private Container makeContainer(String hostname, boolean running, Optional<Integer> pid) {
+    private Container makeContainer(String hostname, int pid) {
         final Container container = new Container(hostname, new DockerImage("mock"),
-                new ContainerName(hostname), running, pid);
+                new ContainerName(hostname), pid);
         when(dockerOperations.getContainer(eq(hostname))).thenReturn(Optional.of(container));
         return container;
     }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java
index a7c5703f4cc..19afa801643 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java
@@ -60,8 +60,7 @@ public class NodeAdminImplTest {
         final String hostName = "host";
         final DockerImage dockerImage = new DockerImage("image");
         final ContainerName containerName = new ContainerName("container");
-        final boolean isRunning = true;
-        final Container existingContainer = new Container(hostName, dockerImage, containerName, isRunning);
+        final Container existingContainer = new Container(hostName, dockerImage, containerName, 5);
         final ContainerNodeSpec nodeSpec = new ContainerNodeSpec.Builder()
                 .hostname(hostName)
                 .wantedDockerImage(dockerImage)
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index 8a8206f8ccb..84648f4561d 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -92,7 +92,7 @@ public class NodeAgentImplTest {
                 .build();
 
         Docker.ContainerStats containerStats = new ContainerStatsImpl(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>());
-        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, true)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, 1)));
         when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(false);
         when(dockerOperations.startContainerIfNeeded(eq(nodeSpec))).thenReturn(false);
@@ -190,7 +190,7 @@ public class NodeAgentImplTest {
 
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(true);
         when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec));
-        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, oldDockerImage, containerName, true)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, oldDockerImage, containerName, 1)));
 
         nodeAgent.tick();
 
@@ -255,7 +255,7 @@ public class NodeAgentImplTest {
                 .build();
 
         when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec));
-        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, true)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, 1)));
 
         nodeAgent.vespaVersion = nodeSpec.vespaVersion;
 
@@ -292,7 +292,7 @@ public class NodeAgentImplTest {
                 .build();
 
         when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec));
-        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, true)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, 1)));
 
         nodeAgent.vespaVersion = nodeSpec.vespaVersion;
 
@@ -330,7 +330,7 @@ public class NodeAgentImplTest {
                 .build();
 
         when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec));
-        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, true)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, dockerImage, containerName, 1)));
 
         nodeAgent.vespaVersion = nodeSpec.vespaVersion;
 
@@ -370,12 +370,13 @@ public class NodeAgentImplTest {
 
         final ContainerNodeSpec nodeSpec = nodeSpecBuilder.build();
 
+        boolean shouldBeRunning = nodeState != Node.State.dirty && nodeState != Node.State.provisioned;
         when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec));
         when(dockerOperations.getContainer(eq(hostName))).thenReturn(
                 Optional.of(new Container(hostName,
                                           dockerImage,
                                           containerName,
-                                          nodeState != Node.State.dirty && nodeState != Node.State.provisioned)));
+                                          shouldBeRunning ? 1 : 0)));
 
         nodeAgent.tick();
 
@@ -433,7 +434,7 @@ public class NodeAgentImplTest {
 
         Docker.ContainerStats containerStats = new ContainerStatsImpl(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>());
         when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
-        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, wantedDockerImage, containerName, true)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, wantedDockerImage, containerName, 1)));
         when(nodeRepository.getContainerNodeSpec(eq(hostName))).thenReturn(Optional.of(nodeSpec));
         when(dockerOperations.shouldScheduleDownloadOfImage(eq(wantedDockerImage))).thenReturn(false);
 
@@ -481,7 +482,7 @@ public class NodeAgentImplTest {
         when(dockerOperations.getContainerStats(eq(containerName))).thenReturn(Optional.of(stats));
 
         when(dockerOperations.getContainer(eq(hostName)))
-                .thenReturn(Optional.of(new Container(hostName, new DockerImage("wantedDockerImage"), containerName, true)));
+                .thenReturn(Optional.of(new Container(hostName, new DockerImage("wantedDockerImage"), containerName, 1)));
 
         nodeAgent.vespaVersion = Optional.of("1.2.3");
         ContainerNodeSpec.Owner owner = new ContainerNodeSpec.Owner("tester", "testapp", "testinstance");