Missing container exception fix

12 files changed, 106 insertions, 59 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 03b74532b49..cafc4254f60 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
@@ -8,6 +8,7 @@ import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
 import com.yahoo.vespa.hosted.node.admin.orchestrator.Orchestrator;
 
+import java.util.List;
 import java.util.Optional;
 
 public interface DockerOperations {
@@ -36,5 +37,12 @@ public interface DockerOperations {
 
     void trySuspendNode(ContainerName containerName);
 
-    Docker.ContainerStats getContainerStats(ContainerName containerName);
+    Optional<Docker.ContainerStats> getContainerStats(ContainerName containerName);
+
+    /**
+     * Returns the list of containers managed by node-admin
+     */
+    List<Container> getAllManagedContainers();
+
+    void deleteUnusedDockerImages();
 }
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 ec46428a4d3..f4d031f2f98 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.docker;
 
 import com.google.common.base.Joiner;
 import com.google.common.io.CharStreams;
+import com.yahoo.net.HostName;
 import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
@@ -10,6 +11,9 @@ import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.DockerImpl;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
+import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions;
+import com.yahoo.vespa.hosted.dockerapi.metrics.GaugeWrapper;
+import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
 import com.yahoo.vespa.hosted.node.admin.orchestrator.Orchestrator;
 import com.yahoo.vespa.hosted.node.admin.util.Environment;
@@ -47,6 +51,8 @@ public class DockerOperationsImpl implements DockerOperations {
 
     private static final Pattern VESPA_VERSION_PATTERN = Pattern.compile("^(\\S*)$", Pattern.MULTILINE);
 
+    private static final String MANAGER_NAME = "node-admin";
+
     // Map of directories to mount and whether they should be writeable by everyone
     private static final Map<String, Boolean> DIRECTORIES_TO_MOUNT = new HashMap<>();
     static {
@@ -73,11 +79,13 @@ public class DockerOperationsImpl implements DockerOperations {
     private final Docker docker;
     private final Environment environment;
     private final Maintainer maintainer;
+    private GaugeWrapper numberOfRunningContainersGauge;
 
-    public DockerOperationsImpl(Docker docker, Environment environment, Maintainer maintainer) {
+    public DockerOperationsImpl(Docker docker, Environment environment, Maintainer maintainer, MetricReceiverWrapper metricReceiver) {
         this.docker = docker;
         this.environment = environment;
         this.maintainer = maintainer;
+        setMetrics(metricReceiver);
     }
 
     @Override
@@ -114,6 +122,7 @@ public class DockerOperationsImpl implements DockerOperations {
         if (docker.getContainer(nodeSpec.hostname).isPresent()) return false;
 
         startContainer(nodeSpec);
+        numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
         return true;
     }
 
@@ -188,7 +197,7 @@ public class DockerOperationsImpl implements DockerOperations {
                     nodeSpec.wantedDockerImage.get(),
                     nodeSpec.containerName,
                     nodeSpec.hostname)
-                    .withLabel(DockerImpl.LABEL_NAME_MANAGEDBY, DockerImpl.LABEL_VALUE_MANAGEDBY)
+                    .withManagedBy(MANAGER_NAME)
                     .withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME)
                     .withIpAddress(nodeInetAddress)
                     .withEnvironment("CONFIG_SERVER_ADDRESS", configServers)
@@ -221,7 +230,7 @@ public class DockerOperationsImpl implements DockerOperations {
             if (isIPv6) {
                 docker.connectContainerToNetwork(nodeSpec.containerName, "bridge");
                 docker.startContainer(nodeSpec.containerName);
-                setupContainerNetworkingWithScript(nodeSpec.containerName, nodeSpec.hostname);
+                setupContainerNetworkingWithScript(nodeSpec.containerName);
             } else {
                 docker.startContainer(nodeSpec.containerName);
             }
@@ -233,14 +242,16 @@ public class DockerOperationsImpl implements DockerOperations {
         }
     }
 
-    private void setupContainerNetworkingWithScript(ContainerName containerName, String hostName) {
+    private void setupContainerNetworkingWithScript(ContainerName containerName) {
         PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
 
-        Docker.ContainerInfo containerInfo = docker.inspectContainer(containerName);
-        Optional<Integer> containerPid = containerInfo.getPid();
+        Optional<Docker.ContainerInfo> containerInfo = docker.inspectContainer(containerName);
+        if (!containerInfo.isPresent()) {
+            throw new RuntimeException("Container " + containerName + " does not exist");
+        }
+        Optional<Integer> containerPid = containerInfo.get().getPid();
         if (!containerPid.isPresent()) {
-            throw new RuntimeException("Container " + containerName + " for host "
-                    + hostName + " isn't running (pid not found)");
+            throw new RuntimeException("Container " + containerName + " isn't running (pid not found)");
         }
 
         final List<String> command = new LinkedList<>();
@@ -307,6 +318,7 @@ public class DockerOperationsImpl implements DockerOperations {
 
         logger.info("Deleting container " + containerName);
         docker.deleteContainer(containerName);
+        numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
     }
 
     @Override
@@ -335,7 +347,28 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
-    public Docker.ContainerStats getContainerStats(ContainerName containerName) {
+    public Optional<Docker.ContainerStats> getContainerStats(ContainerName containerName) {
         return docker.getContainerStats(containerName);
     }
+
+    @Override
+    public List<Container> getAllManagedContainers() {
+        return docker.getAllContainersManagedBy(MANAGER_NAME);
+    }
+
+    @Override
+    public void deleteUnusedDockerImages() {
+        docker.deleteUnusedDockerImages();
+    }
+
+    private void setMetrics(MetricReceiverWrapper metricReceiver) {
+        Dimensions dimensions = new Dimensions.Builder()
+                .add("host", HostName.getLocalhost())
+                .add("role", "docker").build();
+
+        numberOfRunningContainersGauge = metricReceiver.declareGauge(dimensions, "containers.running");
+
+        // Some containers could already be running, count them and initialize to that value
+        numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
+    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
index 5842166921a..fcfa6a3cfd7 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
@@ -11,6 +11,7 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent;
 import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
@@ -43,7 +44,7 @@ public class NodeAdminImpl implements NodeAdmin {
     private static final PrefixLogger logger = PrefixLogger.getNodeAdminLogger(NodeAdmin.class);
     private final ScheduledExecutorService metricsFetcherScheduler = Executors.newScheduledThreadPool(1);
 
-    private final Docker docker;
+    private final DockerOperations dockerOperations;
     private final Function<String, NodeAgent> nodeAgentFactory;
     private final StorageMaintainer storageMaintainer;
     private AtomicBoolean frozen = new AtomicBoolean(false);
@@ -56,14 +57,10 @@ public class NodeAdminImpl implements NodeAdmin {
     private GaugeWrapper numberOfContainersInLoadImageState;
     private CounterWrapper numberOfUnhandledExceptionsInNodeAgent;
 
-    /**
-     * @param docker interface to docker daemon and docker-related tasks
-     * @param nodeAgentFactory factory for {@link NodeAgent} objects
-     */
-    public NodeAdminImpl(final Docker docker, final Function<String, NodeAgent> nodeAgentFactory,
+    public NodeAdminImpl(final DockerOperations dockerOperations, final Function<String, NodeAgent> nodeAgentFactory,
                          final StorageMaintainer storageMaintainer, int nodeAgentScanIntervalMillis,
                          final MetricReceiverWrapper metricReceiver) {
-        this.docker = docker;
+        this.dockerOperations = dockerOperations;
         this.nodeAgentFactory = nodeAgentFactory;
         this.storageMaintainer = storageMaintainer;
         this.nodeAgentScanIntervalMillis = nodeAgentScanIntervalMillis;
@@ -86,11 +83,11 @@ public class NodeAdminImpl implements NodeAdmin {
     }
 
     public void refreshContainersToRun(final List<ContainerNodeSpec> containersToRun) {
-        final List<Container> existingContainers = docker.getAllManagedContainers();
+        final List<Container> existingContainers = dockerOperations.getAllManagedContainers();
 
         storageMaintainer.cleanNodeAdmin();
         synchronizeNodeSpecsToNodeAgents(containersToRun, existingContainers);
-        docker.deleteUnusedDockerImages();
+        dockerOperations.deleteUnusedDockerImages();
 
         updateNodeAgentMetrics();
     }
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 f9b59a26eeb..ecd99939332 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
@@ -467,10 +467,10 @@ public class NodeAgentImpl implements NodeAgent {
         }
 
         if (nodeSpec == null || nodeSpec.nodeState != Node.State.active) return;
-        final Optional<Container> container = dockerOperations.getContainer(nodeSpec.hostname);
-        if ( ! container.isPresent() || ! container.get().isRunning ) return;
+        Optional<Docker.ContainerStats> containerStats = dockerOperations.getContainerStats(nodeSpec.containerName);
+        if ( ! containerStats.isPresent()) return;
 
-        Docker.ContainerStats stats = dockerOperations.getContainerStats(nodeSpec.containerName);
+        Docker.ContainerStats stats = containerStats.get();
         Dimensions.Builder dimensionsBuilder = new Dimensions.Builder()
                 .add("host", hostname)
                 .add("role", "tenants")
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
index a6d35247615..556573dcb62 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
@@ -6,6 +6,7 @@ import com.yahoo.net.HostName;
 import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdmin;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl;
@@ -53,14 +54,15 @@ public class ComponentsProviderImpl implements ComponentsProvider {
         Orchestrator orchestrator = new OrchestratorImpl(configServerHosts);
         NodeRepository nodeRepository = new NodeRepositoryImpl(configServerHosts, WEB_SERVICE_PORT, baseHostName);
 
+        final DockerOperations dockerOperations = new DockerOperationsImpl(
+                docker, environment, storageMaintainer.getMaintainer(), metricReceiver);
         final Function<String, NodeAgent> nodeAgentFactory =
-                (hostName) -> new NodeAgentImpl(hostName, nodeRepository,
-                        orchestrator, new DockerOperationsImpl(docker, environment, storageMaintainer.getMaintainer()),
+                (hostName) -> new NodeAgentImpl(hostName, nodeRepository, orchestrator, dockerOperations,
                         storageMaintainer, metricReceiver, environment, storageMaintainer.getMaintainer());
-        final NodeAdmin nodeAdmin = new NodeAdminImpl(docker, nodeAgentFactory, storageMaintainer,
+        final NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer,
                 NODE_AGENT_SCAN_INTERVAL_MILLIS, metricReceiver);
-        nodeAdminStateUpdater = new NodeAdminStateUpdater(
-                nodeRepository, nodeAdmin, INITIAL_SCHEDULER_DELAY_MILLIS, NODE_ADMIN_STATE_INTERVAL_MILLIS, orchestrator, baseHostName);
+        nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepository, nodeAdmin, INITIAL_SCHEDULER_DELAY_MILLIS,
+                NODE_ADMIN_STATE_INTERVAL_MILLIS, orchestrator, baseHostName);
 
         metricReceiverWrapper = metricReceiver;
     }
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 c7a641feda4..5a431c491b8 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
@@ -1,9 +1,11 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.docker;
 
+import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
+import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.util.Environment;
 import com.yahoo.vespa.hosted.node.admin.util.InetAddressResolver;
 import com.yahoo.vespa.hosted.node.maintenance.Maintainer;
@@ -30,7 +32,8 @@ public class DockerOperationsImplTest {
                                               "parent.host.name.yahoo.com",
                                               new InetAddressResolver());
     private final Docker docker = mock(Docker.class);
-    private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker, environment, new Maintainer());
+    private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker, environment,
+            new Maintainer(), new MetricReceiverWrapper(MetricReceiver.nullImplementation));
 
     @Test
     public void absenceOfNodeProgramIsSuccess() throws Exception {
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 a7f3b6b78ac..7f2292f3b84 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
@@ -85,7 +85,7 @@ public class LocalZoneUtils {
                 .replaceAll("\\$NODE_ADMIN_FROM_IMAGE", vespaBaseImage.asString())
                 .replaceAll("\\$VESPA_HOME", Defaults.getDefaults().vespaHome());
 
-        /**
+        /*
          * Because the daemon could be running on a remote machine, docker build command will upload the entire
          * build path to daemon and then execute the Dockerfile. This means that:
          * 1. We cant use relative paths in Dockerfile
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java
index 63ba0cbb7ed..a32f0edcf99 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests;
 
 import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdmin;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater;
@@ -37,13 +38,13 @@ public class ComponentsProviderWithMocks implements ComponentsProvider {
                                                       "parent.host.name.yahoo.com",
                                                       new InetAddressResolver());
     private final MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
+    private final DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, maintainer, mr);
     private final Function<String, NodeAgent> nodeAgentFactory =
             (hostName) -> new NodeAgentImpl(hostName,
-                                            nodeRepositoryMock, orchestratorMock,
-                                            new DockerOperationsImpl(dockerMock, environment, maintainer),
+                                            nodeRepositoryMock, orchestratorMock, dockerOperations,
                                             maintenanceSchedulerMock, mr,
                                             environment, maintainer);
-    private NodeAdmin nodeAdmin = new NodeAdminImpl(dockerMock, nodeAgentFactory, maintenanceSchedulerMock, 100, mr);
+    private NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, maintenanceSchedulerMock, 100, mr);
 
 
     @Override
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 79e5562678c..c916e8d9909 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
@@ -19,7 +19,7 @@ import java.util.stream.Collectors;
 /**
  * Mock with some simple logic
  *
- * @author valerijf
+ * @author freva
  */
 public class DockerMock implements Docker {
     private List<Container> containers = new ArrayList<>();
@@ -57,12 +57,19 @@ public class DockerMock implements Docker {
     }
 
     @Override
-    public ContainerInfo inspectContainer(ContainerName containerName) {
-        return () -> Optional.of(2);
+    public List<Container> getAllContainersManagedBy(String manager) {
+        synchronized (monitor) {
+            return new ArrayList<>(containers);
+        }
+    }
+
+    @Override
+    public Optional<ContainerInfo> inspectContainer(ContainerName containerName) {
+        return Optional.of(() -> Optional.of(2));
     }
 
     @Override
-    public ContainerStats getContainerStats(ContainerName containerName) {
+    public Optional<ContainerStats> getContainerStats(ContainerName containerName) {
         return null;
     }
 
@@ -95,13 +102,6 @@ public class DockerMock implements Docker {
     }
 
     @Override
-    public List<Container> getAllManagedContainers() {
-        synchronized (monitor) {
-            return new ArrayList<>(containers);
-        }
-    }
-
-    @Override
     public Optional<Container> getContainer(String hostname) {
         synchronized (monitor) {
             return containers.stream().filter(container -> container.hostname.equals(hostname)).findFirst();
@@ -200,6 +200,11 @@ public class DockerMock implements Docker {
         }
 
         @Override
+        public CreateContainerCommand withManagedBy(String manager) {
+            return this;
+        }
+
+        @Override
         public void create() {
 
         }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index bed825bc960..63f1a4003fd 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdmin;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl;
@@ -63,13 +64,11 @@ public class DockerTester implements AutoCloseable {
                                                   inetAddressResolver);
 
         MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
-        Function<String, NodeAgent> nodeAgentFactory = (hostName) -> {
-            final Maintainer maintainer = new Maintainer();
-            return new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock,
-                                     new DockerOperationsImpl(dockerMock, environment, maintainer),
-                                     maintenanceSchedulerMock, mr, environment, maintainer);
-        };
-        nodeAdmin = new NodeAdminImpl(dockerMock, nodeAgentFactory, maintenanceSchedulerMock, 100, mr);
+        final Maintainer maintainer = new Maintainer();
+        final DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, maintainer, mr);
+        Function<String, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock,
+                orchestratorMock, dockerOperations, maintenanceSchedulerMock, mr, environment, maintainer);
+        nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, maintenanceSchedulerMock, 100, mr);
         updater = new NodeAdminStateUpdater(nodeRepositoryMock, nodeAdmin, 1, 1, orchestratorMock, "basehostname");
     }
 
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 5af24e71c3d..2b414c06661 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
@@ -7,8 +7,8 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
 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.DockerImage;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl;
@@ -48,11 +48,11 @@ public class NodeAdminImplTest {
 
     @Test
     public void nodeAgentsAreProperlyLifeCycleManaged() throws Exception {
-        final Docker docker = mock(Docker.class);
+        final DockerOperations dockerOperations = mock(DockerOperations.class);
         final Function<String, NodeAgent> nodeAgentFactory = mock(NodeAgentFactory.class);
         final StorageMaintainer storageMaintainer = mock(StorageMaintainer.class);
 
-        final NodeAdminImpl nodeAdmin = new NodeAdminImpl(docker, nodeAgentFactory, storageMaintainer, 100,
+        final NodeAdminImpl nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer, 100,
                 new MetricReceiverWrapper(MetricReceiver.nullImplementation));
 
         final NodeAgent nodeAgent1 = mock(NodeAgentImpl.class);
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 b90ea2233bc..0fa6eae9e67 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
@@ -60,8 +60,7 @@ public class NodeAgentImplTest {
     private final Maintainer maintainer = mock(Maintainer.class);
     private final MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
 
-
-    Environment environment = new Environment(Collections.emptySet(),
+    private final Environment environment = new Environment(Collections.emptySet(),
                                               "dev",
                                               "us-east-1",
                                               "parent.host.name.yahoo.com",
@@ -92,7 +91,7 @@ public class NodeAgentImplTest {
 
         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.getContainerStats(any())).thenReturn(containerStats);
+        when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(false);
         when(dockerOperations.startContainerIfNeeded(eq(nodeSpec))).thenReturn(false);
         when(dockerOperations.getVespaVersion(eq(containerName))).thenReturn(vespaVersion);
@@ -140,7 +139,7 @@ public class NodeAgentImplTest {
 
         Docker.ContainerStats containerStats = new ContainerStatsImpl(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>());
         when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.empty());
-        when(dockerOperations.getContainerStats(any())).thenReturn(containerStats);
+        when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(false);
         when(dockerOperations.startContainerIfNeeded(eq(nodeSpec))).thenReturn(true);
         when(dockerOperations.getVespaVersion(eq(containerName))).thenReturn(vespaVersion);
@@ -412,7 +411,7 @@ public class NodeAgentImplTest {
                 MIN_DISK_AVAILABLE_GB);
 
         Docker.ContainerStats containerStats = new ContainerStatsImpl(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>());
-        when(dockerOperations.getContainerStats(any())).thenReturn(containerStats);
+        when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
         when(dockerOperations.getContainer(eq(hostName))).thenReturn(Optional.of(new Container(hostName, wantedDockerImage, containerName, true)));
         when(nodeRepository.getContainerNodeSpec(eq(hostName))).thenReturn(Optional.of(nodeSpec));
         when(dockerOperations.shouldScheduleDownloadOfImage(eq(wantedDockerImage))).thenReturn(false);
@@ -458,7 +457,7 @@ public class NodeAgentImplTest {
         Docker.ContainerStats stats = new ContainerStatsImpl(networks, cpu_stats, memory_stats, blkio_stats);
 
         final ContainerName containerName = new ContainerName("cont-name");
-        when(dockerOperations.getContainerStats(eq(containerName))).thenReturn(stats);
+        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)));