Merge pull request #4639 from vespa-engine/smorgrav/use_nat_network

Preliminary NAT implementation for docker

10 files changed, 153 insertions, 42 deletions

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 bc94c39d135..00493e3e016 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
@@ -45,6 +45,10 @@ public interface Docker {
         Map<String, Object> getBlkioStats();
     }
 
+    default boolean networkNATed() {
+        return false;
+    }
+
     Optional<ContainerStats> getContainerStats(ContainerName containerName);
     
     void startContainer(ContainerName containerName);
@@ -113,5 +117,5 @@ public interface Docker {
      */
     ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... command);
 
-
+    String getGlobalIPv6Address(ContainerName name);
 }
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 fa093e0b4dc..15e88f4f253 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
@@ -4,6 +4,7 @@ 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.InspectContainerCmd;
 import com.github.dockerjava.api.command.InspectContainerResponse;
 import com.github.dockerjava.api.command.InspectExecResponse;
 import com.github.dockerjava.api.command.InspectImageResponse;
@@ -125,15 +126,23 @@ public class DockerImpl implements Docker {
                 Duration minAgeToDelete = Duration.ofMinutes(config.imageGCMinTimeToLiveMinutes());
                 dockerImageGC = Optional.of(new DockerImageGarbageCollector(minAgeToDelete));
 
-                try {
-                    setupDockerNetworkIfNeeded();
-                } catch (Exception e) {
-                    throw new DockerException("Could not setup docker network", e);
+
+                if (!config.networkNATed()) {
+                    try {
+                        setupDockerNetworkIfNeeded();
+                    } catch (Exception e) {
+                        throw new DockerException("Could not setup docker network", e);
+                    }
                 }
             }
         }
     }
 
+    @Override
+    public boolean networkNATed() {
+        return config.networkNATed();
+    }
+
     static DefaultDockerClientConfig.Builder buildDockerClientConfig(DockerConfig config) {
         DefaultDockerClientConfig.Builder dockerConfigBuilder = new DefaultDockerClientConfig.Builder()
                 .withDockerHost(config.uri());
@@ -393,6 +402,12 @@ public class DockerImpl implements Docker {
         return asContainer(containerName.asString()).findFirst();
     }
 
+    @Override
+    public String getGlobalIPv6Address(ContainerName name) {
+        InspectContainerCmd cmd = dockerClient.inspectContainerCmd(name.asString());
+        return cmd.exec().getNetworkSettings().getGlobalIPv6Address();
+    }
+
     private Stream<Container> asContainer(String container) {
         return inspectContainerCmd(container)
                 .map(response ->
diff --git a/docker-api/src/main/resources/configdefinitions/docker.def b/docker-api/src/main/resources/configdefinitions/docker.def
index 5c6e52b2f63..b4585318cd8 100644
--- a/docker-api/src/main/resources/configdefinitions/docker.def
+++ b/docker-api/src/main/resources/configdefinitions/docker.def
@@ -14,3 +14,5 @@ readTimeoutMillis int default = 1800000 # 30 min
 
 isRunningLocally bool default = false
 imageGCMinTimeToLiveMinutes int default = 45
+
+networkNATed bool default = false
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 1d11e221a9b..80978d30da5 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
@@ -15,7 +15,7 @@ public interface DockerOperations {
 
     void startContainer(ContainerName containerName, ContainerNodeSpec nodeSpec);
 
-    void removeContainer(Container existingContainer);
+    void removeContainer(Container existingContainer, ContainerNodeSpec nodeSpec);
 
     Optional<Container> getContainer(ContainerName containerName);
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index d2863468ee7..88f5c9acfed 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
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.dockerapi.DockerImpl;
 import com.yahoo.vespa.hosted.dockerapi.DockerNetworkCreator;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
+import com.yahoo.vespa.hosted.node.admin.maintenance.acl.iptables.NATCommand;
 import com.yahoo.vespa.hosted.node.admin.util.Environment;
 import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 
@@ -104,14 +105,13 @@ public class DockerOperationsImpl implements DockerOperations {
             String configServers = environment.getConfigServerUris().stream()
                     .map(URI::getHost)
                     .collect(Collectors.joining(","));
+
             Docker.CreateContainerCommand command = docker.createContainerCommand(
                     nodeSpec.wantedDockerImage.get(),
                     ContainerResources.from(nodeSpec.minCpuCores, nodeSpec.minMainMemoryAvailableGb),
                     containerName,
                     nodeSpec.hostname)
                     .withManagedBy(MANAGER_NAME)
-                    .withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME)
-                    .withIpAddress(nodeInetAddress)
                     .withEnvironment("CONFIG_SERVER_ADDRESS", configServers)
                     .withUlimit("nofile", 262_144, 262_144)
                     .withUlimit("nproc", 32_768, 409_600)
@@ -119,7 +119,13 @@ public class DockerOperationsImpl implements DockerOperations {
                     .withAddCapability("SYS_PTRACE") // Needed for gcore, pstack etc.
                     .withAddCapability("SYS_ADMIN"); // Needed for perf
 
-            command.withVolume("/etc/hosts", "/etc/hosts");
+            if (!docker.networkNATed()) {
+                logger.info("Network not nated - setting up with specific ip address on a macvlan");
+                command.withIpAddress(nodeInetAddress);
+                command.withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME);
+                command.withVolume("/etc/hosts", "/etc/hosts"); // TODO This is probably not nessesary - review later
+            }
+
             for (String pathInNode : DIRECTORIES_TO_MOUNT.keySet()) {
                 String pathInHost = environment.pathInHostFromPathInNode(containerName, pathInNode).toString();
                 command.withVolume(pathInHost, pathInNode);
@@ -137,11 +143,17 @@ public class DockerOperationsImpl implements DockerOperations {
             command.create();
 
             if (isIPv6) {
-                docker.connectContainerToNetwork(containerName, "bridge");
+                if (!docker.networkNATed()) {
+                    docker.connectContainerToNetwork(containerName, "bridge");
+                }
+
                 docker.startContainer(containerName);
-                setupContainerNetworkingWithScript(containerName);
+                setupContainerNetworkConnectivity(containerName, nodeInetAddress);
             } else {
                 docker.startContainer(containerName);
+                if (docker.networkNATed()) {
+                    setupContainerNetworkConnectivity(containerName, nodeInetAddress);
+                }
             }
 
             DIRECTORIES_TO_MOUNT.entrySet().stream().filter(Map.Entry::getValue).forEach(entry ->
@@ -152,7 +164,7 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
-    public void removeContainer(final Container existingContainer) {
+    public void removeContainer(final Container existingContainer, ContainerNodeSpec nodeSpec) {
         final ContainerName containerName = existingContainer.name;
         PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
         if (existingContainer.state.isRunning()) {
@@ -162,6 +174,23 @@ public class DockerOperationsImpl implements DockerOperations {
 
         logger.info("Deleting container " + containerName.asString());
         docker.deleteContainer(containerName);
+
+        if (docker.networkNATed()) {
+            logger.info("Delete iptables NAT rules for " + containerName.asString());
+            try {
+                InetAddress nodeInetAddress = environment.getInetAddressForHost(nodeSpec.hostname);
+                String ipv6Str = docker.getGlobalIPv6Address(containerName);
+                String drop = NATCommand.drop(nodeInetAddress, InetAddress.getByName(ipv6Str));
+                Pair<Integer, String> result = processExecuter.exec(drop);
+                if (result.getFirst() != 0) {
+                    // Might be because the one or two (out of three) rules where not present - debug log and ignore
+                    // Trailing rules will always be overridden by a new one due the fact that we insert
+                    logger.debug("Unable to drop NAT rule - error message: " + result.getSecond());
+                }
+            } catch (IOException e) {
+                logger.warning("Unable to drop NAT rule for container " + containerName, e);
+            }
+        }
     }
 
     @Override
@@ -191,13 +220,21 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     /**
+     * For macvlan:
      * Due to a bug in docker (https://github.com/docker/libnetwork/issues/1443), we need to manually set
      * IPv6 gateway in containers connected to more than one docker network
+     *
+     * For nat:
+     * Setup iptables NAT rules to map the hosts public ips to the containers
      */
-    private void setupContainerNetworkingWithScript(ContainerName containerName) throws IOException {
-        InetAddress hostDefaultGateway = DockerNetworkCreator.getDefaultGatewayLinux(true);
-        executeCommandInNetworkNamespace(containerName,
-                "route", "-A", "inet6", "add", "default", "gw", hostDefaultGateway.getHostAddress(), "dev", "eth1");
+    private void setupContainerNetworkConnectivity(ContainerName containerName, InetAddress externalAddress) throws IOException {
+        if (docker.networkNATed()) {
+            insertNAT(containerName, externalAddress);
+        } else {
+            InetAddress hostDefaultGateway = DockerNetworkCreator.getDefaultGatewayLinux(true);
+            executeCommandInNetworkNamespace(containerName,
+                    "route", "-A", "inet6", "add", "default", "gw", hostDefaultGateway.getHostAddress(), "dev", "eth1");
+        }
     }
 
     @Override
@@ -289,4 +326,25 @@ public class DockerOperationsImpl implements DockerOperations {
     public void deleteUnusedDockerImages() {
         docker.deleteUnusedDockerImages();
     }
+
+    /**
+     * Only insert NAT rules if they don't exist (or else they compounded)
+     */
+    private void insertNAT(ContainerName containerName, InetAddress externalAddress) throws IOException {
+        PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
+        String ipv6Str = docker.getGlobalIPv6Address(containerName);
+
+        // Check if exist
+        String checkCommand = NATCommand.check(externalAddress, InetAddress.getByName(ipv6Str));
+        Pair<Integer, String> result = processExecuter.exec(checkCommand);
+        if (result.getFirst() == 0 ) return;
+
+        // Setup NAT
+        String natCommand = NATCommand.insert(externalAddress, InetAddress.getByName(ipv6Str));
+        logger.info("Setting up NAT rules: " + natCommand);
+        result = processExecuter.exec(checkCommand);
+        if (result.getFirst() != 0 ) {
+            throw new IOException("Unable to setup NAT rule - error message: " + result.getSecond());
+        }
+    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java
index 87bb5fddf23..7b380a0afd3 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java
@@ -5,10 +5,12 @@ import java.net.Inet6Address;
 import java.net.InetAddress;
 
 /**
- * Creates two commands that:
+ * Creates three ip(6)tables commands customized to map docker containers on a private net to their
+ * respective public ip addresses that all are assign to the host
  *
- *  1. replaces an external/public destination ip to an internal/private ip before routing it (pre-routing)
- *  2. replaces an internal/private source ip to an external/public ip before writing it on the wire (post-routing)
+ *  1. DNAT PRE-ROUTING: replaces an external/public destination ip to an internal/private ip when it arrives on an interface
+ *  2. DNAT LOOPBACK OUTPUT: replaces an internal/public ip destination to an internal/private ip on the loopback device (for host to container communication)
+ *  3. SNAT POST-ROUTING: replaces an internal/private source ip to an external/public ip before writing it on the wire
  *
  * @author smorgrav
  */
@@ -16,27 +18,48 @@ public class NATCommand implements Command {
 
     private final String snatCommand;
     private final String dnatCommand;
+    private final String dnatLoopBackCommand;
 
-    NATCommand(InetAddress externalIp, InetAddress internalIp, String iface) {
+    public NATCommand(InetAddress externalIp, InetAddress internalIp, String chainCommand) {
         String command = externalIp instanceof Inet6Address ? "ip6tables" : "iptables";
-        this.snatCommand = String.format("%s -t nat -A POSTROUTING -o %s -s %s -j SNAT --to %s",
+        this.snatCommand = String.format("%s -t nat %s POSTROUTING -s %s -j SNAT --to %s",
                 command,
-                iface,
+                chainCommand,
                 internalIp.getHostAddress(),
                 externalIp.getHostAddress());
-
-        this.dnatCommand = String.format("%s -t nat -A PREROUTING -i %s -d %s -j DNAT --to-destination %s",
+        this.dnatLoopBackCommand = String.format("%s -t nat %s OUTPUT -o lo -d %s -j DNAT --to-destination %s",
+                command,
+                chainCommand,
+                externalIp.getHostAddress(),
+                internalIp.getHostAddress());
+        this.dnatCommand = String.format("%s -t nat %s PREROUTING -d %s -j DNAT --to-destination %s",
                 command,
-                iface,
+                chainCommand,
                 externalIp.getHostAddress(),
                 internalIp.getHostAddress());
     }
 
     @Override
     public String asString() {
-        return snatCommand + "; " + dnatCommand;
+        return concat("&&");
     }
 
     @Override
     public String asString(String commandName) { return asString(); }
+
+    private String concat(String delimiter) {
+        return String.join(delimiter, snatCommand, dnatCommand, dnatLoopBackCommand);
+    }
+
+    public static String insert(InetAddress externalIp, InetAddress internalIp) {
+        return new NATCommand(externalIp, internalIp, "-I").concat(" && ");
+    }
+
+    public static String drop(InetAddress externalIp, InetAddress internalIp) {
+        return new NATCommand(externalIp, internalIp, "-D").concat("; ");
+    }
+
+    public static String check(InetAddress externalIp, InetAddress internalIp) {
+        return new NATCommand(externalIp, internalIp, "-C").concat(" && ");
+    }
 }
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 8ca3ace92e5..32f6186707a 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
@@ -345,7 +345,7 @@ public class NodeAgentImpl implements NodeAgent {
                 }
             }
             if (currentFilebeatRestarter != null) currentFilebeatRestarter.cancel(true);
-            dockerOperations.removeContainer(existingContainer);
+            dockerOperations.removeContainer(existingContainer, nodeSpec);
             containerState = ABSENT;
             logger.info("Container successfully removed, new containerState is " + containerState);
             return Optional.empty();
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 7a5d713936d..8a4c4fd8c88 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
@@ -162,6 +162,11 @@ public class DockerMock implements Docker {
     }
 
 
+    @Override
+    public String getGlobalIPv6Address(ContainerName name) {
+        return "2001:db8:1:2:0:242:ac13:2";
+    }
+
     public static class StartContainerCommandMock implements CreateContainerCommand {
         @Override
         public CreateContainerCommand withLabel(String name, String value) {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java
index c2a2575f6b1..b0df67b7b46 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java
@@ -20,19 +20,23 @@ public class NATCommandTest {
     public void sampleNATCommandIPv6() throws UnknownHostException{
         InetAddress externalIP = Inet6Address.getByName("2001:db8::1");
         InetAddress internalIP = Inet6Address.getByName("2001:db8::2");
-        String iface = "eth0";
 
-        NATCommand command = new NATCommand(externalIP, internalIP, iface);
-        Assert.assertEquals("ip6tables -t nat -A POSTROUTING -o eth0 -s 2001:db8:0:0:0:0:0:2 -j SNAT --to 2001:db8:0:0:0:0:0:1; ip6tables -t nat -A PREROUTING -i eth0 -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2", command.asString());
+        String insert = NATCommand.insert(externalIP, internalIP);
+        Assert.assertEquals("ip6tables -t nat -I POSTROUTING -s 2001:db8:0:0:0:0:0:2 -j SNAT --to 2001:db8:0:0:0:0:0:1 && ip6tables -t nat -I PREROUTING -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2 && ip6tables -t nat -I OUTPUT -o lo -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2", insert);
+
+        String drop = NATCommand.drop(externalIP, internalIP);
+        Assert.assertEquals("ip6tables -t nat -D POSTROUTING -s 2001:db8:0:0:0:0:0:2 -j SNAT --to 2001:db8:0:0:0:0:0:1; ip6tables -t nat -D PREROUTING -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2; ip6tables -t nat -D OUTPUT -o lo -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2", drop);
     }
 
     @Test
     public void sampleNATCommandIPv4() throws UnknownHostException{
         InetAddress externalIP = Inet4Address.getByName("192.168.0.1");
         InetAddress internalIP = Inet4Address.getByName("192.168.0.2");
-        String iface = "eth0";
 
-        NATCommand command = new NATCommand(externalIP, internalIP, iface);
-        Assert.assertEquals("iptables -t nat -A POSTROUTING -o eth0 -s 192.168.0.2 -j SNAT --to 192.168.0.1; iptables -t nat -A PREROUTING -i eth0 -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2", command.asString());
+        String insert = NATCommand.insert(externalIP, internalIP);
+        Assert.assertEquals("iptables -t nat -I POSTROUTING -s 192.168.0.2 -j SNAT --to 192.168.0.1 && iptables -t nat -I PREROUTING -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2 && iptables -t nat -I OUTPUT -o lo -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2", insert);
+
+        String drop = NATCommand.drop(externalIP, internalIP);
+        Assert.assertEquals("iptables -t nat -D POSTROUTING -s 192.168.0.2 -j SNAT --to 192.168.0.1; iptables -t nat -D PREROUTING -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2; iptables -t nat -D OUTPUT -o lo -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2", drop);
     }
 }
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 cce000f858a..8067bf9ba69 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
@@ -115,7 +115,7 @@ public class NodeAgentImplTest {
 
         nodeAgent.converge();
 
-        verify(dockerOperations, never()).removeContainer(any());
+        verify(dockerOperations, never()).removeContainer(any(), any());
         verify(orchestrator, never()).suspend(any(String.class));
         verify(dockerOperations, never()).pullImageAsyncIfNeeded(any());
         verify(storageMaintainer, never()).removeOldFilesFromNode(eq(containerName));
@@ -181,7 +181,7 @@ public class NodeAgentImplTest {
 
         nodeAgent.converge();
 
-        verify(dockerOperations, never()).removeContainer(any());
+        verify(dockerOperations, never()).removeContainer(any(), any());
         verify(orchestrator, never()).suspend(any(String.class));
 
         final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository, aclMaintainer);
@@ -223,7 +223,7 @@ public class NodeAgentImplTest {
 
         verify(orchestrator, never()).suspend(any(String.class));
         verify(orchestrator, never()).resume(any(String.class));
-        verify(dockerOperations, never()).removeContainer(any());
+        verify(dockerOperations, never()).removeContainer(any(), any());
 
         final InOrder inOrder = inOrder(dockerOperations);
         inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(newDockerImage));
@@ -262,7 +262,7 @@ public class NodeAgentImplTest {
         inOrder.verify(orchestrator).resume(any(String.class));
         inOrder.verify(orchestrator).resume(any(String.class));
         inOrder.verify(orchestrator).suspend(any(String.class));
-        inOrder.verify(dockerOperations).removeContainer(any());
+        inOrder.verify(dockerOperations).removeContainer(any(), any());
         inOrder.verify(dockerOperations).startContainer(eq(containerName), eq(thirdSpec));
         inOrder.verify(orchestrator).resume(any(String.class));
     }
@@ -313,7 +313,7 @@ public class NodeAgentImplTest {
 
         nodeAgent.converge();
 
-        verify(dockerOperations, never()).removeContainer(any());
+        verify(dockerOperations, never()).removeContainer(any(), any());
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository).updateNodeAttributes(
                 hostName, new NodeAttributes()
@@ -344,7 +344,7 @@ public class NodeAgentImplTest {
 
         // Should only be called once, when we initialize
         verify(dockerOperations, times(1)).getContainer(eq(containerName));
-        verify(dockerOperations, never()).removeContainer(any());
+        verify(dockerOperations, never()).removeContainer(any(), any());
         verify(dockerOperations, never()).startContainer(eq(containerName), eq(nodeSpec));
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository).updateNodeAttributes(
@@ -378,7 +378,7 @@ public class NodeAgentImplTest {
         nodeAgent.converge();
 
         final InOrder inOrder = inOrder(storageMaintainer, dockerOperations);
-        inOrder.verify(dockerOperations, never()).removeContainer(any());
+        inOrder.verify(dockerOperations, never()).removeContainer(any(), any());
 
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository).updateNodeAttributes(
@@ -435,7 +435,7 @@ public class NodeAgentImplTest {
         nodeAgent.converge();
 
         final InOrder inOrder = inOrder(storageMaintainer, dockerOperations, nodeRepository);
-        inOrder.verify(dockerOperations, times(1)).removeContainer(any());
+        inOrder.verify(dockerOperations, times(1)).removeContainer(any(), any());
         inOrder.verify(storageMaintainer, times(1)).cleanupNodeStorage(eq(containerName), eq(nodeSpec));
         inOrder.verify(nodeRepository, times(1)).markNodeAvailableForNewAllocation(eq(hostName));
 
@@ -492,7 +492,7 @@ public class NodeAgentImplTest {
 
         nodeAgent.tick();
 
-        verify(dockerOperations, times(1)).removeContainer(any());
+        verify(dockerOperations, times(1)).removeContainer(any(), any());
         verify(dockerOperations, times(1)).startContainer(eq(containerName), eq(nodeSpec));
     }