Merge pull request #17601 from vespa-engine/hakonhall/allow-disabling-acl-cache-with-flag

Allow disabling ACL cache with flag

4 files changed, 44 insertions, 7 deletions

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 d4fdbb006f2..da20b22a2ac 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -192,6 +192,13 @@ public class Flags {
             "Takes effect on next run of S3 log sync task in host-admin",
             TENANT_ID, ZONE_ID);
 
+    public static final UnboundBooleanFlag CACHE_ACL = defineFeatureFlag(
+            "cache-acl", true,
+            List.of("hakon"), "2021-04-26", "2021-05-26",
+            "Whether host-admin should cache the ACL responses w/TTL 115s, or always ask config server.",
+            "Takes effect on next host-admin tick.",
+            ZONE_ID);
+
     public static final UnboundIntFlag CLUSTER_CONTROLLER_MAX_HEAP_SIZE_IN_MB = defineIntFlag(
             "cluster-controller-max-heap-size-in-mb", 128,
             List.of("hmusum"), "2021-02-10", "2021-05-15",
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 9d44f125efc..772e17291ef 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
@@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin;
 
 import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.config.provision.HostName;
-import java.util.logging.Level;
+import com.yahoo.vespa.flags.BooleanFlag;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
@@ -27,6 +29,7 @@ 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;
 
@@ -53,6 +56,7 @@ public class NodeAdminStateUpdater {
     private final Orchestrator orchestrator;
     private final NodeAdmin nodeAdmin;
     private final String hostHostname;
+    private final BooleanFlag cacheAclFlag;
 
     public enum State { TRANSITIONING, RESUMED, SUSPENDED_NODE_ADMIN, SUSPENDED }
 
@@ -64,13 +68,15 @@ public class NodeAdminStateUpdater {
             Orchestrator orchestrator,
             NodeAdmin nodeAdmin,
             HostName hostHostname,
-            Clock clock) {
+            Clock clock,
+            FlagSource flagSource) {
         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));
+        this.cacheAclFlag = Flags.CACHE_ACL.bindTo(flagSource);
     }
 
     public void start() {
@@ -168,9 +174,11 @@ public class NodeAdminStateUpdater {
         try {
             Map<String, NodeSpec> nodeSpecByHostname = nodeRepository.getNodes(hostHostname).stream()
                     .collect(Collectors.toMap(NodeSpec::hostname, Function.identity()));
-            Map<String, Acl> aclByHostname = Optional.of(cachedAclSupplier.get())
-                    .filter(acls -> acls.keySet().containsAll(nodeSpecByHostname.keySet()))
-                    .orElseGet(cachedAclSupplier::invalidateAndGet);
+            Map<String, Acl> aclByHostname = cacheAclFlag.value() ?
+                    Optional.of(cachedAclSupplier.get())
+                            .filter(acls -> acls.keySet().containsAll(nodeSpecByHostname.keySet()))
+                            .orElseGet(cachedAclSupplier::invalidateAndGet) :
+                    cachedAclSupplier.invalidateAndGet();
 
             Set<NodeAgentContext> nodeAgentContexts = nodeSpecByHostname.keySet().stream()
                     .map(hostname -> nodeAgentContextFactory.create(
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index de908f9a62e..9830bfce3a4 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
@@ -93,7 +93,7 @@ public class DockerTester implements AutoCloseable {
         NodeAgentContextFactory nodeAgentContextFactory = (nodeSpec, acl) ->
                 new NodeAgentContextImpl.Builder(nodeSpec).acl(acl).fileSystem(fileSystem).build();
         nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeAgentContextFactory, nodeRepository, orchestrator,
-                nodeAdmin, HOST_HOSTNAME, clock);
+                nodeAdmin, HOST_HOSTNAME, clock, flagSource);
 
         loopThread = new Thread(() -> {
             nodeAdminStateUpdater.start();
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 36530ff014c..d348d2b74e9 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,6 +3,8 @@ package com.yahoo.vespa.hosted.node.admin.nodeadmin;
 
 import com.yahoo.config.provision.HostName;
 import com.yahoo.test.ManualClock;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
@@ -49,9 +51,10 @@ public class NodeAdminStateUpdaterTest {
     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 InMemoryFlagSource flagSource = new InMemoryFlagSource();
 
     private final NodeAdminStateUpdater updater = spy(new NodeAdminStateUpdater(
-            nodeAgentContextFactory, nodeRepository, orchestrator, nodeAdmin, hostHostname, clock));
+            nodeAgentContextFactory, nodeRepository, orchestrator, nodeAdmin, hostHostname, clock, flagSource));
 
 
     @Test
@@ -213,6 +216,25 @@ public class NodeAdminStateUpdaterTest {
     }
 
     @Test
+    public void node_spec_and_acl_aligned_with_acl_cache_disabled() {
+        flagSource.withBooleanFlag(Flags.CACHE_ACL.id(), false);
+
+        Acl acl = new Acl.Builder().withTrustedPorts(22).build();
+        mockNodeRepo(NodeState.active, 3);
+        mockAcl(acl, 1, 2, 3);
+
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+        updater.adjustNodeAgentsToRunFromNodeRepository();
+
+        verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host1.yahoo.com")), eq(acl));
+        verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host2.yahoo.com")), eq(acl));
+        verify(nodeAgentContextFactory, times(3)).create(argThat(spec -> spec.hostname().equals("host3.yahoo.com")), eq(acl));
+        verify(nodeRepository, times(3)).getNodes(eq(hostHostname.value()));
+        verify(nodeRepository, times(3)).getAcls(eq(hostHostname.value()));
+    }
+
+    @Test
     public void node_spec_and_acl_mismatch_missing_one_acl() {
         Acl acl = new Acl.Builder().withTrustedPorts(22).build();
         mockNodeRepo(NodeState.active, 3);