DockerOperations now takes in Enviroment instance instead of static method for testing

6 files changed, 36 insertions, 32 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index e1908b7813c..1f56024b1c7 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -66,9 +66,11 @@ public class DockerOperationsImpl implements DockerOperations {
             getDefaults().underVespaHome("var/zookeeper"));
 
     private final Docker docker;
+    private final Environment environment;
 
-    public DockerOperationsImpl(Docker docker) {
+    public DockerOperationsImpl(Docker docker, Environment environment) {
         this.docker = docker;
+        this.environment = environment;
     }
 
     @Override
@@ -203,11 +205,11 @@ public class DockerOperationsImpl implements DockerOperations {
 
         logger.info("Starting container " + nodeSpec.containerName);
         try {
-            InetAddress nodeInetAddress = InetAddress.getByName(nodeSpec.hostname.s());
+            InetAddress nodeInetAddress = environment.getInetAddressForHost(nodeSpec.hostname.s());
             String nodeIpAddress = nodeInetAddress.getHostAddress();
             final boolean isRunningIPv6 = nodeInetAddress instanceof Inet6Address;
 
-            String configServers = Environment.getConfigServerHosts().stream().map(HostName::toString).collect(Collectors.joining(","));
+            String configServers = environment.getConfigServerHosts().stream().map(HostName::toString).collect(Collectors.joining(","));
             Docker.StartContainerCommand command = docker.createStartContainerCommand(
                     nodeSpec.wantedDockerImage.get(),
                     nodeSpec.containerName,
@@ -267,7 +269,7 @@ public class DockerOperationsImpl implements DockerOperations {
         command.add("sudo");
         command.add(getDefaults().underVespaHome("libexec/vespa/node-admin/configure-container-networking.py"));
 
-        Environment.NetworkType networkType = Environment.networkType();
+        Environment.NetworkType networkType = environment.networkType();
         if (networkType != Environment.NetworkType.normal) {
             command.add("--" + networkType);
         }
@@ -318,8 +320,6 @@ public class DockerOperationsImpl implements DockerOperations {
             assert nodeSpec.wantedDockerImage.get().equals(dockerImage);
             callback.run();
         });
-
-        return;
     }
 
     private void removeContainer(final ContainerNodeSpec nodeSpec, final Container existingContainer, Orchestrator orchestrator)
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java
index 7c095c7a907..768807f1665 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.node.admin.orchestrator;
 import com.yahoo.vespa.hosted.node.admin.noderepository.NodeRepositoryImpl;
 
 import com.yahoo.vespa.hosted.node.admin.util.ConfigServerHttpRequestExecutor;
-import com.yahoo.vespa.hosted.node.admin.util.Environment;
 import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 
 import com.yahoo.vespa.orchestrator.restapi.HostApi;
@@ -35,10 +34,18 @@ public class OrchestratorImpl implements Orchestrator {
 
     private final ConfigServerHttpRequestExecutor requestExecutor;
 
-    public OrchestratorImpl(ConfigServerHttpRequestExecutor requestExecutor) {
+    // For testing
+    OrchestratorImpl(ConfigServerHttpRequestExecutor requestExecutor) {
         this.requestExecutor = requestExecutor;
     }
 
+    public OrchestratorImpl(Set<HostName> configServerHosts) {
+        if (configServerHosts.isEmpty()) {
+            throw new IllegalStateException("Environment setting for config servers missing or empty.");
+        }
+        this.requestExecutor = ConfigServerHttpRequestExecutor.create(configServerHosts);
+    }
+
     @Override
     public boolean suspend(final HostName hostName) {
         PrefixLogger logger = PrefixLogger.getNodeAgentLogger(OrchestratorImpl.class,
@@ -95,12 +102,4 @@ public class OrchestratorImpl implements Orchestrator {
             return false;
         }
     }
-
-    public static OrchestratorImpl createOrchestratorFromSettings() {
-        final Set<HostName> configServerHosts = Environment.getConfigServerHosts();
-        if (configServerHosts.isEmpty()) {
-            throw new IllegalStateException("Environment setting for config servers missing or empty.");
-        }
-        return new OrchestratorImpl(ConfigServerHttpRequestExecutor.create(configServerHosts));
-    }
 }
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 09ae4906cb6..9d3d4c31afc 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
@@ -42,17 +42,15 @@ public class ComponentsProviderImpl implements ComponentsProvider {
         String baseHostName = java.util.Optional.ofNullable(System.getenv(ENV_HOSTNAME))
                 .orElseThrow(() -> new IllegalStateException("Environment variable " + ENV_HOSTNAME + " unset"));
 
-        Set<HostName> configServerHosts = Environment.getConfigServerHosts();
-        if (configServerHosts.isEmpty()) {
-            throw new IllegalStateException("Environment setting for config servers missing or empty.");
-        }
+        Environment environment = new Environment();
+        Set<HostName> configServerHosts = environment.getConfigServerHosts();
 
+        Orchestrator orchestrator = new OrchestratorImpl(configServerHosts);
         NodeRepository nodeRepository = new NodeRepositoryImpl(configServerHosts, HARDCODED_NODEREPOSITORY_PORT, baseHostName);
-        Orchestrator orchestrator = OrchestratorImpl.createOrchestratorFromSettings();
         MaintenanceScheduler maintenanceScheduler = new MaintenanceSchedulerImpl();
 
-        final Function<HostName, NodeAgent> nodeAgentFactory = (hostName) ->
-                new NodeAgentImpl(hostName, nodeRepository, orchestrator, new DockerOperationsImpl(docker), maintenanceScheduler);
+        final Function<HostName, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName, nodeRepository,
+                orchestrator, new DockerOperationsImpl(docker, environment), maintenanceScheduler);
         final NodeAdmin nodeAdmin = new NodeAdminImpl(docker, nodeAgentFactory, maintenanceScheduler, NODE_AGENT_SCAN_INTERVAL_MILLIS);
         nodeAdminStateUpdater = new NodeAdminStateUpdater(
                 nodeRepository, nodeAdmin, INITIAL_SCHEDULER_DELAY_MILLIS, NODE_ADMIN_STATE_INTERVAL_MILLIS, orchestrator, baseHostName);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java
index ddcf2ef4242..6a47d16ee97 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java
@@ -3,6 +3,8 @@ package com.yahoo.vespa.hosted.node.admin.util;
 
 import com.yahoo.vespa.applicationmodel.HostName;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -15,14 +17,12 @@ import java.util.stream.Collectors;
  * @author bakksjo
  */
 public class Environment {
-    private Environment() {} // Prevents instantiation.
-
     private static final String ENV_CONFIGSERVERS = "services__addr_configserver";
     private static final String ENV_NETWORK_TYPE = "NETWORK_TYPE";
 
     public enum NetworkType { normal, local, vm }
 
-    public static Set<HostName> getConfigServerHosts() {
+    public Set<HostName> getConfigServerHosts() {
         final String configServerHosts = System.getenv(ENV_CONFIGSERVERS);
         if (configServerHosts == null) {
             return Collections.emptySet();
@@ -34,11 +34,15 @@ public class Environment {
                 .collect(Collectors.toSet());
     }
 
-    public static NetworkType networkType() throws IllegalArgumentException {
+    public NetworkType networkType() throws IllegalArgumentException {
         String networkTypeInEnvironment = System.getenv(ENV_NETWORK_TYPE);
         if (networkTypeInEnvironment == null) {
             return NetworkType.normal;
         }
         return NetworkType.valueOf(networkTypeInEnvironment);
     }
+
+    public InetAddress getInetAddressForHost(String hostname) throws UnknownHostException {
+        return InetAddress.getByName(hostname);
+    }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
index 4b2ef6d83e0..a36059df104 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
@@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.docker;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
-import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl;
+import com.yahoo.vespa.hosted.node.admin.util.Environment;
 import org.hamcrest.CoreMatchers;
 import org.junit.Test;
 import org.mockito.InOrder;
@@ -23,8 +23,9 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class DockerOperationsImplTest {
+    private final Environment environment = new Environment();
     private final Docker docker = mock(Docker.class);
-    private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker);
+    private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker, environment);
 
     @Test
     public void absenceOfNodeProgramIsSuccess() throws Exception {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java
index 742a0b4277c..a3d42ca3161 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ComponentsProviderWithMocks.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl;
 import com.yahoo.vespa.hosted.node.admin.provider.ComponentsProvider;
+import com.yahoo.vespa.hosted.node.admin.util.Environment;
 
 import java.util.function.Function;
 
@@ -19,13 +20,14 @@ import java.util.function.Function;
  * @author dybis
  */
 public class ComponentsProviderWithMocks implements ComponentsProvider {
-    public NodeRepoMock nodeRepositoryMock = new NodeRepoMock();
+    private NodeRepoMock nodeRepositoryMock = new NodeRepoMock();
     private MaintenanceSchedulerMock maintenanceSchedulerMock = new MaintenanceSchedulerMock();
     private OrchestratorMock orchestratorMock = new OrchestratorMock();
     private Docker dockerMock = new DockerMock();
 
-    private final Function<HostName, NodeAgent> nodeAgentFactory = (hostName) ->
-            new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock, new DockerOperationsImpl(dockerMock), maintenanceSchedulerMock);
+    private Environment environment = new Environment();
+    private final Function<HostName, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName,
+            nodeRepositoryMock, orchestratorMock, new DockerOperationsImpl(dockerMock, environment), maintenanceSchedulerMock);
     private NodeAdmin nodeAdmin = new NodeAdminImpl(dockerMock, nodeAgentFactory, maintenanceSchedulerMock, 100);