Simplify ComponentsProvider

6 files changed, 49 insertions, 84 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProvider.java
index 3211e4feb56..93a77a13bf9 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProvider.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProvider.java
@@ -1,7 +1,6 @@
 // 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.provider;
 
-import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater;
 
 /**
@@ -11,6 +10,4 @@ import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater;
  */
 public interface ComponentsProvider {
     NodeAdminStateUpdater getNodeAdminStateUpdater();
-
-    MetricReceiverWrapper getMetricReceiverWrapper();
 }
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 98d7593ef69..1e57c55d687 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
@@ -3,10 +3,8 @@ package com.yahoo.vespa.hosted.node.admin.provider;
 
 import com.google.inject.Inject;
 import com.yahoo.net.HostName;
-import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 
 import com.yahoo.system.ProcessExecuter;
-import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
@@ -24,95 +22,53 @@ import com.yahoo.vespa.hosted.node.admin.orchestrator.Orchestrator;
 import com.yahoo.vespa.hosted.node.admin.orchestrator.OrchestratorImpl;
 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.SecretAgentScheduleMaker;
 
-import java.io.IOException;
-import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.time.Clock;
-import java.util.Set;
+import java.time.Duration;
 import java.util.function.Function;
 
+import static com.yahoo.vespa.defaults.Defaults.getDefaults;
+
 /**
  * Set up node admin for production.
  *
  * @author dybis
  */
 public class ComponentsProviderImpl implements ComponentsProvider {
-    private static final ContainerName NODE_ADMIN_CONTAINER_NAME = new ContainerName("node-admin");
-
     private final NodeAdminStateUpdater nodeAdminStateUpdater;
-    private final MetricReceiverWrapper metricReceiverWrapper;
 
-    private static final int NODE_AGENT_SCAN_INTERVAL_MILLIS = 30000;
     private static final int WEB_SERVICE_PORT = getDefaults().vespaWebServicePort();
-
-    // Converge towards desired node admin state every 30 seconds
-    private static final int NODE_ADMIN_CONVERGE_STATE_INTERVAL_MILLIS = 30000;
+    private static final Duration NODE_AGENT_SCAN_INTERVAL = Duration.ofSeconds(30);
+    private static final Duration NODE_ADMIN_CONVERGE_STATE_INTERVAL = Duration.ofSeconds(30);
 
     @Inject
     public ComponentsProviderImpl(Docker docker, MetricReceiverWrapper metricReceiver) {
-        String baseHostName = HostName.getLocalhost();
-        Environment environment = new Environment();
-        Set<String> configServerHosts = environment.getConfigServerHosts();
-        if (configServerHosts.isEmpty()) {
-            throw new IllegalStateException("Environment setting for config servers missing or empty.");
-        }
-
         Clock clock = Clock.systemUTC();
+        String dockerHostHostName = HostName.getLocalhost();
         ProcessExecuter processExecuter = new ProcessExecuter();
-        ConfigServerHttpRequestExecutor requestExecutor = ConfigServerHttpRequestExecutor.create(configServerHosts);
-        Orchestrator orchestrator = new OrchestratorImpl(requestExecutor);
-        NodeRepository nodeRepository = new NodeRepositoryImpl(requestExecutor, WEB_SERVICE_PORT, baseHostName);
+        Environment environment = new Environment();
+
+        ConfigServerHttpRequestExecutor requestExecutor = ConfigServerHttpRequestExecutor.create(environment.getConfigServerHosts());
+        NodeRepository nodeRepository = new NodeRepositoryImpl(requestExecutor, WEB_SERVICE_PORT);
+        Orchestrator orchestrator = new OrchestratorImpl(requestExecutor, WEB_SERVICE_PORT);
         DockerOperations dockerOperations = new DockerOperationsImpl(docker, environment, processExecuter);
 
         StorageMaintainer storageMaintainer = new StorageMaintainer(docker, processExecuter, metricReceiver, environment, clock);
-        AclMaintainer aclMaintainer = new AclMaintainer(dockerOperations, nodeRepository, baseHostName);
+        AclMaintainer aclMaintainer = new AclMaintainer(dockerOperations, nodeRepository, dockerHostHostName);
 
         Function<String, NodeAgent> nodeAgentFactory =
                 (hostName) -> new NodeAgentImpl(hostName, nodeRepository, orchestrator, dockerOperations,
-                        storageMaintainer, aclMaintainer, environment, clock);
+                        storageMaintainer, aclMaintainer, environment, clock, NODE_AGENT_SCAN_INTERVAL);
         NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer, aclMaintainer,
-                NODE_AGENT_SCAN_INTERVAL_MILLIS, metricReceiver, clock);
-        nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepository, nodeAdmin, storageMaintainer, clock, orchestrator, baseHostName);
-        nodeAdminStateUpdater.start(NODE_ADMIN_CONVERGE_STATE_INTERVAL_MILLIS);
-
-        metricReceiverWrapper = metricReceiver;
+                metricReceiver, clock);
 
-        setCorePattern(docker);
-        initializeNodeAgentSecretAgent(docker);
+        nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepository, orchestrator, storageMaintainer, nodeAdmin,
+                dockerHostHostName, clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL);
+        nodeAdminStateUpdater.start();
     }
 
     @Override
     public NodeAdminStateUpdater getNodeAdminStateUpdater() {
         return nodeAdminStateUpdater;
     }
-
-    @Override
-    public MetricReceiverWrapper getMetricReceiverWrapper() {
-        return metricReceiverWrapper;
-    }
-
-
-    private void setCorePattern(Docker docker) {
-        final String[] sysctlCorePattern = {"sysctl", "-w", "kernel.core_pattern=" +
-                                            getDefaults().underVespaHome("var/crash/%e.core.%p")};
-        docker.executeInContainerAsRoot(NODE_ADMIN_CONTAINER_NAME, sysctlCorePattern);
-    }
-
-    private void initializeNodeAgentSecretAgent(Docker docker) {
-        final Path yamasAgentFolder = Paths.get("/etc/yamas-agent/");
-        docker.executeInContainerAsRoot(NODE_ADMIN_CONTAINER_NAME, "chmod", "a+w", yamasAgentFolder.toString());
-
-        Path nodeAdminCheckPath = Paths.get("/usr/bin/curl");
-        SecretAgentScheduleMaker scheduleMaker = new SecretAgentScheduleMaker("node-admin", 60, nodeAdminCheckPath,
-                "localhost:4080/rest/metrics");
-
-        try {
-            scheduleMaker.writeTo(yamasAgentFolder);
-            docker.executeInContainerAsRoot(NODE_ADMIN_CONTAINER_NAME, "service", "yamas-agent", "restart");
-        } catch (IOException e) {
-            throw new RuntimeException("Failed to write secret-agent schedules for node-admin", e);
-        }
-    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java
index ff6ac9ce1e7..73c544c8c80 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/restapi/RestApiHandler.java
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater;
 import com.yahoo.vespa.hosted.node.admin.provider.ComponentsProvider;
 
+import javax.inject.Inject;
 import javax.ws.rs.core.MediaType;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -36,10 +37,13 @@ public class RestApiHandler extends LoggingRequestHandler{
     private final NodeAdminStateUpdater refresher;
     private final MetricReceiverWrapper metricReceiverWrapper;
 
-    public RestApiHandler(Executor executor, AccessLog accessLog, ComponentsProvider componentsProvider) {
+    @Inject
+    public RestApiHandler(Executor executor, AccessLog accessLog,
+                          ComponentsProvider componentsProvider,
+                          MetricReceiverWrapper metricReceiverWrapper) {
         super(executor, accessLog);
         this.refresher = componentsProvider.getNodeAdminStateUpdater();
-        this.metricReceiverWrapper = componentsProvider.getMetricReceiverWrapper();
+        this.metricReceiverWrapper = metricReceiverWrapper;
     }
 
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java
index 4434213989f..9c8dc198388 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java
@@ -52,6 +52,10 @@ public class ConfigServerHttpRequestExecutor {
     }
 
     public static ConfigServerHttpRequestExecutor create(Set<String> configServerHosts) {
+        if (configServerHosts.isEmpty()) {
+            throw new IllegalStateException("Environment setting for config servers missing or empty.");
+        }
+
         PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager();
         // Increase max total connections to 200, which should be enough
         cm.setMaxTotal(200);
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 518af5abe6b..522ad07a558 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
@@ -17,6 +17,7 @@ import com.yahoo.vespa.hosted.node.admin.provider.ComponentsProvider;
 import com.yahoo.vespa.hosted.node.admin.util.Environment;
 
 import java.time.Clock;
+import java.time.Duration;
 import java.util.function.Function;
 
 import static org.mockito.Mockito.mock;
@@ -27,6 +28,9 @@ import static org.mockito.Mockito.mock;
  * @author dybis
  */
 public class ComponentsProviderWithMocks implements ComponentsProvider {
+    private static final Duration NODE_AGENT_SCAN_INTERVAL = Duration.ofMillis(100);
+    private static final Duration NODE_ADMIN_CONVERGE_STATE_INTERVAL = Duration.ofMillis(5);
+
     static final NodeRepository nodeRepositoryMock = mock(NodeRepository.class);
     static final Orchestrator orchestratorMock = mock(Orchestrator.class);
     static final DockerOperations dockerOperationsMock = mock(DockerOperations.class);
@@ -36,22 +40,18 @@ public class ComponentsProviderWithMocks implements ComponentsProvider {
     private final Environment environment = new Environment.Builder().build();
     private final MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
     private final Function<String, NodeAgent> nodeAgentFactory =
-            (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock,
-                    dockerOperationsMock, storageMaintainer, aclMaintainer, environment, Clock.systemUTC());
-    private final NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperationsMock, nodeAgentFactory, storageMaintainer, aclMaintainer, 100, mr, Clock.systemUTC());
-    private final NodeAdminStateUpdater nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepositoryMock, nodeAdmin, storageMaintainer, Clock.systemUTC(), orchestratorMock, "localhost.test.yahoo.com");
+            (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock, dockerOperationsMock,
+                    storageMaintainer, aclMaintainer, environment, Clock.systemUTC(), NODE_AGENT_SCAN_INTERVAL);
+    private final NodeAdmin nodeAdmin = new NodeAdminImpl(dockerOperationsMock, nodeAgentFactory, storageMaintainer, aclMaintainer, mr, Clock.systemUTC());
+    private final NodeAdminStateUpdater nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepositoryMock,
+            orchestratorMock, storageMaintainer, nodeAdmin, "localhost.test.yahoo.com", Clock.systemUTC(), NODE_ADMIN_CONVERGE_STATE_INTERVAL);
 
     public ComponentsProviderWithMocks() {
-        nodeAdminStateUpdater.start(10);
+        nodeAdminStateUpdater.start();
     }
 
     @Override
     public NodeAdminStateUpdater getNodeAdminStateUpdater() {
         return nodeAdminStateUpdater;
     }
-
-    @Override
-    public MetricReceiverWrapper getMetricReceiverWrapper() {
-        return null;
-    }
 }
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 61658d4b03d..59cd1b653f9 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
@@ -21,6 +21,7 @@ import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.nio.file.Paths;
 import java.time.Clock;
+import java.time.Duration;
 import java.util.function.Function;
 
 import static org.mockito.Matchers.any;
@@ -32,6 +33,9 @@ import static org.mockito.Mockito.when;
  */
 // Need to deconstruct nodeAdminStateUpdater
 public class DockerTester implements AutoCloseable {
+    private static final Duration NODE_AGENT_SCAN_INTERVAL = Duration.ofMillis(100);
+    private static final Duration NODE_ADMIN_CONVERGE_STATE_INTERVAL = Duration.ofMillis(10);
+
     final CallOrderVerifier callOrderVerifier = new CallOrderVerifier();
     final Docker dockerMock = new DockerMock(callOrderVerifier);
     final NodeRepoMock nodeRepositoryMock = new NodeRepoMock(callOrderVerifier);
@@ -40,7 +44,7 @@ public class DockerTester implements AutoCloseable {
     private final OrchestratorMock orchestratorMock = new OrchestratorMock(callOrderVerifier);
 
 
-    public DockerTester() {
+    DockerTester() {
         InetAddressResolver inetAddressResolver = mock(InetAddressResolver.class);
         try {
             when(inetAddressResolver.getInetAddressForHost(any(String.class))).thenReturn(InetAddress.getByName("1.1.1.1"));
@@ -57,16 +61,16 @@ public class DockerTester implements AutoCloseable {
 
 
         MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
-        final DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, null);
+        DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, null);
         Function<String, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock,
-                orchestratorMock, dockerOperations, storageMaintainer, aclMaintainer, environment, clock);
-        nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer, aclMaintainer, 100, mr, Clock.systemUTC());
-        nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepositoryMock, nodeAdmin, storageMaintainer,
-                clock, orchestratorMock, "basehostname");
-        nodeAdminStateUpdater.start(5);
+                orchestratorMock, dockerOperations, storageMaintainer, aclMaintainer, environment, clock, NODE_AGENT_SCAN_INTERVAL);
+        nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, storageMaintainer, aclMaintainer, mr, Clock.systemUTC());
+        nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepositoryMock, orchestratorMock, storageMaintainer,
+                nodeAdmin, "basehostname", clock, NODE_ADMIN_CONVERGE_STATE_INTERVAL);
+        nodeAdminStateUpdater.start();
     }
 
-    public void addContainerNodeSpec(ContainerNodeSpec containerNodeSpec) {
+    void addContainerNodeSpec(ContainerNodeSpec containerNodeSpec) {
         nodeRepositoryMock.updateContainerNodeSpec(containerNodeSpec);
     }