Use list of all managed containers in ACL maintainer

3 files changed, 23 insertions, 36 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
index 55abbe1c79e..0b44f526670 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainer.java
@@ -1,7 +1,6 @@
 package com.yahoo.vespa.hosted.node.admin.maintenance.acl;
 
-import com.yahoo.net.HostName;
-import com.yahoo.vespa.hosted.dockerapi.Container;
+import com.yahoo.collections.Pair;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.node.admin.ContainerAclSpec;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
@@ -17,7 +16,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 /**
@@ -35,25 +33,20 @@ import java.util.stream.Collectors;
  * @author mpolden
  */
 public class AclMaintainer implements Runnable {
-
     private static final PrefixLogger log = PrefixLogger.getNodeAdminLogger(AclMaintainer.class);
 
     private static final String IPTABLES_COMMAND = "ip6tables";
 
     private final DockerOperations dockerOperations;
     private final NodeRepository nodeRepository;
-    private final Supplier<String> nodeAdminHostnameSupplier;
+    private final String nodeAdminHostname;
     private final Map<ContainerName, Acl> containerAcls;
 
-    public AclMaintainer(DockerOperations dockerOperations, NodeRepository nodeRepository) {
-        this(dockerOperations, nodeRepository, HostName::getLocalhost);
-    }
-
-    AclMaintainer(DockerOperations dockerOperations, NodeRepository nodeRepository,
-                  Supplier<String> nodeAdminHostnameSupplier) {
+    public AclMaintainer(DockerOperations dockerOperations, NodeRepository nodeRepository,
+                  String nodeAdminHostname) {
         this.dockerOperations = dockerOperations;
         this.nodeRepository = nodeRepository;
-        this.nodeAdminHostnameSupplier = nodeAdminHostnameSupplier;
+        this.nodeAdminHostname = nodeAdminHostname;
         this.containerAcls = new HashMap<>();
     }
 
@@ -85,23 +78,17 @@ public class AclMaintainer implements Runnable {
     }
 
     private void configureAcls() {
-        final List<ContainerAclSpec> aclSpecs = nodeRepository.getContainerAclSpecs(nodeAdminHostnameSupplier.get());
-        final Map<ContainerName, List<ContainerAclSpec>> aclSpecsGroupedByHostname = aclSpecs.stream()
+        final Map<ContainerName, List<ContainerAclSpec>> aclSpecsGroupedByHostname = nodeRepository
+                .getContainerAclSpecs(nodeAdminHostname).stream()
                 .collect(Collectors.groupingBy(ContainerAclSpec::trustedBy));
 
-        for (Map.Entry<ContainerName, List<ContainerAclSpec>> entry : aclSpecsGroupedByHostname.entrySet()) {
-            final ContainerName containerName = entry.getKey();
-            final Optional<Container> container = dockerOperations.getContainer(containerName);
-            if (!container.isPresent()) {
-                // Container belongs to this Docker host, but is currently unallocated
-                continue;
-            }
-            if (!container.get().state.isRunning()) {
-                log.info(String.format("Container with name %s is not running, skipping", container.get().name.asString()));
-                continue;
-            }
-            applyAcl(container.get().name, new Acl(container.get().pid, entry.getValue()));
-        }
+        dockerOperations
+                .getAllManagedContainers().stream()
+                .filter(container -> container.state.isRunning())
+                .map(container -> new Pair<>(container, aclSpecsGroupedByHostname.get(container.name)))
+                .filter(pair -> pair.getSecond() != null)
+                .forEach(pair ->
+                        applyAcl(pair.getFirst().name, new Acl(pair.getFirst().pid, pair.getSecond())));
     }
 
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
index 3f119c87389..ac4b56ff3b4 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
@@ -66,7 +66,7 @@ public class ComponentsProviderImpl implements ComponentsProvider {
         Optional<StorageMaintainer> storageMaintainer = isRunningLocally ?
                 Optional.empty() : Optional.of(new StorageMaintainer(docker, metricReceiver, environment));
         Optional<AclMaintainer> aclMaintainer = isRunningLocally ?
-                Optional.empty() : Optional.of(new AclMaintainer(dockerOperations, nodeRepository));
+                Optional.empty() : Optional.of(new AclMaintainer(dockerOperations, nodeRepository, baseHostName));
 
         Function<String, NodeAgent> nodeAgentFactory =
                 (hostName) -> new NodeAgentImpl(hostName, nodeRepository, orchestrator, dockerOperations,
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
index 860d42fb928..eea72619032 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/AclMaintainerTest.java
@@ -11,8 +11,8 @@ import org.junit.Before;
 import org.junit.Test;
 import org.mockito.verification.VerificationMode;
 
+import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -27,18 +27,20 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class AclMaintainerTest {
-
-    private static final String NODE_ADMIN_HOSTNAME = "node-admin";
+    private static final String NODE_ADMIN_HOSTNAME = "node-admin.region-1.yahoo.com";
 
     private AclMaintainer aclMaintainer;
     private DockerOperations dockerOperations;
     private NodeRepoMock nodeRepository;
+    private List<Container> containers;
 
     @Before
     public void before() {
         this.dockerOperations = mock(DockerOperations.class);
         this.nodeRepository = new NodeRepoMock(new CallOrderVerifier());
-        this.aclMaintainer = new AclMaintainer(dockerOperations, nodeRepository, () -> NODE_ADMIN_HOSTNAME);
+        this.aclMaintainer = new AclMaintainer(dockerOperations, nodeRepository, NODE_ADMIN_HOSTNAME);
+        this.containers = new ArrayList<>();
+        when(dockerOperations.getAllManagedContainers()).thenReturn(containers);
     }
 
     @Test
@@ -157,15 +159,13 @@ public class AclMaintainerTest {
         final ContainerName containerName = new ContainerName(hostname);
         final Container container = new Container(hostname, new DockerImage("mock"),
                 containerName, state, pid);
-        when(dockerOperations.getContainer(eq(containerName))).thenReturn(Optional.of(container));
+        containers.add(container);
         return container;
     }
 
     private static List<ContainerAclSpec> makeAclSpecs(int count, ContainerName containerName) {
         return IntStream.rangeClosed(1, count)
-                .mapToObj(i -> new ContainerAclSpec("node-" + i, "::" + i,
-                        containerName))
+                .mapToObj(i -> new ContainerAclSpec("node-" + i, "::" + i, containerName))
                 .collect(Collectors.toList());
     }
-
 }