Merge pull request #15035 from vespa-engine/mpolden/registry-credentials

Add support for registry credentials in container engine

12 files changed, 142 insertions, 34 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerEngine.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerEngine.java
index 984e1261d63..cd5f208e9e0 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerEngine.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerEngine.java
@@ -79,7 +79,7 @@ public interface ContainerEngine {
      * @param image Docker image to pull
      * @return true iff image being pulled, false otherwise
      */
-    boolean pullImageAsyncIfNeeded(DockerImage image);
+    boolean pullImageAsyncIfNeeded(DockerImage image, RegistryCredentials registryCredentials);
 
     boolean noManagedContainersRunning(String manager);
 
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java
index 0322059745d..7d63c66131d 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerEngine.java
@@ -10,6 +10,7 @@ import com.github.dockerjava.api.command.UpdateContainerCmd;
 import com.github.dockerjava.api.exception.DockerClientException;
 import com.github.dockerjava.api.exception.NotFoundException;
 import com.github.dockerjava.api.exception.NotModifiedException;
+import com.github.dockerjava.api.model.AuthConfig;
 import com.github.dockerjava.api.model.HostConfig;
 import com.github.dockerjava.api.model.Image;
 import com.github.dockerjava.api.model.Statistics;
@@ -72,7 +73,7 @@ public class DockerEngine implements ContainerEngine {
     }
 
     @Override
-    public boolean pullImageAsyncIfNeeded(DockerImage image) {
+    public boolean pullImageAsyncIfNeeded(DockerImage image, RegistryCredentials registryCredentials) {
         try {
             synchronized (monitor) {
                 if (scheduledPulls.contains(image)) return true;
@@ -81,7 +82,14 @@ public class DockerEngine implements ContainerEngine {
                 scheduledPulls.add(image);
 
                 logger.log(Level.INFO, "Starting download of " + image.asString());
-
+                if (!registryCredentials.equals(RegistryCredentials.none)) {
+                    AuthConfig authConfig = new AuthConfig().withUsername(registryCredentials.username())
+                                                            .withPassword(registryCredentials.password())
+                                                            .withRegistryAddress(registryCredentials.registryAddress());
+                    dockerClient.authCmd()
+                                .withAuthConfig(authConfig)
+                                .exec();
+                }
                 dockerClient.pullImageCmd(image.asString()).exec(new ImagePullCallback(image));
                 return true;
             }
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/RegistryCredentials.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/RegistryCredentials.java
new file mode 100644
index 00000000000..39a000a633f
--- /dev/null
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/RegistryCredentials.java
@@ -0,0 +1,57 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi;
+
+import java.util.Objects;
+
+/**
+ * Credentials for a container registry server.
+ *
+ * @author mpolden
+ */
+public class RegistryCredentials {
+
+    public static final RegistryCredentials none = new RegistryCredentials("", "", "");
+
+    private final String username;
+    private final String password;
+    private final String registryAddress;
+
+    public RegistryCredentials(String username, String password, String registryAddress) {
+        this.username = Objects.requireNonNull(username);
+        this.password = Objects.requireNonNull(password);
+        this.registryAddress = Objects.requireNonNull(registryAddress);
+    }
+
+    public String username() {
+        return username;
+    }
+
+    public String password() {
+        return password;
+    }
+
+    public String registryAddress() {
+        return registryAddress;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        RegistryCredentials that = (RegistryCredentials) o;
+        return username.equals(that.username) &&
+               password.equals(that.password) &&
+               registryAddress.equals(that.registryAddress);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(username, password, registryAddress);
+    }
+
+    @Override
+    public String toString() {
+        return "registry credentials for " + registryAddress + " [username=" + username + ",password=" + password + "]";
+    }
+
+}
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java
index 792955ed130..69055e6402c 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerEngineTest.java
@@ -93,12 +93,12 @@ public class DockerEngineTest {
         when(dockerClient.inspectImageCmd(image.asString())).thenReturn(imageInspectCmd);
         when(dockerClient.pullImageCmd(eq(image.asString()))).thenReturn(pullImageCmd);
 
-        assertTrue("Should return true, we just scheduled the pull", docker.pullImageAsyncIfNeeded(image));
-        assertTrue("Should return true, the pull i still ongoing", docker.pullImageAsyncIfNeeded(image));
+        assertTrue("Should return true, we just scheduled the pull", docker.pullImageAsyncIfNeeded(image, RegistryCredentials.none));
+        assertTrue("Should return true, the pull i still ongoing", docker.pullImageAsyncIfNeeded(image, RegistryCredentials.none));
 
         assertTrue(docker.imageIsDownloaded(image));
         resultCallback.getValue().onComplete();
-        assertFalse(docker.pullImageAsyncIfNeeded(image));
+        assertFalse(docker.pullImageAsyncIfNeeded(image, RegistryCredentials.none));
     }
 
     @Test
@@ -117,15 +117,15 @@ public class DockerEngineTest {
         when(dockerClient.inspectImageCmd(image.asString())).thenReturn(imageInspectCmd);
         when(dockerClient.pullImageCmd(eq(image.asString()))).thenReturn(pullImageCmd);
 
-        assertTrue("Should return true, we just scheduled the pull", docker.pullImageAsyncIfNeeded(image));
-        assertTrue("Should return true, the pull i still ongoing", docker.pullImageAsyncIfNeeded(image));
+        assertTrue("Should return true, we just scheduled the pull", docker.pullImageAsyncIfNeeded(image, RegistryCredentials.none));
+        assertTrue("Should return true, the pull i still ongoing", docker.pullImageAsyncIfNeeded(image, RegistryCredentials.none));
 
         try {
             resultCallback.getValue().onComplete();
         } catch (Exception ignored) { }
 
         assertFalse(docker.imageIsDownloaded(image));
-        assertTrue("Should return true, new pull scheduled", docker.pullImageAsyncIfNeeded(image));
+        assertTrue("Should return true, new pull scheduled", docker.pullImageAsyncIfNeeded(image, RegistryCredentials.none));
     }
 
 }
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index ecf95717624..dd6d84e3ad7 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -376,6 +376,12 @@ public class Flags {
             "Takes effect on next internal redeployment",
             APPLICATION_ID);
 
+    public static final UnboundBooleanFlag REGIONAL_CONTAINER_REGISTRY = defineFeatureFlag(
+            "regional-container-registry",
+            false,
+            "Whether host-admin should download images from the zone's regional container registry",
+            "Takes effect on host-admin restart");
+
     /** WARNING: public for testing: All flags should be defined in {@link Flags}. */
     public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description,
                                                        String modificationEffect, FetchVector.Dimension... dimensions) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java
index 9e6b6200b8c..8d62196d092 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java
@@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
 import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
+import com.yahoo.vespa.hosted.dockerapi.RegistryCredentials;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult;
@@ -14,6 +15,9 @@ import java.time.Duration;
 import java.util.List;
 import java.util.Optional;
 
+/**
+ * @author hakonhall
+ */
 public interface ContainerOperations {
 
     void createContainer(NodeAgentContext context, ContainerData containerData, ContainerResources containerResources);
@@ -26,7 +30,7 @@ public interface ContainerOperations {
 
     Optional<Container> getContainer(NodeAgentContext context);
 
-    boolean pullImageAsyncIfNeeded(DockerImage dockerImage);
+    boolean pullImageAsyncIfNeeded(DockerImage dockerImage, RegistryCredentials registryCredentials);
 
     ProcessResult executeCommandInContainerAsRoot(NodeAgentContext context, String... command);
 
@@ -57,4 +61,5 @@ public interface ContainerOperations {
 
     /** Deletes the local images that are currently not in use by any container and not recently used. */
     boolean deleteUnusedContainerImages(List<DockerImage> excludes, Duration minImageAgeToDelete);
+
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java
index 9d53730cfc9..a6f2184179e 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java
@@ -11,6 +11,7 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerEngine;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
 import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
+import com.yahoo.vespa.hosted.dockerapi.RegistryCredentials;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
@@ -209,8 +210,8 @@ public class ContainerOperationsImpl implements ContainerOperations {
     }
 
     @Override
-    public boolean pullImageAsyncIfNeeded(DockerImage dockerImage) {
-        return containerEngine.pullImageAsyncIfNeeded(dockerImage);
+    public boolean pullImageAsyncIfNeeded(DockerImage dockerImage, RegistryCredentials registryCredentials) {
+        return containerEngine.pullImageAsyncIfNeeded(dockerImage, registryCredentials);
     }
 
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/RegistryCredentialsProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/RegistryCredentialsProvider.java
new file mode 100644
index 00000000000..09f0ea77e47
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/RegistryCredentialsProvider.java
@@ -0,0 +1,15 @@
+// Copyright Verizon Media. 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.vespa.hosted.dockerapi.RegistryCredentials;
+
+/**
+ * Interface for retrieving credentials for a container registry.
+ *
+ * @author mpolden
+ */
+public interface RegistryCredentialsProvider {
+
+    RegistryCredentials get();
+
+}
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 f09598a6fb0..95dfad4c6d7 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
@@ -5,13 +5,13 @@ import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.zone.ZoneApi;
-import java.util.logging.Level;
 import com.yahoo.vespa.flags.DoubleFlag;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
+import com.yahoo.vespa.hosted.dockerapi.RegistryCredentials;
 import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException;
 import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
@@ -21,6 +21,7 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorException;
 import com.yahoo.vespa.hosted.node.admin.docker.ContainerOperations;
+import com.yahoo.vespa.hosted.node.admin.docker.RegistryCredentialsProvider;
 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.maintenance.identity.CredentialsMaintainer;
@@ -36,6 +37,7 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl.ContainerState.ABSENT;
@@ -58,6 +60,7 @@ public class NodeAgentImpl implements NodeAgent {
     private final NodeRepository nodeRepository;
     private final Orchestrator orchestrator;
     private final ContainerOperations containerOperations;
+    private final RegistryCredentialsProvider registryCredentialsProvider;
     private final StorageMaintainer storageMaintainer;
     private final Optional<CredentialsMaintainer> credentialsMaintainer;
     private final Optional<AclMaintainer> aclMaintainer;
@@ -97,21 +100,26 @@ public class NodeAgentImpl implements NodeAgent {
 
     // Created in NodeAdminImpl
     public NodeAgentImpl(NodeAgentContextSupplier contextSupplier, NodeRepository nodeRepository,
-                         Orchestrator orchestrator, ContainerOperations containerOperations, StorageMaintainer storageMaintainer,
+                         Orchestrator orchestrator, ContainerOperations containerOperations,
+                         RegistryCredentialsProvider registryCredentialsProvider, StorageMaintainer storageMaintainer,
                          FlagSource flagSource, Optional<CredentialsMaintainer> credentialsMaintainer,
                          Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Clock clock) {
-        this(contextSupplier, nodeRepository, orchestrator, containerOperations, storageMaintainer, flagSource, credentialsMaintainer,
-             aclMaintainer, healthChecker, clock, DEFAULT_WARM_UP_DURATION);
+        this(contextSupplier, nodeRepository, orchestrator, containerOperations, registryCredentialsProvider,
+             storageMaintainer, flagSource, credentialsMaintainer, aclMaintainer, healthChecker, clock,
+             DEFAULT_WARM_UP_DURATION);
     }
 
     public NodeAgentImpl(NodeAgentContextSupplier contextSupplier, NodeRepository nodeRepository,
-                         Orchestrator orchestrator, ContainerOperations containerOperations, StorageMaintainer storageMaintainer,
+                         Orchestrator orchestrator, ContainerOperations containerOperations,
+                         RegistryCredentialsProvider registryCredentialsProvider, StorageMaintainer storageMaintainer,
                          FlagSource flagSource, Optional<CredentialsMaintainer> credentialsMaintainer,
-                         Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Clock clock, Duration warmUpDuration) {
+                         Optional<AclMaintainer> aclMaintainer, Optional<HealthChecker> healthChecker, Clock clock,
+                         Duration warmUpDuration) {
         this.contextSupplier = contextSupplier;
         this.nodeRepository = nodeRepository;
         this.orchestrator = orchestrator;
         this.containerOperations = containerOperations;
+        this.registryCredentialsProvider = registryCredentialsProvider;
         this.storageMaintainer = storageMaintainer;
         this.credentialsMaintainer = credentialsMaintainer;
         this.aclMaintainer = aclMaintainer;
@@ -388,7 +396,10 @@ public class NodeAgentImpl implements NodeAgent {
     private boolean downloadImageIfNeeded(NodeSpec node, Optional<Container> container) {
         if (node.wantedDockerImage().equals(container.map(c -> c.image))) return false;
 
-        return node.wantedDockerImage().map(containerOperations::pullImageAsyncIfNeeded).orElse(false);
+        RegistryCredentials credentials = registryCredentialsProvider.get();
+        return node.wantedDockerImage()
+                   .map(image -> containerOperations.pullImageAsyncIfNeeded(image, credentials))
+                   .orElse(false);
     }
 
     public void converge(NodeAgentContext context) {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java
index 2ea48ed0e9c..121dca54e37 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java
@@ -8,6 +8,7 @@ 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.ProcessResult;
+import com.yahoo.vespa.hosted.dockerapi.RegistryCredentials;
 
 import java.net.InetAddress;
 import java.nio.file.Path;
@@ -75,7 +76,7 @@ public class ContainerEngineMock implements ContainerEngine {
     }
 
     @Override
-    public boolean pullImageAsyncIfNeeded(DockerImage image) {
+    public boolean pullImageAsyncIfNeeded(DockerImage image, RegistryCredentials credentials) {
         synchronized (monitor) {
             return false;
         }
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 e7292d567c6..7ec2cf5bf23 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
@@ -5,6 +5,7 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.dockerapi.ContainerEngine;
+import com.yahoo.vespa.hosted.dockerapi.RegistryCredentials;
 import com.yahoo.vespa.hosted.dockerapi.metrics.Metrics;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator;
@@ -84,7 +85,8 @@ public class DockerTester implements AutoCloseable {
         ContainerOperations containerOperations = new ContainerOperationsImpl(containerEngine, terminal, ipAddresses, fileSystem);
 
         NodeAgentFactory nodeAgentFactory = (contextSupplier, nodeContext) -> new NodeAgentImpl(
-                contextSupplier, nodeRepository, orchestrator, containerOperations, storageMaintainer, flagSource,
+                contextSupplier, nodeRepository, orchestrator, containerOperations, () -> RegistryCredentials.none,
+                storageMaintainer, flagSource,
                 Optional.empty(), Optional.empty(), Optional.empty(), clock, Duration.ofSeconds(-1));
         nodeAdmin = new NodeAdminImpl(nodeAgentFactory, metrics, clock, Duration.ofMillis(10), Duration.ZERO);
         NodeAgentContextFactory nodeAgentContextFactory = (nodeSpec, acl) ->
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 ea5d7ac7529..6a264466569 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.flags.InMemoryFlagSource;
 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.RegistryCredentials;
 import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
@@ -83,7 +84,7 @@ public class NodeAgentImplTest {
 
         verify(containerOperations, never()).removeContainer(eq(context), any());
         verify(orchestrator, never()).suspend(any(String.class));
-        verify(containerOperations, never()).pullImageAsyncIfNeeded(any());
+        verify(containerOperations, never()).pullImageAsyncIfNeeded(any(), any());
 
         final InOrder inOrder = inOrder(containerOperations, orchestrator, nodeRepository);
         // TODO: Verify this isn't run unless 1st time
@@ -153,7 +154,7 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = makeNodeAgent(null, false);
 
         when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node));
-        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false);
+        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage), any())).thenReturn(false);
 
         nodeAgent.doConverge(context);
 
@@ -162,7 +163,7 @@ public class NodeAgentImplTest {
         verify(orchestrator, never()).suspend(any(String.class));
 
         final InOrder inOrder = inOrder(containerOperations, orchestrator, nodeRepository, aclMaintainer, healthChecker);
-        inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
+        inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage), any());
         inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any(), any());
         inOrder.verify(containerOperations, times(1)).startContainer(eq(context));
         inOrder.verify(aclMaintainer, times(1)).converge(eq(context));
@@ -185,7 +186,7 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true);
 
         when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node));
-        when(containerOperations.pullImageAsyncIfNeeded(any())).thenReturn(true);
+        when(containerOperations.pullImageAsyncIfNeeded(any(), any())).thenReturn(true);
 
         nodeAgent.doConverge(context);
 
@@ -194,7 +195,7 @@ public class NodeAgentImplTest {
         verify(containerOperations, never()).removeContainer(eq(context), any());
 
         final InOrder inOrder = inOrder(containerOperations);
-        inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(eq(newDockerImage));
+        inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(eq(newDockerImage), any());
     }
 
     @Test
@@ -207,7 +208,7 @@ public class NodeAgentImplTest {
         NodeAgentContext firstContext = createContext(specBuilder.build());
         NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true);
 
-        when(containerOperations.pullImageAsyncIfNeeded(any())).thenReturn(true);
+        when(containerOperations.pullImageAsyncIfNeeded(any(), any())).thenReturn(true);
 
         InOrder inOrder = inOrder(orchestrator, containerOperations);
 
@@ -254,7 +255,7 @@ public class NodeAgentImplTest {
         NodeAgentContext firstContext = createContext(specBuilder.build());
         NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true);
 
-        when(containerOperations.pullImageAsyncIfNeeded(any())).thenReturn(true);
+        when(containerOperations.pullImageAsyncIfNeeded(any(), any())).thenReturn(true);
 
         nodeAgent.doConverge(firstContext);
         NodeAgentContext secondContext = createContext(specBuilder.memoryGb(20).build());
@@ -314,7 +315,7 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true);
 
         when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node));
-        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false);
+        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage), any())).thenReturn(false);
         doThrow(new ConvergenceException("Connection refused")).doNothing()
                 .when(healthChecker).verifyHealth(eq(context));
 
@@ -541,7 +542,7 @@ public class NodeAgentImplTest {
         NodeAgentContext context = createContext(node);
         NodeAgentImpl nodeAgent = spy(makeNodeAgent(null, false));
 
-        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false);
+        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage), any())).thenReturn(false);
         doThrow(new DockerException("Failed to set up network")).doNothing().when(containerOperations).startContainer(eq(context));
 
         try {
@@ -578,7 +579,7 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = makeNodeAgent(null, false);
 
         when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node));
-        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false);
+        when(containerOperations.pullImageAsyncIfNeeded(eq(dockerImage), any())).thenReturn(false);
 
         nodeAgent.doConverge(context);
 
@@ -586,7 +587,7 @@ public class NodeAgentImplTest {
         verify(orchestrator, never()).suspend(any(String.class));
 
         final InOrder inOrder = inOrder(containerOperations, orchestrator, nodeRepository, aclMaintainer);
-        inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
+        inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage), any());
         inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any(), any());
         inOrder.verify(containerOperations, times(1)).startContainer(eq(context));
         inOrder.verify(aclMaintainer, times(1)).converge(eq(context));
@@ -732,8 +733,9 @@ public class NodeAgentImplTest {
         }).when(containerOperations).updateContainer(any(), any());
 
         return new NodeAgentImpl(contextSupplier, nodeRepository, orchestrator, containerOperations,
-                storageMaintainer, flagSource, Optional.of(credentialsMaintainer), Optional.of(aclMaintainer),
-                Optional.of(healthChecker), clock, warmUpDuration);
+                                 () -> RegistryCredentials.none, storageMaintainer, flagSource,
+                                 Optional.of(credentialsMaintainer), Optional.of(aclMaintainer),
+                                 Optional.of(healthChecker), clock, warmUpDuration);
     }
 
     private void mockGetContainer(DockerImage dockerImage, boolean isRunning) {