Remove Environment from NodeAgent and AclMaintainer

3 files changed, 25 insertions, 56 deletions

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 5b882461937..a270585bcaa 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
@@ -1,14 +1,13 @@
 // 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.maintenance.acl;
 
+import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
-import com.yahoo.vespa.hosted.node.admin.component.Environment;
 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.docker.DockerNetworking;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesMock;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
@@ -32,21 +31,18 @@ import static org.mockito.Mockito.when;
 
 public class AclMaintainerTest {
 
-    private static final String NODE_ADMIN_HOSTNAME = "node-admin.region-1.yahoo.com";
-
+    private final HostName hostHostname = HostName.from("node-admin.region-1.yahoo.com");
     private final IPAddressesMock ipAddresses = new IPAddressesMock();
     private final DockerOperations dockerOperations = mock(DockerOperations.class);
     private final NodeRepository nodeRepository = mock(NodeRepository.class);
     private final Map<String, Container> containers = new HashMap<>();
     private final List<Container> containerList = new ArrayList<>();
-    private final Environment env = mock(Environment.class);
     private final AclMaintainer aclMaintainer =
-            new AclMaintainer(dockerOperations, nodeRepository, NODE_ADMIN_HOSTNAME, ipAddresses, env);
+            new AclMaintainer(dockerOperations, nodeRepository, hostHostname, ipAddresses);
 
     @Before
     public void before() {
         when(dockerOperations.getAllManagedContainers()).thenReturn(containerList);
-        when(env.getDockerNetworking()).thenReturn(DockerNetworking.NPT);
     }
 
     @Test
@@ -54,14 +50,14 @@ public class AclMaintainerTest {
         Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
         Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
 
-        when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls);
+        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
 
         whenListRules(container.name, "filter", IPVersion.IPv6, "");
         whenListRules(container.name, "filter", IPVersion.IPv4, "");
         whenListRules(container.name, "nat", IPVersion.IPv4, "");
         whenListRules(container.name, "nat", IPVersion.IPv6, "");
 
-        aclMaintainer.run();
+        aclMaintainer.converge();
 
         verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), anyVararg()); //we don;t have a ip4 address for the container so no redirect either
         verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), anyVararg());
@@ -72,14 +68,14 @@ public class AclMaintainerTest {
         Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
         Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
 
-        when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls);
+        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
 
         whenListRules(container.name, "filter", IPVersion.IPv6, "");
         whenListRules(container.name, "filter", IPVersion.IPv4, "");
         whenListRules(container.name, "nat", IPVersion.IPv4, "");
         whenListRules(container.name, "nat", IPVersion.IPv6, "");
 
-        aclMaintainer.run();
+        aclMaintainer.converge();
 
         verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), anyVararg()); //we don;t have a ip4 address for the container so no redirect either
         verify(dockerOperations, times(2)).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), anyVararg());
@@ -90,9 +86,9 @@ public class AclMaintainerTest {
         Container container = addContainer("container1", "container1.host.com", Container.State.EXITED);
         Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
 
-        when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls);
+        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
 
-        aclMaintainer.run();
+        aclMaintainer.converge();
 
         verify(dockerOperations, never()).executeCommandInNetworkNamespace(eq(container.name), anyVararg());
     }
@@ -102,7 +98,7 @@ public class AclMaintainerTest {
         Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
         Map<String, Acl> acls = makeAcl(container.hostname, "4321,2345,22", "2001::1", "fd01:1234::4321");
 
-        when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls);
+        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
 
         String IPV6 = "-P INPUT ACCEPT\n" +
                 "-P FORWARD ACCEPT\n" +
@@ -125,7 +121,7 @@ public class AclMaintainerTest {
         whenListRules(container.name, "filter", IPVersion.IPv4, ""); //IPv4 will then differ from wanted
         whenListRules(container.name, "nat", IPVersion.IPv6, NATv6);
 
-        aclMaintainer.run();
+        aclMaintainer.converge();
 
         verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name), eq("iptables-restore"), anyVararg());
         verify(dockerOperations, never()).executeCommandInNetworkNamespace(eq(container.name), eq("ip6tables-restore"), anyVararg());
@@ -136,7 +132,7 @@ public class AclMaintainerTest {
         Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
         Map<String, Acl> acls = makeAcl(container.hostname, "22,4443,2222", "2001::1", "192.64.13.2");
 
-        when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls);
+        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
 
         String IPV4_FILTER = "-P INPUT ACCEPT\n" +
                 "-P FORWARD ACCEPT\n" +
@@ -168,7 +164,7 @@ public class AclMaintainerTest {
         whenListRules(container.name, "nat", IPVersion.IPv6, IPV6_NAT);
         whenListRules(container.name, "filter", IPVersion.IPv4, IPV4_FILTER);
 
-        aclMaintainer.run();
+        aclMaintainer.converge();
 
         verify(dockerOperations, never()).executeCommandInNetworkNamespace(any(), eq("ip6tables-restore"), anyVararg());
         verify(dockerOperations, never()).executeCommandInNetworkNamespace(any(), eq("iptables-restore"), anyVararg());
@@ -179,7 +175,7 @@ public class AclMaintainerTest {
     public void rollback_is_attempted_when_applying_acl_fail() {
         Container container = addContainer("container1", "container1.host.com", Container.State.RUNNING);
         Map<String, Acl> acls = makeAcl(container.hostname, "4321", "2001::1");
-        when(nodeRepository.getAcls(NODE_ADMIN_HOSTNAME)).thenReturn(acls);
+        when(nodeRepository.getAcls(hostHostname.value())).thenReturn(acls);
 
         String IPV6_NAT = "-P PREROUTING ACCEPT\n" +
                 "-P INPUT ACCEPT\n" +
@@ -199,7 +195,7 @@ public class AclMaintainerTest {
                 eq(container.name),
                 eq("iptables-restore"), anyVararg())).thenThrow(new RuntimeException("iptables restore failed"));
 
-        aclMaintainer.run();
+        aclMaintainer.converge();
 
         verify(dockerOperations, times(1)).executeCommandInNetworkNamespace(eq(container.name),
                 eq("ip6tables"), eq("-F"), eq("-t"), eq("filter"));
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 2431433b17f..bb229a10f59 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
@@ -14,6 +14,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 
@@ -37,10 +38,9 @@ public class NodeAdminImplTest {
     // Trick to allow mocking of typed interface without casts/warnings.
     private interface NodeAgentFactory extends Function<String, NodeAgent> {}
     private final Function<String, NodeAgent> nodeAgentFactory = mock(NodeAgentFactory.class);
-    private final Runnable aclMaintainer = mock(Runnable.class);
     private final ManualClock clock = new ManualClock();
 
-    private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentFactory, aclMaintainer,
+    private final NodeAdminImpl nodeAdmin = new NodeAdminImpl(nodeAgentFactory, Optional.empty(),
             new MetricReceiverWrapper(MetricReceiver.nullImplementation), clock);
 
     @Test
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 42e20311eb5..00326f90caa 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,22 +12,17 @@ import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 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.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
-import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 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.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator;
-import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.maintenance.identity.AthenzCredentialsMaintainer;
-import com.yahoo.vespa.hosted.node.admin.component.PathResolver;
 import com.yahoo.vespa.hosted.provision.Node;
 import org.junit.Test;
 import org.mockito.InOrder;
 
-import java.io.IOException;
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -80,18 +75,7 @@ public class NodeAgentImplTest {
             Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap());
     private final AthenzCredentialsMaintainer athenzCredentialsMaintainer = mock(AthenzCredentialsMaintainer.class);
 
-    private final PathResolver pathResolver = mock(PathResolver.class);
     private final ManualClock clock = new ManualClock();
-    private final Environment environment = new Environment.Builder()
-            .configServerConfig(new ConfigServerConfig(new ConfigServerConfig.Builder()))
-            .environment("dev")
-            .region("us-east-1")
-            .system("main")
-            .cloud("mycloud")
-            .parentHostHostname("parent.host.name.yahoo.com")
-            .pathResolver(pathResolver)
-            .dockerNetworking(DockerNetworking.HOST_NETWORK)
-            .build();
 
     private final NodeSpec.Builder nodeBuilder = new NodeSpec.Builder()
             .hostname(context.hostname().value())
@@ -196,7 +180,7 @@ public class NodeAgentImplTest {
     }
 
     @Test
-    public void absentContainerCausesStart() throws Exception {
+    public void absentContainerCausesStart() {
         final Optional<Long> restartGeneration = Optional.of(1L);
         final long rebootGeneration = 0;
         final NodeSpec node = nodeBuilder
@@ -211,8 +195,6 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = makeNodeAgent(null, false);
 
         when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node));
-        when(pathResolver.getApplicationStoragePathForNodeAdmin()).thenReturn(Files.createTempDirectory("foo"));
-        when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar"));
         when(dockerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false);
         when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L));
 
@@ -226,7 +208,7 @@ public class NodeAgentImplTest {
         inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
         inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), eq(node), any());
         inOrder.verify(dockerOperations, times(1)).startContainer(eq(context));
-        inOrder.verify(aclMaintainer, times(1)).run();
+        inOrder.verify(aclMaintainer, times(1)).converge();
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context));
         inOrder.verify(nodeRepository).updateNodeAttributes(
                 hostName, new NodeAttributes()
@@ -268,7 +250,7 @@ public class NodeAgentImplTest {
     }
 
     @Test
-    public void containerIsRestartedIfFlavorChanged() throws IOException {
+    public void containerIsRestartedIfFlavorChanged() {
         final long wantedRestartGeneration = 1;
         final long currentRestartGeneration = 1;
         NodeSpec.Builder specBuilder = nodeBuilder
@@ -291,7 +273,6 @@ public class NodeAgentImplTest {
                 .thenReturn(Optional.of(thirdSpec));
         when(dockerOperations.pullImageAsyncIfNeeded(any())).thenReturn(true);
         when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L));
-        when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar"));
 
         nodeAgent.converge();
         nodeAgent.converge();
@@ -501,7 +482,7 @@ public class NodeAgentImplTest {
     }
 
     @Test
-    public void testRestartDeadContainerAfterNodeAdminRestart() throws IOException {
+    public void testRestartDeadContainerAfterNodeAdminRestart() {
         final NodeSpec node = nodeBuilder
                 .currentDockerImage(dockerImage)
                 .wantedDockerImage(dockerImage)
@@ -512,8 +493,6 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, false);
 
         when(nodeRepository.getOptionalNode(eq(hostName))).thenReturn(Optional.of(node));
-        when(pathResolver.getApplicationStoragePathForNodeAdmin()).thenReturn(Files.createTempDirectory("foo"));
-        when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar"));
         when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L));
 
         nodeAgent.tick();
@@ -590,7 +569,7 @@ public class NodeAgentImplTest {
     }
 
     @Test
-    public void start_container_subtask_failure_leads_to_container_restart() throws IOException {
+    public void start_container_subtask_failure_leads_to_container_restart() {
         final long restartGeneration = 1;
         final long rebootGeneration = 0;
         final NodeSpec node = nodeBuilder
@@ -605,8 +584,6 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = spy(makeNodeAgent(null, false));
 
         when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node));
-        when(pathResolver.getApplicationStoragePathForNodeAdmin()).thenReturn(Files.createTempDirectory("foo"));
-        when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar"));
         when(dockerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false);
         when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L));
         doThrow(new DockerException("Failed to set up network")).doNothing().when(dockerOperations).startContainer(eq(context));
@@ -669,7 +646,6 @@ public class NodeAgentImplTest {
         when(dockerOperations.getContainerStats(eq(context)))
                 .thenReturn(Optional.of(stats1))
                 .thenReturn(Optional.of(stats2));
-        when(pathResolver.getApplicationStoragePathForHost()).thenReturn(Files.createTempDirectory("bar"));
 
         nodeAgent.converge(); // Run the converge loop once to initialize lastNode
         nodeAgent.updateContainerNodeMetrics(); // Update metrics once to init and lastCpuMetric
@@ -718,7 +694,7 @@ public class NodeAgentImplTest {
     }
 
     @Test
-    public void testRunningConfigServer() throws IOException {
+    public void testRunningConfigServer() {
         final long rebootGeneration = 0;
         final NodeSpec node = nodeBuilder
                 .nodeType(NodeType.config)
@@ -730,8 +706,6 @@ public class NodeAgentImplTest {
         NodeAgentImpl nodeAgent = makeNodeAgent(null, false);
 
         when(nodeRepository.getOptionalNode(hostName)).thenReturn(Optional.of(node));
-        Path tempDirectory = Files.createTempDirectory("foo");
-        when(pathResolver.getApplicationStoragePathForHost()).thenReturn(tempDirectory);
         when(dockerOperations.pullImageAsyncIfNeeded(eq(dockerImage))).thenReturn(false);
         when(storageMaintainer.getDiskUsageFor(eq(context))).thenReturn(Optional.of(201326592000L));
 
@@ -744,7 +718,7 @@ public class NodeAgentImplTest {
         inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
         inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), eq(node), any());
         inOrder.verify(dockerOperations, times(1)).startContainer(eq(context));
-        inOrder.verify(aclMaintainer, times(1)).run();
+        inOrder.verify(aclMaintainer, times(1)).converge();
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context));
         inOrder.verify(nodeRepository).updateNodeAttributes(
                 hostName, new NodeAttributes()
@@ -757,11 +731,10 @@ public class NodeAgentImplTest {
         mockGetContainer(dockerImage, isRunning);
 
         when(dockerOperations.getContainerStats(any())).thenReturn(Optional.of(emptyContainerStats));
-        doNothing().when(storageMaintainer).writeFilebeatConfig(any(), any());
         doNothing().when(storageMaintainer).writeMetricsConfig(any(), any());
 
         return new NodeAgentImpl(context, nodeRepository, orchestrator, dockerOperations,
-                storageMaintainer, aclMaintainer, environment, clock, NODE_AGENT_SCAN_INTERVAL, Optional.of(athenzCredentialsMaintainer));
+                storageMaintainer, clock, NODE_AGENT_SCAN_INTERVAL, Optional.of(athenzCredentialsMaintainer), Optional.of(aclMaintainer));
     }
 
     private void mockGetContainer(DockerImage dockerImage, boolean isRunning) {