Merge pull request #6998 from vespa-engine/freva/clean-up-docker-api

Clean up docker-api

15 files changed, 148 insertions, 145 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
index 4c538d6a194..346223d0e7e 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java
@@ -7,8 +7,8 @@ package com.yahoo.vespa.hosted.dockerapi;
 public class ContainerResources {
     public static final ContainerResources UNLIMITED = ContainerResources.from(0, 0);
 
-    public final int cpuShares;
-    public final long memoryBytes;
+    private final int cpuShares;
+    private final long memoryBytes;
 
     ContainerResources(int cpuShares, long memoryBytes) {
         this.cpuShares = cpuShares;
@@ -21,6 +21,14 @@ public class ContainerResources {
                 (long) ((1L << 30) * memoryGb));
     }
 
+    public int cpuShares() {
+        return cpuShares;
+    }
+
+    public long memoryBytes() {
+        return memoryBytes;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerStatsImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerStats.java
index a56c1e41a51..738a65bc08b 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerStatsImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerStats.java
@@ -10,14 +10,14 @@ import java.util.Optional;
  *
  * @author freva
  */
-public class ContainerStatsImpl implements Docker.ContainerStats {
+public class ContainerStats {
     private final Map<String, Object> networks;
     private final Map<String, Object> cpuStats;
     private final Map<String, Object> memoryStats;
     private final Map<String, Object> blkioStats;
 
-    public ContainerStatsImpl(Map<String, Object> networks, Map<String, Object> cpuStats,
-                              Map<String, Object> memoryStats, Map<String, Object> blkioStats) {
+    public ContainerStats(Map<String, Object> networks, Map<String, Object> cpuStats,
+                          Map<String, Object> memoryStats, Map<String, Object> blkioStats) {
         // Network stats are null when container uses host network
         this.networks = Optional.ofNullable(networks).orElse(Collections.emptyMap());
         this.cpuStats = cpuStats;
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 d95f7b7b8e1..cf168df4634 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
@@ -6,6 +6,7 @@ import com.github.dockerjava.api.command.CreateContainerCmd;
 import com.github.dockerjava.api.model.Bind;
 import com.github.dockerjava.api.model.Capability;
 import com.github.dockerjava.api.model.Ulimit;
+import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
 
 import java.net.Inet6Address;
 import java.net.InetAddress;
@@ -24,6 +25,7 @@ import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
 class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
+
     private final DockerClient docker;
     private final DockerImage dockerImage;
     private final ContainerResources containerResources;
@@ -148,8 +150,8 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
 
         final CreateContainerCmd containerCmd = docker
                 .createContainerCmd(dockerImage.asString())
-                .withCpuShares(containerResources.cpuShares)
-                .withMemory(containerResources.memoryBytes)
+                .withCpuShares(containerResources.cpuShares())
+                .withMemory(containerResources.memoryBytes())
                 .withName(containerName.asString())
                 .withHostName(hostName)
                 .withLabels(labels)
@@ -202,11 +204,11 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
                 .skip(1)
                 .collect(Collectors.joining(" "));
 
-        return String.join(" ",
+        return Stream.of(
                 "--name " + containerName.asString(),
                 "--hostname " + hostName,
-                "--cpu-shares " + containerResources.cpuShares,
-                "--memory " + containerResources.memoryBytes,
+                "--cpu-shares " + containerResources.cpuShares(),
+                "--memory " + containerResources.memoryBytes(),
                 toRepeatedOption("--label", labelList),
                 toRepeatedOption("--ulimit", ulimitList),
                 toRepeatedOption("--env", environmentAssignments),
@@ -219,7 +221,9 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
                 toOptionalOption("--entrypoint", entrypointExecuteable),
                 toFlagOption("--privileged", privileged),
                 dockerImage.asString(),
-                entrypointArgs);
+                entrypointArgs)
+                .filter(s -> !s.isEmpty())
+                .collect(Collectors.joining(" "));
     }
 
     /**
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 5e8a0feb099..966b5da9def 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
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.dockerapi;
 
 import java.net.InetAddress;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 
 /**
@@ -61,21 +60,8 @@ public interface Docker {
             ContainerName containerName,
             String hostName);
 
-    interface ContainerStats {
-        Map<String, Object> getNetworks();
-        Map<String, Object> getCpuStats();
-        Map<String, Object> getMemoryStats();
-        Map<String, Object> getBlkioStats();
-    }
-
-    default boolean networkNATed() {
-        return false;
-    }
-
     Optional<ContainerStats> getContainerStats(ContainerName containerName);
 
-    void createContainer(CreateContainerCommand createContainerCommand);
-
     void startContainer(ContainerName containerName);
 
     void stopContainer(ContainerName containerName);
@@ -97,8 +83,6 @@ public interface Docker {
      */
     boolean pullImageAsyncIfNeeded(DockerImage image);
 
-    void deleteImage(DockerImage dockerImage);
-
     /**
      * Deletes the local images that are currently not in use by any container and not recently used.
      */
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 c5c4547f796..00753f59461 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
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.dockerapi;
 
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.command.ExecCreateCmdResponse;
-import com.github.dockerjava.api.command.ExecStartCmd;
 import com.github.dockerjava.api.command.InspectContainerResponse;
 import com.github.dockerjava.api.command.InspectExecResponse;
 import com.github.dockerjava.api.command.InspectImageResponse;
@@ -22,6 +21,9 @@ import com.github.dockerjava.core.command.PullImageResultCallback;
 import com.github.dockerjava.jaxrs.JerseyDockerCmdExecFactory;
 import com.google.inject.Inject;
 import com.yahoo.log.LogLevel;
+import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException;
+import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
+import com.yahoo.vespa.hosted.dockerapi.exception.DockerExecTimeoutException;
 import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper;
 import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
@@ -51,7 +53,7 @@ import static com.yahoo.vespa.hosted.dockerapi.DockerNetworkCreator.NetworkAddre
 public class DockerImpl implements Docker {
     private static final Logger logger = Logger.getLogger(DockerImpl.class.getName());
 
-    public static final String DOCKER_CUSTOM_MACVLAN_NETWORK_NAME = "vespa-macvlan";
+    private static final String DOCKER_CUSTOM_MACVLAN_NETWORK_NAME = "vespa-macvlan";
     static final String LABEL_NAME_MANAGEDBY = "com.yahoo.vespa.managedby";
     private static final String FRAMEWORK_CONTAINER_PREFIX = "/";
 
@@ -83,7 +85,7 @@ public class DockerImpl implements Docker {
     }
 
     // For testing
-    DockerImpl(final DockerClient dockerClient) {
+    DockerImpl(DockerClient dockerClient) {
         this(null, null);
         this.dockerClient = dockerClient;
     }
@@ -106,11 +108,6 @@ public class DockerImpl implements Docker {
         }
     }
 
-    @Override
-    public boolean networkNATed() {
-        return config.networkNATed();
-    }
-
     private void setupDockerNetworkIfNeeded() throws IOException {
         if (!dockerClient.listNetworksCmd().withNameFilter(DOCKER_CUSTOM_MACVLAN_NETWORK_NAME).exec().isEmpty()) return;
 
@@ -140,7 +137,7 @@ public class DockerImpl implements Docker {
     }
 
     @Override
-    public boolean pullImageAsyncIfNeeded(final DockerImage image) {
+    public boolean pullImageAsyncIfNeeded(DockerImage image) {
         try {
             synchronized (monitor) {
                 if (scheduledPulls.contains(image)) return true;
@@ -159,7 +156,7 @@ public class DockerImpl implements Docker {
         }
     }
 
-    private void removeScheduledPoll(final DockerImage image) {
+    private void removeScheduledPoll(DockerImage image) {
         synchronized (monitor) {
             scheduledPulls.remove(image);
         }
@@ -168,7 +165,7 @@ public class DockerImpl implements Docker {
     /**
      * Check if a given image is already in the local registry
      */
-    boolean imageIsDownloaded(final DockerImage dockerImage) {
+    boolean imageIsDownloaded(DockerImage dockerImage) {
         return inspectImage(dockerImage).isPresent();
     }
 
@@ -195,6 +192,8 @@ public class DockerImpl implements Docker {
             dockerClient.connectToNetworkCmd()
                     .withContainerId(containerName.asString())
                     .withNetworkId(networkName).exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (RuntimeException e) {
             numberOfDockerDaemonFails.add();
             throw new DockerException("Failed to connect container '" + containerName.asString() +
@@ -222,33 +221,27 @@ public class DockerImpl implements Docker {
      */
     private ProcessResult executeInContainerAsUser(ContainerName containerName, String user, Optional<Long> timeoutSeconds, String... command) {
         try {
-            final ExecCreateCmdResponse response = dockerClient.execCreateCmd(containerName.asString())
-                    .withCmd(command)
-                    .withAttachStdout(true)
-                    .withAttachStderr(true)
-                    .withUser(user)
-                    .exec();
+            ExecCreateCmdResponse response = execCreateCmd(containerName, user, command);
 
             ByteArrayOutputStream output = new ByteArrayOutputStream();
             ByteArrayOutputStream errors = new ByteArrayOutputStream();
-            ExecStartCmd execStartCmd = dockerClient.execStartCmd(response.getId());
-            ExecStartResultCallback callback = execStartCmd.exec(new ExecStartResultCallback(output, errors));
+            ExecStartResultCallback callback = dockerClient.execStartCmd(response.getId())
+                    .exec(new ExecStartResultCallback(output, errors));
 
             if (timeoutSeconds.isPresent()) {
-                if (!callback.awaitCompletion(timeoutSeconds.get(), TimeUnit.SECONDS)) {
-                    throw new DockerExecTimeoutException(String.format("Command '%s' did not finish within %s seconds.", command[0], timeoutSeconds));
-                }
+                if (!callback.awaitCompletion(timeoutSeconds.get(), TimeUnit.SECONDS))
+                    throw new DockerExecTimeoutException(String.format(
+                            "Command '%s' did not finish within %s seconds.", command[0], timeoutSeconds));
             } else {
                 // Wait for completion no timeout
                 callback.awaitCompletion();
             }
 
-            final InspectExecResponse state = dockerClient.inspectExecCmd(execStartCmd.getExecId()).exec();
-            assert !state.isRunning();
-            Integer exitCode = state.getExitCode();
-            assert exitCode != null;
+            InspectExecResponse state = dockerClient.inspectExecCmd(response.getId()).exec();
+            if (state.isRunning())
+                throw new DockerException("Command '%s' did not finish within %s seconds.");
 
-            return new ProcessResult(exitCode, new String(output.toByteArray()), new String(errors.toByteArray()));
+            return new ProcessResult(state.getExitCode(), new String(output.toByteArray()), new String(errors.toByteArray()));
         } catch (RuntimeException | InterruptedException e) {
             numberOfDockerDaemonFails.add();
             throw new DockerException("Container '" + containerName.asString()
@@ -256,6 +249,19 @@ public class DockerImpl implements Docker {
         }
     }
 
+    private ExecCreateCmdResponse execCreateCmd(ContainerName containerName, String user, String... command) {
+        try {
+            return dockerClient.execCreateCmd(containerName.asString())
+                    .withCmd(command)
+                    .withAttachStdout(true)
+                    .withAttachStderr(true)
+                    .withUser(user)
+                    .exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
+        }
+    }
+
     private Optional<InspectContainerResponse> inspectContainerCmd(String container) {
         try {
             return Optional.of(dockerClient.inspectContainerCmd(container).exec());
@@ -273,7 +279,7 @@ public class DockerImpl implements Docker {
             DockerStatsCallback statsCallback = dockerClient.statsCmd(containerName.asString()).exec(new DockerStatsCallback());
             statsCallback.awaitCompletion(5, TimeUnit.SECONDS);
 
-            return statsCallback.stats.map(stats -> new ContainerStatsImpl(
+            return statsCallback.stats.map(stats -> new ContainerStats(
                     stats.getNetworks(), stats.getCpuStats(), stats.getMemoryStats(), stats.getBlkioStats()));
         } catch (NotFoundException ignored) {
             return Optional.empty();
@@ -284,21 +290,11 @@ public class DockerImpl implements Docker {
     }
 
     @Override
-    public void createContainer(CreateContainerCommand createContainerCommand) {
-        try {
-            dockerClient.execCreateCmd(createContainerCommand.toString());
-        } catch (NotModifiedException ignored) {
-            // If is already created, ignore
-        } catch (RuntimeException e) {
-            numberOfDockerDaemonFails.add();
-            throw new DockerException("Failed to create container '" + createContainerCommand.toString() + "'", e);
-        }
-    }
-
-    @Override
     public void startContainer(ContainerName containerName) {
         try {
             dockerClient.startContainerCmd(containerName.asString()).exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (NotModifiedException ignored) {
             // If is already started, ignore
         } catch (RuntimeException e) {
@@ -308,9 +304,11 @@ public class DockerImpl implements Docker {
     }
 
     @Override
-    public void stopContainer(final ContainerName containerName) {
+    public void stopContainer(ContainerName containerName) {
         try {
             dockerClient.stopContainerCmd(containerName.asString()).withTimeout(secondsToWaitBeforeKilling).exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (NotModifiedException ignored) {
             // If is already stopped, ignore
         } catch (RuntimeException e) {
@@ -328,8 +326,8 @@ public class DockerImpl implements Docker {
             });
 
             dockerClient.removeContainerCmd(containerName.asString()).exec();
-        } catch (NotFoundException ignored) {
-            // If container doesn't exist ignore
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (RuntimeException e) {
             numberOfDockerDaemonFails.add();
             throw new DockerException("Failed to delete container '" + containerName.asString() + "'", e);
@@ -366,7 +364,7 @@ public class DockerImpl implements Docker {
                 .orElse(Stream.empty());
     }
 
-    private boolean isManagedBy(final com.github.dockerjava.api.model.Container container, String manager) {
+    private boolean isManagedBy(com.github.dockerjava.api.model.Container container, String manager) {
         final Map<String, String> labels = container.getLabels();
         return labels != null && manager.equals(labels.get(LABEL_NAME_MANAGEDBY));
     }
@@ -393,8 +391,7 @@ public class DockerImpl implements Docker {
         }
     }
 
-    @Override
-    public void deleteImage(final DockerImage dockerImage) {
+    private void deleteImage(final DockerImage dockerImage) {
         try {
             dockerClient.removeImageCmd(dockerImage.asString()).exec();
         } catch (NotFoundException ignored) {
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/ContainerNotFoundException.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/ContainerNotFoundException.java
new file mode 100644
index 00000000000..b237228ee8e
--- /dev/null
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/ContainerNotFoundException.java
@@ -0,0 +1,13 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi.exception;
+
+import com.yahoo.vespa.hosted.dockerapi.ContainerName;
+
+/**
+ * @author freva
+ */
+public class ContainerNotFoundException extends DockerException {
+    public ContainerNotFoundException(ContainerName containerName) {
+        super("No such container: " + containerName.asString());
+    }
+}
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerException.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/DockerException.java
index b5b622977fb..df6bb702bf7 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerException.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/DockerException.java
@@ -1,5 +1,5 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.dockerapi;
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi.exception;
 
 /**
  * This exception wraps any exception thrown by docker-java
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerExecTimeoutException.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/DockerExecTimeoutException.java
index a315bc09d0b..39813db5c1e 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerExecTimeoutException.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/DockerExecTimeoutException.java
@@ -1,5 +1,5 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.dockerapi;
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi.exception;
 
 /**
  * Runtime exception to be thrown when the exec commands did not finish in time.
@@ -10,7 +10,7 @@ package com.yahoo.vespa.hosted.dockerapi;
  * @author smorgrav
  */
 @SuppressWarnings("serial")
-public class DockerExecTimeoutException extends RuntimeException {
+public class DockerExecTimeoutException extends DockerException {
     public DockerExecTimeoutException(String msg) {
         super(msg);
     }
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/package-info.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/package-info.java
new file mode 100644
index 00000000000..a5ec5f6c235
--- /dev/null
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/package-info.java
@@ -0,0 +1,5 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+@ExportPackage
+package com.yahoo.vespa.hosted.dockerapi.exception;
+
+import com.yahoo.osgi.annotation.ExportPackage;
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 90d71c067bc..4a19e5fe215 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
@@ -3,7 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.docker;
 
 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.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
@@ -36,9 +36,14 @@ public interface DockerOperations {
 
     void stopServicesOnNode(ContainerName containerName);
 
+    /**
+     * 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),
+     * and such that we will start serving again as soon as possible afterwards.
+     */
     void trySuspendNode(ContainerName containerName);
 
-    Optional<Docker.ContainerStats> getContainerStats(ContainerName containerName);
+    Optional<ContainerStats> getContainerStats(ContainerName containerName);
 
     /**
      * Returns the list of containers managed by node-admin
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 a197eafe923..9bd3b5c1d82 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
@@ -8,10 +8,12 @@ import com.yahoo.system.ProcessExecuter;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
+import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.DockerNetworkCreator;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
+import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData;
@@ -87,13 +89,12 @@ public class DockerOperationsImpl implements DockerOperations {
                 .withAddCapability("SYS_PTRACE") // Needed for gcore, pstack etc.
                 .withAddCapability("SYS_ADMIN"); // Needed for perf
 
-        if (environment.getNodeType() == NodeType.confighost ||
-                environment.getNodeType() == NodeType.proxyhost) {
+        if (environment.getNodeType() == NodeType.confighost || environment.getNodeType() == NodeType.proxyhost) {
             command.withVolume("/var/lib/sia", "/var/lib/sia");
         }
 
         if (environment.getNodeType() == NodeType.proxyhost) {
-            command.withVolume("/opt/yahoo/share/ssl/certs/", "/opt/yahoo/share/ssl/certs/");
+            command.withVolume("/opt/yahoo/share/ssl/certs", "/opt/yahoo/share/ssl/certs");
         }
 
         if (environment.getNodeType() == NodeType.host) {
@@ -142,8 +143,6 @@ public class DockerOperationsImpl implements DockerOperations {
 
         logger.info("Creating new container with args: " + command);
         command.create();
-
-        docker.createContainer(command);
     }
 
     void addEtcHosts(ContainerData containerData,
@@ -217,26 +216,6 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     /**
-     * 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),
-     * and such that we will start serving again as soon as possible afterwards.
-     * <p>
-     * Any failures are logged and ignored.
-     */
-    @Override
-    public void trySuspendNode(ContainerName containerName) {
-        try {
-            // TODO: Change to waiting w/o timeout (need separate thread that we can stop).
-            executeCommandInContainer(containerName, nodeProgram, "suspend");
-        } catch (RuntimeException e) {
-            PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
-            // It's bad to continue as-if nothing happened, but on the other hand if we do not proceed to
-            // remove container, we will not be able to upgrade to fix any problems in the suspend logic!
-            logger.warning("Failed trying to suspend container " + containerName.asString(), e);
-        }
-    }
-
-    /**
      * For macvlan:
      * <p>
      * Due to a bug in docker (https://github.com/docker/libnetwork/issues/1443), we need to manually set
@@ -304,7 +283,6 @@ public class DockerOperationsImpl implements DockerOperations {
                     Arrays.toString(wrappedCommand), containerName.asString(), containerPid), e);
             throw new RuntimeException(e);
         }
-
     }
 
     @Override
@@ -323,7 +301,21 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
-    public Optional<Docker.ContainerStats> getContainerStats(ContainerName containerName) {
+    public void trySuspendNode(ContainerName containerName) {
+        try {
+            executeCommandInContainer(containerName, nodeProgram, "suspend");
+        } catch (ContainerNotFoundException e) {
+            throw e;
+        } catch (RuntimeException e) {
+            PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
+            // It's bad to continue as-if nothing happened, but on the other hand if we do not proceed to
+            // remove container, we will not be able to upgrade to fix any problems in the suspend logic!
+            logger.warning("Failed trying to suspend container " + containerName.asString(), e);
+        }
+    }
+
+    @Override
+    public Optional<ContainerStats> getContainerStats(ContainerName containerName) {
         return docker.getContainerStats(containerName);
     }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
index cdfc8eef798..914836114ad 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
@@ -189,10 +189,8 @@ public class StorageMaintainer {
         try {
             FilebeatConfigProvider filebeatConfigProvider = new FilebeatConfigProvider(environment);
             Optional<String> config = filebeatConfigProvider.getConfig(node);
-            if (!config.isPresent()) {
-                logger.error("Was not able to generate a config for filebeat, ignoring filebeat file creation." + node.toString());
-                return;
-            }
+            if (!config.isPresent()) return;
+
             Path filebeatPath = environment.pathInNodeAdminFromPathInNode(
                     containerName, Paths.get("/etc/filebeat/filebeat.yml"));
             Files.write(filebeatPath, config.get().getBytes());
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 7c84150009e..be7d31ff1dc 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
@@ -6,9 +6,10 @@ import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
-import com.yahoo.vespa.hosted.dockerapi.Docker;
-import com.yahoo.vespa.hosted.dockerapi.DockerException;
-import com.yahoo.vespa.hosted.dockerapi.DockerExecTimeoutException;
+import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
+import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException;
+import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
+import com.yahoo.vespa.hosted.dockerapi.exception.DockerExecTimeoutException;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.dockerapi.metrics.DimensionMetrics;
@@ -262,7 +263,7 @@ public class NodeAgentImpl implements NodeAgent {
                 .flatMap(container -> removeContainerIfNeeded(node, container))
                 .map(container -> {
                         shouldRestartServices(node).ifPresent(restartReason -> {
-                            logger.info("Will restart services for container " + container + ": " + restartReason);
+                            logger.info("Will restart services: " + restartReason);
                             restartServices(node, container);
                         });
                         return container;
@@ -283,7 +284,7 @@ public class NodeAgentImpl implements NodeAgent {
     private void restartServices(NodeSpec node, Container existingContainer) {
         if (existingContainer.state.isRunning() && node.getState() == Node.State.active) {
             ContainerName containerName = existingContainer.name;
-            logger.info("Restarting services for " + containerName);
+            logger.info("Restarting services");
             // Since we are restarting the services we need to suspend the node.
             orchestratorSuspendNode();
             dockerOperations.restartVespaOnNode(containerName);
@@ -292,9 +293,14 @@ public class NodeAgentImpl implements NodeAgent {
 
     @Override
     public void stopServices() {
-        logger.info("Stopping services for " + containerName);
-        dockerOperations.trySuspendNode(containerName);
-        dockerOperations.stopServicesOnNode(containerName);
+        logger.info("Stopping services");
+        if (containerState == ABSENT) return;
+        try {
+            dockerOperations.trySuspendNode(containerName);
+            dockerOperations.stopServicesOnNode(containerName);
+        } catch (ContainerNotFoundException e) {
+            containerState = ABSENT;
+        }
     }
 
     private Optional<String> shouldRemoveContainer(NodeSpec node, Container existingContainer) {
@@ -324,7 +330,7 @@ public class NodeAgentImpl implements NodeAgent {
     private Optional<Container> removeContainerIfNeeded(NodeSpec node, Container existingContainer) {
         Optional<String> removeReason = shouldRemoveContainer(node, existingContainer);
         if (removeReason.isPresent()) {
-            logger.info("Will remove container " + existingContainer + ": " + removeReason.get());
+            logger.info("Will remove container: " + removeReason.get());
 
             if (existingContainer.state.isRunning()) {
                 if (node.getState() == Node.State.active) {
@@ -378,7 +384,7 @@ public class NodeAgentImpl implements NodeAgent {
                     try {
                         monitor.wait(remainder);
                     } catch (InterruptedException e) {
-                        logger.error("Interrupted, but ignoring this: " + hostname);
+                        logger.error("Interrupted while sleeping before tick, ignoring");
                     }
                 } else break;
             }
@@ -403,9 +409,12 @@ public class NodeAgentImpl implements NodeAgent {
                 converged = true;
             } catch (OrchestratorException e) {
                 logger.info(e.getMessage());
+            } catch (ContainerNotFoundException e) {
+                containerState = ABSENT;
+                logger.warning("Container unexpectedly gone, resetting containerState to " + containerState);
             } catch (DockerException e) {
                 numberOfUnhandledException++;
-                logger.error("Caught a DockerException, resetting containerState to " + containerState, e);
+                logger.error("Caught a DockerException", e);
             } catch (Exception e) {
                 numberOfUnhandledException++;
                 logger.error("Unhandled exception, ignoring.", e);
@@ -554,7 +563,7 @@ public class NodeAgentImpl implements NodeAgent {
         final NodeSpec node = lastNode;
         if (node == null || containerState != UNKNOWN) return;
 
-        Optional<Docker.ContainerStats> containerStats = dockerOperations.getContainerStats(containerName);
+        Optional<ContainerStats> containerStats = dockerOperations.getContainerStats(containerName);
         if (!containerStats.isPresent()) return;
 
         Dimensions.Builder dimensionsBuilder = new Dimensions.Builder()
@@ -566,7 +575,7 @@ public class NodeAgentImpl implements NodeAgent {
                 dimensionsBuilder.add("orchestratorState", allowed ? "ALLOWED_TO_BE_DOWN" : "NO_REMARKS"));
         Dimensions dimensions = dimensionsBuilder.build();
 
-        Docker.ContainerStats stats = containerStats.get();
+        ContainerStats stats = containerStats.get();
         final String APP = MetricReceiverWrapper.APPLICATION_NODE;
         final int totalNumCpuCores = ((List<Number>) ((Map) stats.getCpuStats().get("cpu_usage")).get("percpu_usage")).size();
         final long cpuContainerKernelTime = ((Number) ((Map) stats.getCpuStats().get("cpu_usage")).get("usage_in_kernelmode")).longValue();
@@ -631,7 +640,7 @@ public class NodeAgentImpl implements NodeAgent {
             String[] command = {"vespa-rpc-invoke",  "-t", "2",  "tcp/localhost:19091",  "setExtraMetrics", wrappedMetrics};
             dockerOperations.executeCommandInContainerAsRoot(containerName, 5L, command);
         } catch (DockerExecTimeoutException | JsonProcessingException  e) {
-            logger.warning("Unable to push metrics to container: " + containerName, e);
+            logger.warning("Failed to push metrics to container", e);
         }
     }
 
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 4b4ef05593d..c49e7ad4857 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
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
+import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
@@ -69,13 +70,6 @@ public class DockerMock implements Docker {
     }
 
     @Override
-    public void createContainer(CreateContainerCommand createContainerCommand) {
-        synchronized (monitor) {
-            callOrderVerifier.add("createContainer with " + createContainerCommand.toString());
-        }
-    }
-
-    @Override
     public void startContainer(ContainerName containerName) {
         synchronized (monitor) {
             callOrderVerifier.add("startContainer with " + containerName);
@@ -116,11 +110,6 @@ public class DockerMock implements Docker {
     }
 
     @Override
-    public void deleteImage(DockerImage dockerImage) {
-
-    }
-
-    @Override
     public void deleteUnusedDockerImages() {
 
     }
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 ebed20326a3..e0031f9b9b3 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
@@ -8,9 +8,8 @@ import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
-import com.yahoo.vespa.hosted.dockerapi.ContainerStatsImpl;
-import com.yahoo.vespa.hosted.dockerapi.Docker;
-import com.yahoo.vespa.hosted.dockerapi.DockerException;
+import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
+import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
@@ -29,8 +28,8 @@ import com.yahoo.vespa.hosted.provision.Node;
 import org.junit.Test;
 import org.mockito.InOrder;
 
-import java.io.File;
 import java.io.IOException;
+import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -78,7 +77,7 @@ public class NodeAgentImplTest {
     private final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class);
     private final MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
     private final AclMaintainer aclMaintainer = mock(AclMaintainer.class);
-    private final Docker.ContainerStats emptyContainerStats = new ContainerStatsImpl(Collections.emptyMap(),
+    private final ContainerStats emptyContainerStats = new ContainerStats(Collections.emptyMap(),
             Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap());
     private final AthenzCredentialsMaintainer athenzCredentialsMaintainer = mock(AthenzCredentialsMaintainer.class);
 
@@ -598,7 +597,7 @@ public class NodeAgentImplTest {
     public void testGetRelevantMetrics() throws Exception {
         final ObjectMapper objectMapper = new ObjectMapper();
         ClassLoader classLoader = getClass().getClassLoader();
-        File statsFile = new File(classLoader.getResource("docker.stats.json").getFile());
+        URL statsFile = classLoader.getResource("docker.stats.json");
         Map<String, Map<String, Object>> dockerStats = objectMapper.readValue(statsFile, Map.class);
 
         Map<String, Object> networks = dockerStats.get("networks");
@@ -606,8 +605,8 @@ public class NodeAgentImplTest {
         Map<String, Object> cpu_stats = dockerStats.get("cpu_stats");
         Map<String, Object> memory_stats = dockerStats.get("memory_stats");
         Map<String, Object> blkio_stats = dockerStats.get("blkio_stats");
-        Docker.ContainerStats stats1 = new ContainerStatsImpl(networks, precpu_stats, memory_stats, blkio_stats);
-        Docker.ContainerStats stats2 = new ContainerStatsImpl(networks, cpu_stats, memory_stats, blkio_stats);
+        ContainerStats stats1 = new ContainerStats(networks, precpu_stats, memory_stats, blkio_stats);
+        ContainerStats stats2 = new ContainerStats(networks, cpu_stats, memory_stats, blkio_stats);
 
         NodeSpec.Owner owner = new NodeSpec.Owner("tester", "testapp", "testinstance");
         NodeSpec.Membership membership = new NodeSpec.Membership("clustType", "clustId", "grp", 3, false);