Use docker.state.status to determine container state

13 files changed, 60 insertions, 36 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java
index aa35ee9bc06..94e827b8a49 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Container.java
@@ -10,19 +10,20 @@ public class Container {
     public final String hostname;
     public final DockerImage image;
     public final ContainerName name;
+    public final State state;
     public final int pid;
-    public final boolean isRunning;
 
     public Container(
             final String hostname,
             final DockerImage image,
             final ContainerName containerName,
+            final State state,
             final int pid) {
         this.hostname = hostname;
         this.image = image;
         this.name = containerName;
+        this.state = state;
         this.pid = pid;
-        this.isRunning = pid != 0;
     }
 
     @Override
@@ -48,8 +49,16 @@ public class Container {
                 + " hostname=" + hostname
                 + " image=" + image
                 + " name=" + name
-                + " isRunning=" + isRunning
+                + " state=" + state
                 + " pid=" + pid
                 + "}";
     }
+
+    public enum State {
+        CREATED, RESTARTING, RUNNING, PAUSED, EXITED, DEAD;
+
+        public boolean isRunning() {
+            return this == RUNNING;
+        }
+    }
 }
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 6f51fbd1ad8..00ace9a1958 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
@@ -355,6 +355,7 @@ public class DockerImpl implements Docker {
                                 response.getConfig().getHostName(),
                                 new DockerImage(response.getConfig().getImage()),
                                 new ContainerName(decode(response.getName())),
+                                Container.State.valueOf(response.getState().getStatus().toUpperCase()),
                                 response.getState().getPid()
                         ))
                 .map(Stream::of)
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
index cf24fb7c826..2aac1cff676 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
@@ -13,6 +13,7 @@ import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
@@ -92,17 +93,23 @@ public class DockerTest {
         docker.createContainerCommand(dockerImage, containerName, containerHostname).withManagedBy(MANAGER_NAME).create();
         Optional<Container> container = docker.getContainer(containerHostname);
         assertTrue(container.isPresent());
-        assertFalse(container.get().isRunning);
+        assertEquals(container.get().state, Container.State.CREATED);
 
         docker.startContainer(containerName);
         container = docker.getContainer(containerHostname);
         assertTrue(container.isPresent());
-        assertTrue(container.get().isRunning);
+        assertEquals(container.get().state, Container.State.RUNNING);
 
+        docker.dockerClient.pauseContainerCmd(containerName.asString()).exec();
+        container = docker.getContainer(containerHostname);
+        assertTrue(container.isPresent());
+        assertEquals(container.get().state, Container.State.PAUSED);
+
+        docker.dockerClient.unpauseContainerCmd(containerName.asString()).exec();
         docker.stopContainer(containerName);
         container = docker.getContainer(containerHostname);
         assertTrue(container.isPresent());
-        assertFalse(container.get().isRunning);
+        assertEquals(container.get().state, Container.State.EXITED);
 
         docker.deleteContainer(containerName);
         assertThat(docker.getAllContainersManagedBy(MANAGER_NAME).isEmpty(), is(true));
@@ -151,7 +158,7 @@ public class DockerTest {
 
         // Clean up any non deleted containers from previous tests
         docker.getAllContainersManagedBy(MANAGER_NAME).forEach(container -> {
-            if (container.isRunning) docker.stopContainer(container.name);
+            if (container.state.isRunning()) docker.stopContainer(container.name);
             docker.deleteContainer(container.name);
         });
     }
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java
index f799acd33aa..51928b232a5 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/RunSystemTests.java
@@ -137,7 +137,7 @@ public class RunSystemTests {
 
         Optional<Container> container = docker.getContainer(containerName.asString());
         if (container.isPresent()) {
-            if (container.get().isRunning) return;
+            if (container.get().state.isRunning()) return;
             else docker.deleteContainer(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 be9f8174982..8edfb6c13f6 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
@@ -253,7 +253,7 @@ public class DockerOperationsImpl implements DockerOperations {
     public void removeContainer(final ContainerNodeSpec nodeSpec, final Container existingContainer) {
         PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, nodeSpec.containerName);
         final ContainerName containerName = existingContainer.name;
-        if (existingContainer.isRunning) {
+        if (existingContainer.state.isRunning()) {
             logger.info("Stopping container " + containerName);
             docker.stopContainer(containerName);
         }
@@ -282,7 +282,7 @@ 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())
-                .filter(container -> container.isRunning)
+                .filter(container -> container.state.isRunning())
                 .map(container -> container.pid)
                 .orElseThrow(() -> new RuntimeException("PID not found for container: " + containerName.asString()));
 
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 f716d4157f2..9ca8794ffcf 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,7 +96,7 @@ public class AclMaintainer implements Runnable {
                 // Container belongs to this Docker host, but is currently unallocated
                 continue;
             }
-            if (!container.get().isRunning) {
+            if (!container.get().state.isRunning()) {
                 log.info(String.format("PID for container %s not found (not running?)", container.get().name.asString()));
                 continue;
             }
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 0c3f0f4a139..57a354e5208 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
@@ -161,7 +161,7 @@ public class NodeAgentImpl implements NodeAgent {
 
         // If the container is already running, initialize vespaVersion
         vespaVersion = dockerOperations.getContainer(hostname)
-                .filter(container -> container.isRunning)
+                .filter(container -> container.state.isRunning())
                 .flatMap(container -> dockerOperations.getVespaVersion(container.name));
 
         loopThread = new Thread(this::loop);
@@ -279,7 +279,7 @@ public class NodeAgentImpl implements NodeAgent {
 
     private void restartServices(ContainerNodeSpec nodeSpec, Container existingContainer, Orchestrator orchestrator)
             throws Exception {
-        if (existingContainer.isRunning) {
+        if (existingContainer.state.isRunning()) {
             ContainerName containerName = existingContainer.name;
             if (nodeSpec.nodeState == Node.State.active) {
                 logger.info("Restarting services for " + containerName);
@@ -305,7 +305,7 @@ public class NodeAgentImpl implements NodeAgent {
             return Optional.of("The node is supposed to run a new Docker image: "
                                        + existingContainer + " -> " + nodeSpec.wantedDockerImage.get());
         }
-        if (!existingContainer.isRunning) {
+        if (!existingContainer.state.isRunning()) {
             return Optional.of("Container no longer running");
         }
         return Optional.empty();
@@ -323,7 +323,7 @@ public class NodeAgentImpl implements NodeAgent {
         if (removeReason.isPresent()) {
             logger.info("Will remove container " + existingContainer.get() + ": " + removeReason.get());
 
-            if (existingContainer.get().isRunning) {
+            if (existingContainer.get().state.isRunning()) {
                 final ContainerName containerName = existingContainer.get().name;
                 orchestratorSuspendNode(orchestrator, nodeSpec, logger);
                 dockerOperations.trySuspendNode(containerName);
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 9a643bd2ce3..5eaeb927c53 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
@@ -106,7 +106,7 @@ public class DockerOperationsImplTest {
 
     @Test
     public void runsCommandInNetworkNamespace() {
-        Container container = makeContainer("container-42", 42);
+        Container container = makeContainer("container-42", Container.State.RUNNING, 42);
         List<String> capturedArgs = new ArrayList<>();
         DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker, environment,
                 new MetricReceiverWrapper(MetricReceiver.nullImplementation), capturedArgs::addAll);
@@ -123,9 +123,9 @@ public class DockerOperationsImplTest {
                 "-nvL"), capturedArgs);
     }
 
-    private Container makeContainer(String hostname, int pid) {
+    private Container makeContainer(String hostname, Container.State state, int pid) {
         final Container container = new Container(hostname, new DockerImage("mock"),
-                new ContainerName(hostname), pid);
+                new ContainerName(hostname), state, 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/docker/LocalZoneUtils.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/LocalZoneUtils.java
index 0f1f79d68a1..1153b38d361 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/LocalZoneUtils.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/LocalZoneUtils.java
@@ -57,7 +57,7 @@ public class LocalZoneUtils {
     public static void startConfigServerIfNeeded(Docker docker, Environment environment) throws UnknownHostException {
         Optional<Container> container = docker.getContainer(CONFIG_SERVER_HOSTNAME);
         if (container.isPresent()) {
-            if (container.get().isRunning) return;
+            if (container.get().state.isRunning()) return;
             else docker.deleteContainer(CONFIG_SERVER_CONTAINER_NAME);
         }
 
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 a10e9bdfe47..ac0e24d8c71 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, 2));
+            containers.add(new Container(hostName, dockerImage, containerName, Container.State.RUNNING, 2));
         }
 
         return new StartContainerCommandMock();
@@ -81,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, 0) : container)
+                            new Container(container.hostname, container.image, container.name, Container.State.EXITED, 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 063789dc928..159bae1fbe1 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", 42);
+        Container container = makeContainer("container-1");
         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(), 43);
+        makeContainer(container.name.asString(), Container.State.RUNNING, 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", 0);
+        Container stoppedContainer = makeContainer("container-1", Container.State.EXITED, 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, 42);
+        return makeContainer(hostname, Container.State.RUNNING, 42);
     }
 
-    private Container makeContainer(String hostname, int pid) {
+    private Container makeContainer(String hostname, Container.State state, int pid) {
         final Container container = new Container(hostname, new DockerImage("mock"),
-                new ContainerName(hostname), pid);
+                new ContainerName(hostname), state, 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 19afa801643..17f70ce5efa 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,7 +60,7 @@ public class NodeAdminImplTest {
         final String hostName = "host";
         final DockerImage dockerImage = new DockerImage("image");
         final ContainerName containerName = new ContainerName("container");
-        final Container existingContainer = new Container(hostName, dockerImage, containerName, 5);
+        final Container existingContainer = new Container(hostName, dockerImage, containerName, Container.State.RUNNING, 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 84648f4561d..0781a303a57 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,8 @@ 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, 1)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(
+                Optional.of(new Container(hostName, dockerImage, containerName, Container.State.RUNNING, 1)));
         when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(false);
         when(dockerOperations.startContainerIfNeeded(eq(nodeSpec))).thenReturn(false);
@@ -190,7 +191,8 @@ 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, 1)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(
+                Optional.of(new Container(hostName, oldDockerImage, containerName, Container.State.RUNNING, 1)));
 
         nodeAgent.tick();
 
@@ -255,7 +257,8 @@ 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, 1)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(
+                Optional.of(new Container(hostName, dockerImage, containerName, Container.State.RUNNING, 1)));
 
         nodeAgent.vespaVersion = nodeSpec.vespaVersion;
 
@@ -292,7 +295,8 @@ 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, 1)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(
+                Optional.of(new Container(hostName, dockerImage, containerName, Container.State.RUNNING, 1)));
 
         nodeAgent.vespaVersion = nodeSpec.vespaVersion;
 
@@ -330,7 +334,8 @@ 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, 1)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(
+                Optional.of(new Container(hostName, dockerImage, containerName, Container.State.RUNNING, 1)));
 
         nodeAgent.vespaVersion = nodeSpec.vespaVersion;
 
@@ -376,6 +381,7 @@ public class NodeAgentImplTest {
                 Optional.of(new Container(hostName,
                                           dockerImage,
                                           containerName,
+                                          shouldBeRunning ? Container.State.RUNNING : Container.State.EXITED,
                                           shouldBeRunning ? 1 : 0)));
 
         nodeAgent.tick();
@@ -434,7 +440,8 @@ 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, 1)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(
+                Optional.of(new Container(hostName, wantedDockerImage, containerName, Container.State.RUNNING, 1)));
         when(nodeRepository.getContainerNodeSpec(eq(hostName))).thenReturn(Optional.of(nodeSpec));
         when(dockerOperations.shouldScheduleDownloadOfImage(eq(wantedDockerImage))).thenReturn(false);
 
@@ -481,8 +488,8 @@ public class NodeAgentImplTest {
         final ContainerName containerName = new ContainerName("cont-name");
         when(dockerOperations.getContainerStats(eq(containerName))).thenReturn(Optional.of(stats));
 
-        when(dockerOperations.getContainer(eq(hostName)))
-                .thenReturn(Optional.of(new Container(hostName, new DockerImage("wantedDockerImage"), containerName, 1)));
+        when(dockerOperations.getContainer(eq(hostName))).thenReturn(
+                Optional.of(new Container(hostName, new DockerImage("wantedDockerImage"), containerName, Container.State.RUNNING, 1)));
 
         nodeAgent.vespaVersion = Optional.of("1.2.3");
         ContainerNodeSpec.Owner owner = new ContainerNodeSpec.Owner("tester", "testapp", "testinstance");