Add Acl to NodeAgentContextFactory and cache

9 files changed, 183 insertions, 72 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java
index f377a603ab6..8f39fddfa1f 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java
@@ -5,12 +5,12 @@ import com.google.common.net.InetAddresses;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 
 import java.net.InetAddress;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -22,6 +22,8 @@ import java.util.stream.Collectors;
  */
 public class Acl {
 
+    public static final Acl EMPTY = new Acl(Set.of(), Set.of(), Set.of());
+
     private final Set<Node> trustedNodes;
     private final Set<Integer> trustedPorts;
     private final Set<String> trustedNetworks;
@@ -38,7 +40,7 @@ public class Acl {
     }
 
     public Acl(Set<Integer> trustedPorts, Set<Node> trustedNodes) {
-        this(trustedPorts, trustedNodes, Collections.emptySet());
+        this(trustedPorts, trustedNodes, Set.of());
     }
 
     public List<String> toRules(IPVersion ipVersion) {
@@ -131,10 +133,7 @@ public class Acl {
     }
 
     private static <T> Set<T> copyOfNullable(Set<T> set) {
-        if (set == null) {
-            return Collections.emptySet();
-        }
-        return Set.copyOf(set);
+        return Optional.ofNullable(set).map(Set::copyOf).orElseGet(Set::of);
     }
 
     public static class Node {
@@ -213,7 +212,7 @@ public class Acl {
         }
 
         public Builder withTrustedPorts(Integer... ports) {
-            trustedPorts.addAll(Arrays.asList(ports));
+            trustedPorts.addAll(List.of(ports));
             return this;
         }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
index 456391c65c2..6002e7bcd89 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
@@ -1,10 +1,11 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.nodeadmin;
 
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 
 import java.time.Duration;
 import java.util.List;
+import java.util.Set;
 
 /**
  * NodeAdmin manages the life cycle of NodeAgents.
@@ -12,11 +13,8 @@ import java.util.List;
  */
 public interface NodeAdmin {
 
-    /**
-     * Calling this will cause NodeAdmin to move to the state containersToRun by adding or removing nodes.
-     * @param containersToRun this is the wanted state.
-     */
-    void refreshContainersToRun(final List<NodeSpec> containersToRun);
+    /** Start/stop NodeAgents and schedule next NodeAgent ticks with the given NodeAgentContexts */
+    void refreshContainersToRun(Set<NodeAgentContext> nodeAgentContexts);
 
     /** Gather node agent and its docker container metrics and forward them to the {@code MetricReceiverWrapper} */
     void updateNodeAgentMetrics();
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 792656ca19f..546bf111e39 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
@@ -1,19 +1,15 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.nodeadmin;
 
-import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper;
 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.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
-import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextFactory;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextManager;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentFactory;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentScheduler;
-import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -23,6 +19,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 /**
@@ -31,12 +28,10 @@ import java.util.stream.Collectors;
  * @author stiankri
  */
 public class NodeAdminImpl implements NodeAdmin {
-    private static final PrefixLogger logger = PrefixLogger.getNodeAdminLogger(NodeAdmin.class);
     private static final Duration NODE_AGENT_FREEZE_TIMEOUT = Duration.ofSeconds(5);
     private static final Duration NODE_AGENT_SPREAD = Duration.ofSeconds(3);
 
     private final NodeAgentWithSchedulerFactory nodeAgentWithSchedulerFactory;
-    private final NodeAgentContextFactory nodeAgentContextFactory;
 
     private final Clock clock;
     private final Duration freezeTimeout;
@@ -50,25 +45,20 @@ public class NodeAdminImpl implements NodeAdmin {
     private final GaugeWrapper numberOfContainersInLoadImageState;
     private final CounterWrapper numberOfUnhandledExceptionsInNodeAgent;
 
-    public NodeAdminImpl(NodeAgentFactory nodeAgentFactory,
-                         NodeAgentContextFactory nodeAgentContextFactory,
-                         MetricReceiverWrapper metricReceiver,
-                         Clock clock) {
+    public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, MetricReceiverWrapper metricReceiver, Clock clock) {
         this((NodeAgentWithSchedulerFactory) nodeAgentContext -> create(clock, nodeAgentFactory, nodeAgentContext),
-                nodeAgentContextFactory, metricReceiver, clock, NODE_AGENT_FREEZE_TIMEOUT, NODE_AGENT_SPREAD);
+                metricReceiver, clock, NODE_AGENT_FREEZE_TIMEOUT, NODE_AGENT_SPREAD);
     }
 
-    public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, NodeAgentContextFactory nodeAgentContextFactory,
-                         MetricReceiverWrapper metricReceiver, Clock clock, Duration freezeTimeout, Duration spread) {
+    public NodeAdminImpl(NodeAgentFactory nodeAgentFactory, MetricReceiverWrapper metricReceiver,
+                         Clock clock, Duration freezeTimeout, Duration spread) {
         this((NodeAgentWithSchedulerFactory) nodeAgentContext -> create(clock, nodeAgentFactory, nodeAgentContext),
-                nodeAgentContextFactory, metricReceiver, clock, freezeTimeout, spread);
+                metricReceiver, clock, freezeTimeout, spread);
     }
 
     NodeAdminImpl(NodeAgentWithSchedulerFactory nodeAgentWithSchedulerFactory,
-                  NodeAgentContextFactory nodeAgentContextFactory, MetricReceiverWrapper metricReceiver,
-                  Clock clock, Duration freezeTimeout, Duration spread) {
+                  MetricReceiverWrapper metricReceiver, Clock clock, Duration freezeTimeout, Duration spread) {
         this.nodeAgentWithSchedulerFactory = nodeAgentWithSchedulerFactory;
-        this.nodeAgentContextFactory = nodeAgentContextFactory;
 
         this.clock = clock;
         this.freezeTimeout = freezeTimeout;
@@ -83,9 +73,9 @@ public class NodeAdminImpl implements NodeAdmin {
     }
 
     @Override
-    public void refreshContainersToRun(List<NodeSpec> containersToRun) {
-        final Map<String, NodeAgentContext> nodeAgentContextsByHostname = containersToRun.stream()
-                .collect(Collectors.toMap(NodeSpec::getHostname, nodeAgentContextFactory::create));
+    public void refreshContainersToRun(Set<NodeAgentContext> nodeAgentContexts) {
+        Map<String, NodeAgentContext> nodeAgentContextsByHostname = nodeAgentContexts.stream()
+                .collect(Collectors.toMap(nac -> nac.hostname().value(), Function.identity()));
 
         // Stop and remove NodeAgents that should no longer be running
         diff(nodeAgentWithSchedulerByHostname.keySet(), nodeAgentContextsByHostname.keySet())
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java
index 18c3a836e41..eb306036416 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdater.java
@@ -4,18 +4,28 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin;
 import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.log.LogLevel;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextFactory;
 import com.yahoo.vespa.hosted.provision.Node;
 
+import java.time.Clock;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -37,6 +47,8 @@ public class NodeAdminStateUpdater {
     private final ScheduledExecutorService metricsScheduler =
             Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("metricsscheduler"));
 
+    private final CachedSupplier<Map<String, Acl>> cachedAclSupplier;
+    private final NodeAgentContextFactory nodeAgentContextFactory;
     private final NodeRepository nodeRepository;
     private final Orchestrator orchestrator;
     private final NodeAdmin nodeAdmin;
@@ -47,14 +59,18 @@ public class NodeAdminStateUpdater {
     private volatile State currentState = SUSPENDED_NODE_ADMIN;
 
     public NodeAdminStateUpdater(
+            NodeAgentContextFactory nodeAgentContextFactory,
             NodeRepository nodeRepository,
             Orchestrator orchestrator,
             NodeAdmin nodeAdmin,
-            HostName hostHostname) {
+            HostName hostHostname,
+            Clock clock) {
+        this.nodeAgentContextFactory = nodeAgentContextFactory;
         this.nodeRepository = nodeRepository;
         this.orchestrator = orchestrator;
         this.nodeAdmin = nodeAdmin;
         this.hostHostname = hostHostname.value();
+        this.cachedAclSupplier = new CachedSupplier<>(clock, Duration.ofSeconds(115), () -> nodeRepository.getAcls(this.hostHostname));
     }
 
     public void start() {
@@ -145,11 +161,21 @@ public class NodeAdminStateUpdater {
         currentState = wantedState;
     }
 
-    private void adjustNodeAgentsToRunFromNodeRepository() {
+    void adjustNodeAgentsToRunFromNodeRepository() {
         try {
-            final List<NodeSpec> containersToRun = nodeRepository.getNodes(hostHostname);
-            nodeAdmin.refreshContainersToRun(containersToRun);
-        } catch (Exception e) {
+            Map<String, NodeSpec> nodeSpecByHostname = nodeRepository.getNodes(hostHostname).stream()
+                    .collect(Collectors.toMap(NodeSpec::getHostname, Function.identity()));
+            Map<String, Acl> aclByHostname = Optional.of(cachedAclSupplier.get())
+                    .filter(acls -> acls.keySet().containsAll(nodeSpecByHostname.keySet()))
+                    .orElseGet(cachedAclSupplier::invalidateAndGet);
+
+            Set<NodeAgentContext> nodeAgentContexts = nodeSpecByHostname.keySet().stream()
+                    .map(hostname -> nodeAgentContextFactory.create(
+                            nodeSpecByHostname.get(hostname),
+                            aclByHostname.getOrDefault(hostname, Acl.EMPTY)))
+                    .collect(Collectors.toSet());
+            nodeAdmin.refreshContainersToRun(nodeAgentContexts);
+        } catch (RuntimeException e) {
             log.log(LogLevel.WARNING, "Failed to update which containers should be running", e);
         }
     }
@@ -161,4 +187,33 @@ public class NodeAdminStateUpdater {
                              .map(NodeSpec::getHostname)
                              .collect(Collectors.toList());
     }
+
+    private static class CachedSupplier<T> implements Supplier<T> {
+        private final Clock clock;
+        private final Duration expiration;
+        private final Supplier<T> supplier;
+        private Instant refreshAt;
+        private T cachedValue;
+
+        private CachedSupplier(Clock clock, Duration expiration, Supplier<T> supplier) {
+            this.clock = clock;
+            this.expiration = expiration;
+            this.supplier = supplier;
+            this.refreshAt = Instant.MIN;
+        }
+
+        @Override
+        public T get() {
+            if (! clock.instant().isBefore(refreshAt)) {
+                cachedValue = supplier.get();
+                refreshAt = clock.instant().plus(expiration);
+            }
+            return cachedValue;
+        }
+
+        private T invalidateAndGet() {
+            refreshAt = Instant.MIN;
+            return get();
+        }
+    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextFactory.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextFactory.java
index 0cfafe34717..3e0991d4357 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextFactory.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextFactory.java
@@ -1,6 +1,7 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.nodeagent;
 
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 
 /**
@@ -8,5 +9,5 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
  */
 @FunctionalInterface
 public interface NodeAgentContextFactory {
-    NodeAgentContext create(NodeSpec nodeSpec);
+    NodeAgentContext create(NodeSpec nodeSpec, Acl acl);
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
index e3872d9cf31..804450f05ff 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
@@ -15,7 +15,6 @@ import com.yahoo.vespa.hosted.provision.Node;
 import java.nio.file.FileSystem;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Collections;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Level;
@@ -235,7 +234,7 @@ public class NodeAgentContextImpl implements NodeAgentContext {
         public NodeAgentContextImpl build() {
             return new NodeAgentContextImpl(
                     nodeSpecBuilder.build(),
-                    Optional.ofNullable(acl).orElseGet(() -> new Acl(Collections.emptySet(), Collections.emptySet())),
+                    Optional.ofNullable(acl).orElse(Acl.EMPTY),
                     Optional.ofNullable(identity).orElseGet(() -> new AthenzService("domain", "service")),
                     Optional.ofNullable(dockerNetworking).orElse(DockerNetworking.HOST_NETWORK),
                     Optional.ofNullable(zoneId).orElseGet(() -> new ZoneId(SystemName.dev, Environment.dev, RegionName.defaultName())),
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 2f648cc7a79..6ae373e8b55 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
@@ -96,11 +96,11 @@ public class DockerTester implements AutoCloseable {
         NodeAgentFactory nodeAgentFactory = contextSupplier -> new NodeAgentImpl(
                 contextSupplier, nodeRepository, orchestrator, dockerOperations, storageMaintainer, flagSource,
                 Optional.empty(), Optional.empty(), Optional.empty());
-        NodeAgentContextFactory nodeAgentContextFactory = nodeSpec ->
-                new NodeAgentContextImpl.Builder(nodeSpec).fileSystem(fileSystem).build();
-        nodeAdmin = new NodeAdminImpl(nodeAgentFactory, nodeAgentContextFactory, mr, Clock.systemUTC(), Duration.ofMillis(10), Duration.ZERO);
-        nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepository, orchestrator,
-                nodeAdmin, HOST_HOSTNAME);
+        nodeAdmin = new NodeAdminImpl(nodeAgentFactory, mr, Clock.systemUTC(), Duration.ofMillis(10), Duration.ZERO);
+        NodeAgentContextFactory nodeAgentContextFactory = (nodeSpec, acl) ->
+                new NodeAgentContextImpl.Builder(nodeSpec).acl(acl).fileSystem(fileSystem).build();
+        nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeAgentContextFactory, nodeRepository, orchestrator,
+                nodeAdmin, HOST_HOSTNAME, Clock.systemUTC());
 
         loopThread = new Thread(() -> {
             nodeAdminStateUpdater.start();
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 881d1829b84..2f835a535ff 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
@@ -15,8 +15,9 @@ import org.mockito.InOrder;
 
 import java.time.Duration;
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -43,40 +44,40 @@ public class NodeAdminImplTest {
     private final NodeAgentContextFactory nodeAgentContextFactory = mock(NodeAgentContextFactory.class);
     private final ManualClock clock = new ManualClock();
 
-    private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentWithSchedulerFactory, nodeAgentContextFactory,
+    private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentWithSchedulerFactory,
             new MetricReceiverWrapper(MetricReceiver.nullImplementation), clock, Duration.ZERO, Duration.ZERO);
 
     @Test
     public void nodeAgentsAreProperlyLifeCycleManaged() {
-        final NodeSpec nodeSpec1 = createNodeSpec("host1.test.yahoo.com");
-        final NodeSpec nodeSpec2 = createNodeSpec("host2.test.yahoo.com");
-        final NodeAgentWithScheduler nodeAgent1 = mockNodeAgentWithSchedulerFactory(nodeSpec1);
-        final NodeAgentWithScheduler nodeAgent2 = mockNodeAgentWithSchedulerFactory(nodeSpec2);
+        final NodeAgentContext context1 = createNodeAgentContext("host1.test.yahoo.com");
+        final NodeAgentContext context2 = createNodeAgentContext("host2.test.yahoo.com");
+        final NodeAgentWithScheduler nodeAgent1 = mockNodeAgentWithSchedulerFactory(context1);
+        final NodeAgentWithScheduler nodeAgent2 = mockNodeAgentWithSchedulerFactory(context2);
 
         final InOrder inOrder = inOrder(nodeAgentWithSchedulerFactory, nodeAgent1, nodeAgent2);
-        nodeAdmin.refreshContainersToRun(Collections.emptyList());
+        nodeAdmin.refreshContainersToRun(Set.of());
         verifyNoMoreInteractions(nodeAgentWithSchedulerFactory);
 
-        nodeAdmin.refreshContainersToRun(Collections.singletonList(nodeSpec1));
+        nodeAdmin.refreshContainersToRun(Set.of(context1));
         inOrder.verify(nodeAgent1).start();
         inOrder.verify(nodeAgent2, never()).start();
         inOrder.verify(nodeAgent1, never()).stop();
 
-        nodeAdmin.refreshContainersToRun(Collections.singletonList(nodeSpec1));
+        nodeAdmin.refreshContainersToRun(Set.of(context1));
         inOrder.verify(nodeAgentWithSchedulerFactory, never()).create(any());
         inOrder.verify(nodeAgent1, never()).start();
         inOrder.verify(nodeAgent1, never()).stop();
 
-        nodeAdmin.refreshContainersToRun(Collections.emptyList());
+        nodeAdmin.refreshContainersToRun(Set.of());
         inOrder.verify(nodeAgentWithSchedulerFactory, never()).create(any());
         verify(nodeAgent1).stop();
 
-        nodeAdmin.refreshContainersToRun(Collections.singletonList(nodeSpec2));
+        nodeAdmin.refreshContainersToRun(Set.of(context2));
         inOrder.verify(nodeAgent2).start();
         inOrder.verify(nodeAgent2, never()).stop();
         inOrder.verify(nodeAgent1, never()).stop();
 
-        nodeAdmin.refreshContainersToRun(Collections.emptyList());
+        nodeAdmin.refreshContainersToRun(Set.of());
         inOrder.verify(nodeAgentWithSchedulerFactory, never()).create(any());
         inOrder.verify(nodeAgent2, never()).start();
         inOrder.verify(nodeAgent2).stop();
@@ -86,17 +87,17 @@ public class NodeAdminImplTest {
 
     @Test
     public void testSetFrozen() {
-        List<NodeSpec> nodeSpecs = new ArrayList<>();
+        Set<NodeAgentContext> contexts = new HashSet<>();
         List<NodeAgentWithScheduler> nodeAgents = new ArrayList<>();
         for (int i = 0; i < 3; i++) {
-            NodeSpec nodeSpec = createNodeSpec("host" + i + ".test.yahoo.com");
-            NodeAgentWithScheduler nodeAgent = mockNodeAgentWithSchedulerFactory(nodeSpec);
+            NodeAgentContext context = createNodeAgentContext("host" + i + ".test.yahoo.com");
+            NodeAgentWithScheduler nodeAgent = mockNodeAgentWithSchedulerFactory(context);
 
-            nodeSpecs.add(nodeSpec);
+            contexts.add(context);
             nodeAgents.add(nodeAgent);
         }
 
-        nodeAdmin.refreshContainersToRun(nodeSpecs);
+        nodeAdmin.refreshContainersToRun(contexts);
 
         assertTrue(nodeAdmin.isFrozen()); // Initially everything is frozen to force convergence
         mockNodeAgentSetFrozenResponse(nodeAgents, true, true, true);
@@ -159,19 +160,18 @@ public class NodeAdminImplTest {
         }
     }
 
-    private NodeSpec createNodeSpec(String hostname) {
-        return new NodeSpec.Builder()
+    private NodeAgentContext createNodeAgentContext(String hostname) {
+        NodeSpec nodeSpec = new NodeSpec.Builder()
                 .hostname(hostname)
                 .state(Node.State.active)
                 .nodeType(NodeType.tenant)
                 .flavor("default")
                 .build();
-    }
 
-    private NodeAgentWithScheduler mockNodeAgentWithSchedulerFactory(NodeSpec nodeSpec) {
-        NodeAgentContext context = new NodeAgentContextImpl.Builder(nodeSpec).build();
-        when(nodeAgentContextFactory.create(eq(nodeSpec))).thenReturn(context);
+        return new NodeAgentContextImpl.Builder(nodeSpec).build();
+    }
 
+    private NodeAgentWithScheduler mockNodeAgentWithSchedulerFactory(NodeAgentContext context) {
         NodeAgentWithScheduler nodeAgentWithScheduler = mock(NodeAgentWithScheduler.class);
         when(nodeAgentWithSchedulerFactory.create(eq(context))).thenReturn(nodeAgentWithScheduler);
         return nodeAgentWithScheduler;
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java
index 74ba5561c8e..bd08cc49c02 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterTest.java
@@ -3,15 +3,21 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin;
 
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeType;
+import com.yahoo.test.ManualClock;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextFactory;
 import com.yahoo.vespa.hosted.provision.Node;
 import org.junit.Test;
 
 import java.time.Duration;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -22,6 +28,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -37,13 +44,15 @@ import static org.mockito.Mockito.when;
  * @author freva
  */
 public class NodeAdminStateUpdaterTest {
+    private final NodeAgentContextFactory nodeAgentContextFactory = mock(NodeAgentContextFactory.class);
     private final NodeRepository nodeRepository = mock(NodeRepository.class);
     private final Orchestrator orchestrator = mock(Orchestrator.class);
     private final NodeAdmin nodeAdmin = mock(NodeAdmin.class);
     private final HostName hostHostname = HostName.from("basehost1.test.yahoo.com");
+    private final ManualClock clock = new ManualClock();
 
     private final NodeAdminStateUpdater updater = spy(new NodeAdminStateUpdater(
-            nodeRepository, orchestrator, nodeAdmin, hostHostname));
+            nodeAgentContextFactory, nodeRepository, orchestrator, nodeAdmin, hostHostname, clock));
 
 
     @Test
@@ -167,6 +176,58 @@ public class NodeAdminStateUpdaterTest {
         verify(orchestrator, times(1)).suspend(eq(hostHostname.value()), eq(activeHostnames));
     }
 
+    @Test
+    public void uses_cached_acl() {
+        mockNodeRepo(Node.State.active, 1);
+        mockAcl(Acl.EMPTY, 1);
+
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        verify(nodeRepository, times(1)).getAcls(any());
+
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        verify(nodeRepository, times(1)).getAcls(any());
+
+        clock.advance(Duration.ofMinutes(2));
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        verify(nodeRepository, times(2)).getAcls(any());
+    }
+
+    @Test
+    public void node_spec_and_acl_aligned() {
+        Acl acl = new Acl.Builder().withTrustedPorts(22).build();
+        mockNodeRepo(Node.State.active, 3);
+        mockAcl(acl, 1, 2, 3);
+
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host1.yahoo.com")), eq(acl));
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host2.yahoo.com")), eq(acl));
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host3.yahoo.com")), eq(acl));
+    }
+
+    @Test
+    public void node_spec_and_acl_mismatch_missing_one_acl() {
+        Acl acl = new Acl.Builder().withTrustedPorts(22).build();
+        mockNodeRepo(Node.State.active, 3);
+        mockAcl(acl, 1, 3); // Acl for 2 is missing
+
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host1.yahoo.com")), eq(acl));
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host2.yahoo.com")), eq(Acl.EMPTY));
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host3.yahoo.com")), eq(acl));
+    }
+
+    @Test
+    public void node_spec_and_acl_mismatch_additional_acl() {
+        Acl acl = new Acl.Builder().withTrustedPorts(22).build();
+        mockNodeRepo(Node.State.active, 2);
+        mockAcl(acl, 1, 2, 3); // Acl for 3 is extra
+
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host1.yahoo.com")), eq(acl));
+        verify(nodeAgentContextFactory).create(argThat(spec -> spec.getHostname().equals("host2.yahoo.com")), eq(acl));
+    }
+
     private void assertConvergeError(NodeAdminStateUpdater.State targetState, String reason) {
         try {
             updater.converge(targetState);
@@ -177,9 +238,9 @@ public class NodeAdminStateUpdaterTest {
     }
 
     private void mockNodeRepo(Node.State hostState, int numberOfNodes) {
-        List<NodeSpec> containersToRun = IntStream.range(0, numberOfNodes)
+        List<NodeSpec> containersToRun = IntStream.range(1, numberOfNodes + 1)
                 .mapToObj(i -> new NodeSpec.Builder()
-                        .hostname("host" + i + ".test.yahoo.com")
+                        .hostname("host" + i + ".yahoo.com")
                         .state(Node.State.active)
                         .nodeType(NodeType.tenant)
                         .flavor("docker")
@@ -201,4 +262,12 @@ public class NodeAdminStateUpdaterTest {
                 .minDiskAvailableGb(1)
                 .build());
     }
+
+    private void mockAcl(Acl acl, int... nodeIds) {
+        Map<String, Acl> aclByHostname = Arrays.stream(nodeIds)
+                .mapToObj(i -> "host" + i + ".yahoo.com")
+                .collect(Collectors.toMap(Function.identity(), h -> acl));
+
+        when(nodeRepository.getAcls(eq(hostHostname.value()))).thenReturn(aclByHostname);
+    }
 }