diff options
author | HÃ¥kon Hallingstad <hakon@verizonmedia.com> | 2021-04-26 17:04:36 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-26 17:04:36 +0200 |
commit | 6ab4761b814b9b369a60b1f2e059361f8da8e8e4 (patch) | |
tree | 3cb86fc0ee7fdd0f6efb141d3f12e35c96f827ba | |
parent | 9458ad2d810ff988ccc19cd8554bdbc78dabe588 (diff) | |
parent | c85eed924e8b8243a7e0a5d96de08522e5185ebf (diff) |
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); |