Inject NodeRepositoryClientInterface only where needed

5 files changed, 52 insertions, 54 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index c94cbb79162..502df641313 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -21,7 +21,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.ArtifactRepo
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
 import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService;
 import com.yahoo.vespa.hosted.controller.api.integration.github.GitHub;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryClientInterface;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Organization;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.GlobalRoutingService;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus;
@@ -69,7 +68,6 @@ public class Controller extends AbstractComponent {
     private final GlobalRoutingService globalRoutingService;
     private final ZoneRegistry zoneRegistry;
     private final ConfigServer configServer;
-    private final NodeRepositoryClientInterface nodeRepository;
     private final MetricsService metricsService;
     private final Chef chef;
     private final Organization organization;
@@ -84,13 +82,13 @@ public class Controller extends AbstractComponent {
     public Controller(CuratorDb curator, RotationsConfig rotationsConfig,
                       GitHub gitHub, EntityService entityService, Organization organization,
                       GlobalRoutingService globalRoutingService,
-                      ZoneRegistry zoneRegistry, ConfigServer configServer, NodeRepositoryClientInterface nodeRepository,
+                      ZoneRegistry zoneRegistry, ConfigServer configServer,
                       MetricsService metricsService, NameService nameService,
                       RoutingGenerator routingGenerator, Chef chef, AthenzClientFactory athenzClientFactory,
                       ArtifactRepository artifactRepository, BuildService buildService) {
         this(curator, rotationsConfig,
              gitHub, entityService, organization, globalRoutingService, zoneRegistry,
-             configServer, nodeRepository, metricsService, nameService, routingGenerator, chef,
+             configServer, metricsService, nameService, routingGenerator, chef,
              Clock.systemUTC(), athenzClientFactory, artifactRepository, buildService,
              com.yahoo.net.HostName::getLocalhost);
     }
@@ -98,7 +96,7 @@ public class Controller extends AbstractComponent {
     public Controller(CuratorDb curator, RotationsConfig rotationsConfig,
                       GitHub gitHub, EntityService entityService, Organization organization,
                       GlobalRoutingService globalRoutingService,
-                      ZoneRegistry zoneRegistry, ConfigServer configServer, NodeRepositoryClientInterface nodeRepository,
+                      ZoneRegistry zoneRegistry, ConfigServer configServer,
                       MetricsService metricsService, NameService nameService,
                       RoutingGenerator routingGenerator, Chef chef, Clock clock,
                       AthenzClientFactory athenzClientFactory, ArtifactRepository artifactRepository,
@@ -107,12 +105,11 @@ public class Controller extends AbstractComponent {
         this.hostnameSupplier = Objects.requireNonNull(hostnameSupplier, "HostnameSupplier cannot be null");
         this.curator = Objects.requireNonNull(curator, "Curator cannot be null");
         this.gitHub = Objects.requireNonNull(gitHub, "GitHub cannot be null");
-        this.entityService = Objects.requireNonNull(entityService, "EntityService cannot be null");;
+        this.entityService = Objects.requireNonNull(entityService, "EntityService cannot be null");
         this.organization = Objects.requireNonNull(organization, "Organization cannot be null");
         this.globalRoutingService = Objects.requireNonNull(globalRoutingService, "GlobalRoutingService cannot be null");
-        this.zoneRegistry = Objects.requireNonNull(zoneRegistry, "ZoneRegistry cannot be null");;
-        this.configServer = Objects.requireNonNull(configServer, "ConfigServer cannot be null");;
-        this.nodeRepository = Objects.requireNonNull(nodeRepository, "NodeRepositoryClientInterface cannot be null");
+        this.zoneRegistry = Objects.requireNonNull(zoneRegistry, "ZoneRegistry cannot be null");
+        this.configServer = Objects.requireNonNull(configServer, "ConfigServer cannot be null");
         this.metricsService = Objects.requireNonNull(metricsService, "MetricsService cannot be null");
         this.chef = Objects.requireNonNull(chef, "Chef cannot be null");
         this.clock = Objects.requireNonNull(clock, "Clock cannot be null");
@@ -241,10 +238,6 @@ public class Controller extends AbstractComponent {
         return curator;
     }
 
-    public NodeRepositoryClientInterface nodeRepository() {
-        return nodeRepository;
-    }
-
     private static String printableVersion(Optional<VespaVersion> vespaVersion) {
         return vespaVersion.map(v -> v.versionNumber().toFullString()).orElse("Unknown");
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java
index 07dadf958aa..aa0ad8b63e0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java
@@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeList;
+import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryClientInterface;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
@@ -33,10 +34,13 @@ public class ClusterInfoMaintainer extends Maintainer {
     private static final Logger log = Logger.getLogger(ClusterInfoMaintainer.class.getName());
     
     private final Controller controller;
+    private final NodeRepositoryClientInterface nodeRepositoryClient;
 
-    ClusterInfoMaintainer(Controller controller, Duration duration, JobControl jobControl) {
+    ClusterInfoMaintainer(Controller controller, Duration duration, JobControl jobControl,
+                          NodeRepositoryClientInterface nodeRepositoryClient) {
         super(controller, duration, jobControl);
         this.controller = controller;
+        this.nodeRepositoryClient = nodeRepositoryClient;
     }
 
     private static String clusterid(NodeRepositoryNode node) {
@@ -89,11 +93,10 @@ public class ClusterInfoMaintainer extends Maintainer {
             for (Deployment deployment : application.deployments().values()) {
                 DeploymentId deploymentId = new DeploymentId(application.id(), deployment.zone());
                 try {
-                    NodeList nodes = controller.nodeRepository()
-                            .listNodes(deploymentId.zoneId(),
-                                       deploymentId.applicationId().tenant().value(),
-                                       deploymentId.applicationId().application().value(),
-                                       deploymentId.applicationId().instance().value());
+                    NodeList nodes = nodeRepositoryClient.listNodes(deploymentId.zoneId(),
+                                                                    deploymentId.applicationId().tenant().value(),
+                                                                    deploymentId.applicationId().application().value(),
+                                                                    deploymentId.applicationId().instance().value());
                     Map<ClusterSpec.Id, ClusterInfo> clusterInfo = getClusterInfo(nodes, deployment.zone());
                     controller().applications().lockIfPresent(application.id(), lockedApplication ->
                         controller.applications().store(lockedApplication.withClusterInfo(deployment.zone(), clusterInfo)));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index 0746b479f27..a7c600c4ae4 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -5,6 +5,7 @@ import com.yahoo.component.AbstractComponent;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
+import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryClientInterface;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentIssues;
 import com.yahoo.vespa.hosted.controller.api.integration.chef.Chef;
@@ -42,7 +43,7 @@ public class ControllerMaintenance extends AbstractComponent {
     public ControllerMaintenance(MaintainerConfig maintainerConfig, Controller controller, CuratorDb curator,
                                  JobControl jobControl, Metric metric, Chef chefClient,
                                  DeploymentIssues deploymentIssues, OwnershipIssues ownershipIssues,
-                                 NameService nameService) {
+                                 NameService nameService, NodeRepositoryClientInterface nodeRepositoryClient) {
         Duration maintenanceInterval = Duration.ofMinutes(maintainerConfig.intervalMinutes());
         this.jobControl = jobControl;
         deploymentExpirer = new DeploymentExpirer(controller, maintenanceInterval, jobControl);
@@ -52,7 +53,7 @@ public class ControllerMaintenance extends AbstractComponent {
         versionStatusUpdater = new VersionStatusUpdater(controller, Duration.ofMinutes(3), jobControl);
         upgrader = new Upgrader(controller, maintenanceInterval, jobControl, curator);
         readyJobsTrigger = new ReadyJobsTrigger(controller, Duration.ofSeconds(30), jobControl);
-        clusterInfoMaintainer = new ClusterInfoMaintainer(controller, Duration.ofHours(2), jobControl);
+        clusterInfoMaintainer = new ClusterInfoMaintainer(controller, Duration.ofHours(2), jobControl, nodeRepositoryClient);
         clusterUtilizationMaintainer = new ClusterUtilizationMaintainer(controller, Duration.ofHours(2), jobControl);
         deploymentMetricsMaintainer = new DeploymentMetricsMaintainer(controller, Duration.ofMinutes(10), jobControl);
         applicationOwnershipConfirmer = new ApplicationOwnershipConfirmer(controller, Duration.ofHours(12), jobControl, ownershipIssues);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index 614310d9221..b78bd955eb0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -28,12 +28,12 @@ import com.yahoo.vespa.hosted.controller.api.integration.entity.MemoryEntityServ
 import com.yahoo.vespa.hosted.controller.api.integration.github.GitHubMock;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.MockOrganization;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.MemoryGlobalRoutingService;
+import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock;
 import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock;
-import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService;
 import com.yahoo.vespa.hosted.controller.integration.MockMetricsService;
 import com.yahoo.vespa.hosted.controller.persistence.ApplicationSerializer;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
@@ -72,28 +72,26 @@ public final class ControllerTester {
 
     private Controller controller;
 
-    public ControllerTester() {
-        this(new AthenzDbMock(), new ManualClock(), new ConfigServerMock(),
-             new ZoneRegistryMock(), new GitHubMock(), new MockCuratorDb(), defaultRotationsConfig(),
+    public ControllerTester(ManualClock clock, RotationsConfig rotationsConfig, MockCuratorDb curatorDb) {
+        this(new AthenzDbMock(), clock, new ConfigServerMock(),
+             new ZoneRegistryMock(), new GitHubMock(), curatorDb, rotationsConfig,
              new MemoryNameService(), new ArtifactRepositoryMock(), new MemoryEntityService(), new MockBuildService());
     }
 
     public ControllerTester(ManualClock clock) {
-        this(new AthenzDbMock(), clock, new ConfigServerMock(),
-             new ZoneRegistryMock(), new GitHubMock(), new MockCuratorDb(), defaultRotationsConfig(),
-             new MemoryNameService(), new ArtifactRepositoryMock(), new MemoryEntityService(), new MockBuildService());
+        this(clock, defaultRotationsConfig(), new MockCuratorDb());
     }
 
     public ControllerTester(RotationsConfig rotationsConfig) {
-        this(new AthenzDbMock(), new ManualClock(), new ConfigServerMock(),
-             new ZoneRegistryMock(), new GitHubMock(), new MockCuratorDb(), rotationsConfig, new MemoryNameService(),
-             new ArtifactRepositoryMock(), new MemoryEntityService(), new MockBuildService());
+        this(new ManualClock(), rotationsConfig, new MockCuratorDb());
     }
 
     public ControllerTester(MockCuratorDb curatorDb) {
-        this(new AthenzDbMock(), new ManualClock(), new ConfigServerMock(),
-             new ZoneRegistryMock(), new GitHubMock(), curatorDb, defaultRotationsConfig(),
-             new MemoryNameService(), new ArtifactRepositoryMock(), new MemoryEntityService(), new MockBuildService());
+        this(new ManualClock(), defaultRotationsConfig(), curatorDb);
+    }
+
+    public ControllerTester() {
+        this(new ManualClock());
     }
 
     private ControllerTester(AthenzDbMock athenzDb, ManualClock clock,
@@ -115,7 +113,7 @@ public final class ControllerTester {
         this.controller = createController(curator, rotationsConfig, configServer, clock, gitHub, zoneRegistry,
                                            athenzDb, nameService, artifactRepository, entityService, buildService);
 
-        // Set the log output from the root logger to use timestamps from the manual clock ;)
+        // Make root logger use time from manual clock
         Logger.getLogger("").getHandlers()[0].setFilter(
                 record -> {
                     record.setMillis(clock.millis());
@@ -141,12 +139,8 @@ public final class ControllerTester {
 
     public ConfigServerMock configServer() { return configServer; }
 
-    public GitHubMock gitHub() { return gitHub; }
-
     public ArtifactRepositoryMock artifactRepository() { return artifactRepository; }
 
-    public EntityService entityService() { return entityService; }
-
     public MockBuildService buildService() { return buildService; }
 
     /** Create a new controller instance. Useful to verify that controller state is rebuilt from persistence */
@@ -254,7 +248,7 @@ public final class ControllerTester {
                                                       new DeployOptions(Optional.of(new ScrewdriverBuildJob(app1ScrewdriverId, app1RevisionId)), Optional.empty(), false, deployCurrentVersion));
     }
 
-    // Used by ApplicationSerializerTest to avoid breaking encapsulation. Should not be used by anything else
+    /** Used by ApplicationSerializerTest to avoid breaking encapsulation. Should not be used by anything else */
     public static LockedApplication writable(Application application) {
         return new LockedApplication(application, new Lock("/test", new MockCurator()));
     }
@@ -273,7 +267,6 @@ public final class ControllerTester {
                                                new MemoryGlobalRoutingService(),
                                                zoneRegistryMock,
                                                configServer,
-                                               new NodeRepositoryClientMock(),
                                                new MockMetricsService(),
                                                nameService,
                                                new MockRoutingGenerator(),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainerTest.java
index 13919cefd3b..ba6a858c18e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainerTest.java
@@ -5,13 +5,15 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
+import com.yahoo.vespa.hosted.controller.NodeRepositoryClientMock;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
-import org.junit.Assert;
 import org.junit.Test;
 
 import java.time.Duration;
 
+import static org.junit.Assert.assertEquals;
+
 /**
  * @author smorgrav
  */
@@ -20,19 +22,25 @@ public class ClusterInfoMaintainerTest {
     @Test
     public void maintain() {
         ControllerTester tester = new ControllerTester();
-        ApplicationId app = tester.createAndDeploy("tenant1", "domain1", "app1", Environment.dev, 123).id();
+        ApplicationId app = tester.createAndDeploy("tenant1", "domain1", "app1",
+                                                   Environment.dev, 123).id();
 
         // Precondition: no cluster info attached to the deployments
-        Deployment deployment = tester.controller().applications().get(app).get().deployments().values().stream().findAny().get();
-        Assert.assertEquals(0, deployment.clusterInfo().size());
-
-        ClusterInfoMaintainer mainainer = new ClusterInfoMaintainer(tester.controller(), Duration.ofHours(1), new JobControl(new MockCuratorDb()));
-        mainainer.maintain();
-
-        deployment = tester.controller().applications().get(app).get().deployments().values().stream().findAny().get();
-        Assert.assertEquals(2, deployment.clusterInfo().size());
-        Assert.assertEquals(10, deployment.clusterInfo().get(ClusterSpec.Id.from("clusterA")).getFlavorCost());
-
+        Deployment deployment = tester.controller().applications().get(app).get().deployments().values().stream()
+                                      .findFirst()
+                                      .get();
+        assertEquals(0, deployment.clusterInfo().size());
+
+        ClusterInfoMaintainer maintainer = new ClusterInfoMaintainer(tester.controller(), Duration.ofHours(1),
+                                                                     new JobControl(new MockCuratorDb()),
+                                                                     new NodeRepositoryClientMock());
+        maintainer.maintain();
+
+        deployment = tester.controller().applications().get(app).get().deployments().values().stream()
+                           .findFirst()
+                           .get();
+        assertEquals(2, deployment.clusterInfo().size());
+        assertEquals(10, deployment.clusterInfo().get(ClusterSpec.Id.from("clusterA")).getFlavorCost());
     }
 
-}
\ No newline at end of file
+}