Merge pull request #1939 from yahoo/freva/run-acl-maintainer-before-starting-container

Run ACL maintainer just before starting the container

7 files changed, 89 insertions, 87 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 fece46d31c7..80c8f148cbf 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
@@ -14,8 +14,9 @@ import java.util.Optional;
 public interface DockerOperations {
     Optional<String> getVespaVersion(ContainerName containerName);
 
-    // Returns true if started
-    boolean startContainerIfNeeded(ContainerName containerName, ContainerNodeSpec nodeSpec);
+    void startContainer(ContainerName containerName, ContainerNodeSpec nodeSpec);
+
+    void removeContainer(Container existingContainer);
 
     // Returns false if image is already downloaded
     boolean shouldScheduleDownloadOfImage(DockerImage dockerImage);
@@ -24,8 +25,6 @@ public interface DockerOperations {
 
     void scheduleDownloadOfImage(ContainerName containerName, ContainerNodeSpec nodeSpec, Runnable callback);
 
-    void removeContainer(ContainerNodeSpec nodeSpec, Container existingContainer);
-
     ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String[] command);
 
     void executeCommandInNetworkNamespace(ContainerName containerName, String[] command);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index 1ae6c64b863..ed18b9fe305 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
@@ -133,49 +133,8 @@ public class DockerOperationsImpl implements DockerOperations {
         return matcher.find() ? Optional.of(matcher.group(1)) : Optional.empty();
     }
 
-    // Returns true if started
     @Override
-    public boolean startContainerIfNeeded(ContainerName containerName, final ContainerNodeSpec nodeSpec) {
-        if (docker.getContainer(containerName).isPresent()) return false;
-
-        startContainer(containerName, nodeSpec);
-        numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
-        return true;
-    }
-
-    // Returns true if scheduling download
-    @Override
-    public boolean shouldScheduleDownloadOfImage(final DockerImage dockerImage) {
-        return !docker.imageIsDownloaded(dockerImage);
-    }
-
-    @Override
-    public Optional<Container> getContainer(ContainerName containerName) {
-        return docker.getContainer(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.
-     *
-     * 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, SUSPEND_NODE_COMMAND);
-        } 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() + "  with "
-                   + Arrays.toString(SUSPEND_NODE_COMMAND), e);
-        }
-    }
-
-    private void startContainer(ContainerName containerName, final ContainerNodeSpec nodeSpec) {
+    public void startContainer(ContainerName containerName, final ContainerNodeSpec nodeSpec) {
         PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
 
         logger.info("Starting container " + containerName);
@@ -233,6 +192,54 @@ public class DockerOperationsImpl implements DockerOperations {
         } catch (IOException e) {
             throw new RuntimeException("Failed to create container " + containerName.asString(), e);
         }
+
+        numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
+    }
+
+    @Override
+    public void removeContainer(final Container existingContainer) {
+        final ContainerName containerName = existingContainer.name;
+        PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
+        if (existingContainer.state.isRunning()) {
+            logger.info("Stopping container " + containerName.asString());
+            docker.stopContainer(containerName);
+        }
+
+        logger.info("Deleting container " + containerName.asString());
+        docker.deleteContainer(containerName);
+        numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
+    }
+
+    // Returns true if scheduling download
+    @Override
+    public boolean shouldScheduleDownloadOfImage(final DockerImage dockerImage) {
+        return !docker.imageIsDownloaded(dockerImage);
+    }
+
+    @Override
+    public Optional<Container> getContainer(ContainerName containerName) {
+        return docker.getContainer(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.
+     *
+     * 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, SUSPEND_NODE_COMMAND);
+        } 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() + "  with "
+                   + Arrays.toString(SUSPEND_NODE_COMMAND), e);
+        }
     }
 
     /**
@@ -261,20 +268,6 @@ public class DockerOperationsImpl implements DockerOperations {
         });
     }
 
-    @Override
-    public void removeContainer(final ContainerNodeSpec nodeSpec, final Container existingContainer) {
-        final ContainerName containerName = existingContainer.name;
-        PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
-        if (existingContainer.state.isRunning()) {
-            logger.info("Stopping container " + containerName.asString());
-            docker.stopContainer(containerName);
-        }
-
-        logger.info("Deleting container " + containerName.asString());
-        docker.deleteContainer(containerName);
-        numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
-    }
-
     ProcessResult executeCommandInContainer(ContainerName containerName, String[] command) {
         ProcessResult result = docker.executeInContainerAsRoot(containerName, command);
 
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 3ab48869c71..0dc395913bf 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
@@ -10,6 +10,7 @@ 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.maintenance.StorageMaintainer;
+import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
 import com.yahoo.vespa.hosted.node.admin.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.orchestrator.Orchestrator;
 import com.yahoo.vespa.hosted.node.admin.orchestrator.OrchestratorException;
@@ -60,11 +61,12 @@ public class NodeAgentImpl implements NodeAgent {
     private final Optional<StorageMaintainer> storageMaintainer;
     private final MetricReceiverWrapper metricReceiver;
     private final Environment environment;
+    private final Optional<AclMaintainer> aclMaintainer;
 
     private final Object monitor = new Object();
 
-    private final LinkedList<String> debugMessages = new LinkedList<>();
     private final SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
+    private final LinkedList<String> debugMessages = new LinkedList<>();
 
     private long delaysBetweenEachTickMillis;
     private int numberOfUnhandledException = 0;
@@ -91,7 +93,8 @@ public class NodeAgentImpl implements NodeAgent {
             final DockerOperations dockerOperations,
             final Optional<StorageMaintainer> storageMaintainer,
             final MetricReceiverWrapper metricReceiver,
-            final Environment environment) {
+            final Environment environment,
+            final Optional<AclMaintainer> aclMaintainer) {
         this.nodeRepository = nodeRepository;
         this.orchestrator = orchestrator;
         this.hostname = hostName;
@@ -101,6 +104,7 @@ public class NodeAgentImpl implements NodeAgent {
         this.logger = PrefixLogger.getNodeAgentLogger(NodeAgentImpl.class, containerName);
         this.metricReceiver = metricReceiver;
         this.environment = environment;
+        this.aclMaintainer = aclMaintainer;
     }
 
     @Override
@@ -236,7 +240,10 @@ public class NodeAgentImpl implements NodeAgent {
     }
 
     private void startContainerIfNeeded(final ContainerNodeSpec nodeSpec) {
-        if (dockerOperations.startContainerIfNeeded(containerName, nodeSpec)) {
+        Optional<Container> existingContainer = dockerOperations.getContainer(containerName);
+        if (!existingContainer.isPresent()) {
+            aclMaintainer.ifPresent(AclMaintainer::run);
+            dockerOperations.startContainer(containerName, nodeSpec);
             metricReceiver.unsetMetricsForContainer(hostname);
             lastCpuMetric = new CpuUsageReporter(Instant.now());
             vespaVersion = dockerOperations.getVespaVersion(containerName);
@@ -334,7 +341,7 @@ public class NodeAgentImpl implements NodeAgent {
                 stopServices(containerName);
             }
             vespaVersion = Optional.empty();
-            dockerOperations.removeContainer(nodeSpec, existingContainer.get());
+            dockerOperations.removeContainer(existingContainer.get());
             metricReceiver.unsetMetricsForContainer(hostname);
             return true;
         }
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 021192ad533..d5bc0e3826d 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
@@ -69,7 +69,7 @@ public class ComponentsProviderImpl implements ComponentsProvider {
 
         Function<String, NodeAgent> nodeAgentFactory =
                 (hostName) -> new NodeAgentImpl(hostName, nodeRepository, orchestrator, dockerOperations,
-                        storageMaintainer, metricReceiver, environment);
+                        storageMaintainer, metricReceiver, environment, aclMaintainer);
         NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer,
                 NODE_AGENT_SCAN_INTERVAL_MILLIS, metricReceiver, aclMaintainer);
         nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepository, nodeAdmin, INITIAL_SCHEDULER_DELAY_MILLIS,
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 664d375ec09..7e09a9b76e5 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
@@ -33,7 +33,7 @@ public class ComponentsProviderWithMocks implements ComponentsProvider {
     private final DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, mr);
     private final Function<String, NodeAgent> nodeAgentFactory =
             (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock,
-                    dockerOperations, Optional.empty(), mr, environment);
+                    dockerOperations, Optional.empty(), mr, environment, Optional.empty());
     private NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, Optional.empty(), 100, mr, Optional.empty());
 
 
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 5f1fa19666f..b03811fb824 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
@@ -62,7 +62,7 @@ public class DockerTester implements AutoCloseable {
         MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
         final DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, mr);
         Function<String, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock,
-                orchestratorMock, dockerOperations, Optional.of(storageMaintainer), mr, environment);
+                orchestratorMock, dockerOperations, Optional.of(storageMaintainer), mr, environment, Optional.empty());
         nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, Optional.of(storageMaintainer), 100, mr, Optional.empty());
         updater = new NodeAdminStateUpdater(nodeRepositoryMock, nodeAdmin, 1, 1, orchestratorMock, "basehostname");
     }
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 baf69167296..47e59aca4c7 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
@@ -12,6 +12,7 @@ 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.maintenance.StorageMaintainer;
+import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
 import com.yahoo.vespa.hosted.node.admin.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.orchestrator.Orchestrator;
 import com.yahoo.vespa.hosted.node.admin.util.Environment;
@@ -24,7 +25,7 @@ import org.mockito.InOrder;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
@@ -58,6 +59,9 @@ public class NodeAgentImplTest {
     private final Orchestrator orchestrator = mock(Orchestrator.class);
     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(),
+            Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap());
 
     private final PathResolver pathResolver = mock(PathResolver.class);
     private final Environment environment = new Environment.Builder()
@@ -68,7 +72,7 @@ public class NodeAgentImplTest {
             .pathResolver(pathResolver).build();
 
     private final NodeAgentImpl nodeAgent = new NodeAgentImpl(hostName, nodeRepository, orchestrator, dockerOperations,
-            Optional.of(storageMaintainer), metricReceiver, environment);
+            Optional.of(storageMaintainer), metricReceiver, environment, Optional.of(aclMaintainer));
 
     @Test
     public void upToDateContainerIsUntouched() throws Exception {
@@ -90,19 +94,17 @@ public class NodeAgentImplTest {
                 .minDiskAvailableGb(MIN_DISK_AVAILABLE_GB)
                 .build();
 
-        Docker.ContainerStats containerStats = new ContainerStatsImpl(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>());
         when(dockerOperations.getContainer(eq(containerName))).thenReturn(
                 Optional.of(new Container(hostName, dockerImage, containerName, Container.State.RUNNING, 1)));
-        when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
+        when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(emptyContainerStats));
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(false);
-        when(dockerOperations.startContainerIfNeeded(eq(containerName), eq(nodeSpec))).thenReturn(false);
         when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec));
 
         nodeAgent.vespaVersion = nodeSpec.vespaVersion;
 
         nodeAgent.tick();
 
-        verify(dockerOperations, never()).removeContainer(any(), any());
+        verify(dockerOperations, never()).removeContainer(any());
         verify(orchestrator, never()).suspend(any(String.class));
         verify(dockerOperations, never()).scheduleDownloadOfImage(eq(containerName), any(), any());
 
@@ -143,18 +145,19 @@ public class NodeAgentImplTest {
         when(dockerOperations.getContainer(eq(containerName))).thenReturn(Optional.empty());
         when(dockerOperations.getContainerStats(eq(containerName))).thenReturn(Optional.empty());
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(false);
-        when(dockerOperations.startContainerIfNeeded(eq(containerName), eq(nodeSpec))).thenReturn(true);
         when(dockerOperations.getVespaVersion(eq(containerName))).thenReturn(Optional.of(vespaVersion));
         when(pathResolver.getApplicationStoragePathForNodeAdmin()).thenReturn(Files.createTempDirectory("foo"));
         when(nodeRepository.getContainerNodeSpec(hostName)).thenReturn(Optional.of(nodeSpec));
 
         nodeAgent.tick();
 
-        verify(dockerOperations, never()).removeContainer(any(), any());
+        verify(dockerOperations, never()).removeContainer(any());
         verify(orchestrator, never()).suspend(any(String.class));
         verify(dockerOperations, never()).scheduleDownloadOfImage(eq(containerName), any(), any());
 
-        final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository);
+        final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository, aclMaintainer);
+        inOrder.verify(aclMaintainer, times(1)).run();
+        inOrder.verify(dockerOperations, times(1)).startContainer(eq(containerName), eq(nodeSpec));
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(containerName));
         inOrder.verify(nodeRepository).updateNodeAttributes(
                 hostName, new NodeAttributes()
@@ -193,7 +196,7 @@ public class NodeAgentImplTest {
 
         verify(orchestrator, never()).suspend(any(String.class));
         verify(orchestrator, never()).resume(any(String.class));
-        verify(dockerOperations, never()).removeContainer(any(), any());
+        verify(dockerOperations, never()).removeContainer(any());
 
         final InOrder inOrder = inOrder(dockerOperations);
         inOrder.verify(dockerOperations, times(1)).shouldScheduleDownloadOfImage(eq(newDockerImage));
@@ -219,13 +222,14 @@ public class NodeAgentImplTest {
                 .build();
 
         when(dockerOperations.shouldScheduleDownloadOfImage(any())).thenReturn(false);
+        when(dockerOperations.getContainerStats(eq(containerName))).thenReturn(Optional.empty());
 
         try {
             nodeAgent.tick();
             fail("Expected to throw an exception");
         } catch (Exception ignored) { }
 
-        verify(dockerOperations, never()).startContainerIfNeeded(eq(containerName), eq(nodeSpec));
+        verify(dockerOperations, never()).startContainer(eq(containerName), eq(nodeSpec));
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository, never()).updateNodeAttributes(any(String.class), any(NodeAttributes.class));
     }
@@ -255,7 +259,7 @@ public class NodeAgentImplTest {
 
         nodeAgent.tick();
 
-        verify(dockerOperations, never()).removeContainer(any(), any());
+        verify(dockerOperations, never()).removeContainer(any());
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository).updateNodeAttributes(
                 hostName, new NodeAttributes()
@@ -291,8 +295,8 @@ public class NodeAgentImplTest {
 
         nodeAgent.tick();
 
-        verify(dockerOperations, never()).removeContainer(any(), any());
-        verify(dockerOperations, never()).startContainerIfNeeded(eq(containerName), eq(nodeSpec));
+        verify(dockerOperations, never()).removeContainer(any());
+        verify(dockerOperations, never()).startContainer(eq(containerName), eq(nodeSpec));
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository).updateNodeAttributes(
                 hostName, new NodeAttributes()
@@ -330,7 +334,7 @@ public class NodeAgentImplTest {
 
         final InOrder inOrder = inOrder(storageMaintainer, dockerOperations);
         inOrder.verify(storageMaintainer, times(1)).removeOldFilesFromNode(eq(containerName));
-        inOrder.verify(dockerOperations, never()).removeContainer(eq(nodeSpec), any());
+        inOrder.verify(dockerOperations, never()).removeContainer(any());
 
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository).updateNodeAttributes(
@@ -378,11 +382,11 @@ public class NodeAgentImplTest {
 
         final InOrder inOrder = inOrder(storageMaintainer, dockerOperations, nodeRepository);
         inOrder.verify(storageMaintainer, times(1)).removeOldFilesFromNode(eq(containerName));
-        inOrder.verify(dockerOperations, times(1)).removeContainer(eq(nodeSpec), any());
+        inOrder.verify(dockerOperations, times(1)).removeContainer(any());
         inOrder.verify(storageMaintainer, times(1)).archiveNodeData(eq(containerName));
         inOrder.verify(nodeRepository, times(1)).markAsReady(eq(hostName));
 
-        verify(dockerOperations, never()).startContainerIfNeeded(eq(containerName), any());
+        verify(dockerOperations, never()).startContainer(eq(containerName), any());
         verify(orchestrator, never()).resume(any(String.class));
         // current Docker image and vespa version should be cleared
         verify(nodeRepository, times(1)).updateNodeAttributes(
@@ -427,14 +431,13 @@ public class NodeAgentImplTest {
                 .minDiskAvailableGb(MIN_DISK_AVAILABLE_GB)
                 .build();
 
-        Docker.ContainerStats containerStats = new ContainerStatsImpl(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>());
-        when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(containerStats));
+        when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(emptyContainerStats));
         when(dockerOperations.getContainer(eq(containerName))).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);
 
-        verify(dockerOperations, never()).removeContainer(any(), any());
+        verify(dockerOperations, never()).removeContainer(any());
 
         doThrow(new RuntimeException("Failed 1st time"))
                 .doNothing()