Move things around

6 files changed, 138 insertions, 80 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 7cdd8f3c735..6eadfbd5974 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -77,6 +77,7 @@ import java.security.Principal;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.EnumSet;
@@ -88,6 +89,7 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -541,7 +543,7 @@ public class ApplicationController {
         }
     }
 
-    /** Returns the endpoints of the clusters in the deployment, or empty if the request fails. */
+    /** Returns the non-empty endpoints of the clusters in the deployment, or empty if the request fails. */
     public Optional<Map<ClusterSpec.Id, URI>> clusterEndpoints(DeploymentId id) {
         if ( ! get(id.applicationId())
                 .map(application -> application.deployments().containsKey(id.zoneId()))
@@ -564,6 +566,14 @@ public class ApplicationController {
         }
     }
 
+    /** Returns all zone-specific cluster endpoints for the given application, in the given zones. */
+    public Map<ZoneId, Map<ClusterSpec.Id, URI>> clusterEndpoints(ApplicationId id, Collection<ZoneId> zones) {
+        Map<ZoneId, Map<ClusterSpec.Id, URI>> deployments = new HashMap<>();
+        for (ZoneId zone : zones)
+            clusterEndpoints(new DeploymentId(id, zone)).ifPresent(endpoints -> deployments.put(zone, endpoints));
+        return Map.copyOf(deployments);
+    }
+
     /**
      * Deletes the the given application. All known instances of the applications will be deleted,
      * including PR instances.
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index 04ff0fd2e86..82402205355 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -14,7 +14,6 @@ import com.yahoo.config.provision.AthenzService;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.ZoneId;
-import com.yahoo.data.access.simple.JsonRender;
 import com.yahoo.io.IOUtils;
 import com.yahoo.log.LogLevel;
 import com.yahoo.slime.Cursor;
@@ -49,7 +48,6 @@ import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -101,10 +99,12 @@ public class InternalStepRunner implements StepRunner {
     static final Duration installationTimeout = Duration.ofMinutes(150);
 
     private final Controller controller;
+    private final TestConfigSerializer testConfigSerializer;
     private final DeploymentFailureMails mails;
 
     public InternalStepRunner(Controller controller) {
         this.controller = controller;
+        this.testConfigSerializer = new TestConfigSerializer(controller.system());
         this.mails = new DeploymentFailureMails(controller.zoneRegistry());
     }
 
@@ -322,7 +322,7 @@ public class InternalStepRunner implements StepRunner {
 
     private boolean endpointsAvailable(ApplicationId id, ZoneId zoneId, DualLogger logger) {
         logger.log("Attempting to find deployment endpoints ...");
-        var endpoints = deploymentEndpoints(id, Set.of(zoneId));
+        var endpoints = controller.applications().clusterEndpoints(id, Set.of(zoneId));
         if ( ! endpoints.containsKey(zoneId)) {
             logger.log("Endpoints not yet ready.");
             return false;
@@ -390,10 +390,10 @@ public class InternalStepRunner implements StepRunner {
             return Optional.of(aborted);
         }
 
-        Set<ZoneId> zones = testedZoneAndProductionZones(id);
+        Set<ZoneId> zones = controller.jobController().testedZoneAndProductionZones(id.application(), id.type());
 
         logger.log("Attempting to find endpoints ...");
-        var endpoints = deploymentEndpoints(id.application(), zones);
+        var endpoints = controller.applications().clusterEndpoints(id.application(), zones);
         if ( ! endpoints.containsKey(id.type().zone(controller.system())) && timedOut(deployment.get(), endpointTimeout)) {
             logger.log(WARNING, "Endpoints for the deployment to test vanished again, while it was still active!");
             return Optional.of(error);
@@ -410,9 +410,10 @@ public class InternalStepRunner implements StepRunner {
             logger.log("Starting tests ...");
             controller.jobController().cloud().startTests(testerEndpoint.get(),
                                                           TesterCloud.Suite.of(id.type()),
-                                                          testConfig(id.application(), id.type().zone(controller.system()),
-                                                                     controller.system(), endpoints,
-                                                                     listClusters(id.application(), zones)));
+                                                          testConfigSerializer.testConfig(id.application(),
+                                                                                          id.type(),
+                                                                                          endpoints,
+                                                                                          listClusters(id.application(), zones)));
             return Optional.of(running);
         }
 
@@ -612,23 +613,6 @@ public class InternalStepRunner implements StepRunner {
         throw new IllegalStateException("No step deploys to the zone this run is for!");
     }
 
-    /** Returns a stream containing the zone of the deployment tested in the given run, and all production zones for the application. */
-    private Set<ZoneId> testedZoneAndProductionZones(RunId id) {
-        return Stream.concat(Stream.of(id.type().zone(controller.system())),
-                             application(id.application()).productionDeployments().keySet().stream())
-                     .collect(Collectors.toSet());
-    }
-
-    /** Returns all endpoints for all current deployments of the given real application. */
-    private Map<ZoneId, Map<ClusterSpec.Id, URI>> deploymentEndpoints(ApplicationId id, Collection<ZoneId> zones) {
-        ImmutableMap.Builder<ZoneId, Map<ClusterSpec.Id, URI>> deployments = ImmutableMap.builder();
-        for (ZoneId zone : zones)
-            controller.applications().clusterEndpoints(new DeploymentId(id, zone))
-                      .filter(endpoints -> ! endpoints.isEmpty())
-                      .ifPresent(endpoints -> deployments.put(zone, endpoints));
-        return deployments.build();
-    }
-
     /** Returns all content clusters in all current deployments of the given real application. */
     private Map<ZoneId, List<String>> listClusters(ApplicationId id, Iterable<ZoneId> zones) {
         ImmutableMap.Builder<ZoneId, List<String>> clusters = ImmutableMap.builder();
@@ -707,46 +691,6 @@ public class InternalStepRunner implements StepRunner {
         return deploymentSpec.getBytes(StandardCharsets.UTF_8);
     }
 
-    /** Returns the config for the tests to run for the given job. */
-    static byte[] testConfig(ApplicationId id, ZoneId testerZone, SystemName system,
-                                     Map<ZoneId, Map<ClusterSpec.Id, URI>> deployments, Map<ZoneId, List<String>> clusters) {
-        Slime slime = new Slime();
-        Cursor root = slime.setObject();
-
-        root.setString("application", id.serializedForm());
-        root.setString("zone", testerZone.value());
-        root.setString("system", system.value());
-
-        Cursor endpointsObject = root.setObject("endpoints");
-        deployments.forEach((zone, endpoints) -> {
-            Cursor endpointArray = endpointsObject.setArray(zone.value());
-            for (URI endpoint : endpoints.values())
-                endpointArray.addString(endpoint.toString());
-        });
-
-        Cursor zoneEndpointsObject = root.setObject("zoneEndpoints");
-        deployments.forEach((zone, endpoints) -> {
-            Cursor clusterEndpointsObject = zoneEndpointsObject.setObject(zone.value());
-            endpoints.forEach((cluster, endpoint) -> {
-                clusterEndpointsObject.setString(cluster.value(), endpoint.toString());
-            });
-        });
-
-        Cursor clustersObject = root.setObject("clusters");
-        clusters.forEach((zone, clusterList) -> {
-            Cursor clusterArray = clustersObject.setArray(zone.value());
-            for (String cluster : clusterList)
-                clusterArray.addString(cluster);
-        });
-
-        try {
-            return SlimeUtils.toJsonBytes(slime);
-        }
-        catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
-    }
-
     /** Logger which logs to a {@link JobController}, as well as to the parent class' {@link Logger}. */
     private class DualLogger {
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 5bb9686ce0f..e9bcb714d44 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.deployment;
 import com.google.common.collect.ImmutableMap;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -41,6 +42,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.UnaryOperator;
 import java.util.logging.Level;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static com.google.common.collect.ImmutableList.copyOf;
@@ -414,6 +416,13 @@ public class JobController {
                                              .map(policy -> policy.endpointIn(controller.system()).url()));
     }
 
+    /** Returns a set containing the zone of the deployment tested in the given run, and all production zones for the application. */
+    public Set<ZoneId> testedZoneAndProductionZones(ApplicationId id, JobType type) {
+        return Stream.concat(Stream.of(type.zone(controller.system())),
+                             controller.applications().require(id).productionDeployments().keySet().stream())
+                     .collect(Collectors.toSet());
+    }
+
     // TODO jvenstad: Find a more appropriate way of doing this, at least when this is the only build service.
     private long nextBuild(ApplicationId id) {
         return 1 + controller.applications().require(id).deploymentJobs()
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/TestConfigSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/TestConfigSerializer.java
new file mode 100644
index 00000000000..24ac7268f33
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/TestConfigSerializer.java
@@ -0,0 +1,73 @@
+package com.yahoo.vespa.hosted.controller.deployment;
+
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.slime.Cursor;
+import com.yahoo.slime.Slime;
+import com.yahoo.vespa.config.SlimeUtils;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.net.URI;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Serializes config for integration tests against Vespa deployments.
+ *
+ * @author jonmv
+ */
+public class TestConfigSerializer {
+
+    private final SystemName system;
+
+    public TestConfigSerializer(SystemName system) {
+        this.system = system;
+    }
+
+    /** Returns the config for the tests to run for the given job. */
+    public byte[] testConfig(ApplicationId id,
+                             JobType type,
+                             Map<ZoneId, Map<ClusterSpec.Id, URI>> deployments,
+                             Map<ZoneId, List<String>> clusters) {
+        Slime slime = new Slime();
+        Cursor root = slime.setObject();
+
+        root.setString("application", id.serializedForm());
+        root.setString("zone", type.zone(system).value());
+        root.setString("system", system.value());
+
+        Cursor endpointsObject = root.setObject("endpoints"); // TODO jvenstad: remove.
+        deployments.forEach((zone, endpoints) -> {
+            Cursor endpointArray = endpointsObject.setArray(zone.value());
+            for (URI endpoint : endpoints.values())
+                endpointArray.addString(endpoint.toString());
+        });
+
+        Cursor zoneEndpointsObject = root.setObject("zoneEndpoints");
+        deployments.forEach((zone, endpoints) -> {
+            Cursor clusterEndpointsObject = zoneEndpointsObject.setObject(zone.value());
+            endpoints.forEach((cluster, endpoint) -> {
+                clusterEndpointsObject.setString(cluster.value(), endpoint.toString());
+            });
+        });
+
+        Cursor clustersObject = root.setObject("clusters");
+        clusters.forEach((zone, clusterList) -> {
+            Cursor clusterArray = clustersObject.setArray(zone.value());
+            for (String cluster : clusterList)
+                clusterArray.addString(cluster);
+        });
+
+        try {
+            return SlimeUtils.toJsonBytes(slime);
+        }
+        catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index e1b4ccc0e1f..4fb6a2e34fb 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -403,20 +403,6 @@ public class InternalStepRunnerTest {
                                            "1554970337.947844\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstderr\twarning\tjava.lang.NullPointerException\\n\\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\\n\\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)";
 
     @Test
-    public void testConfig() throws IOException {
-        ZoneId zone = ZoneId.from("test", "eu-north-1");
-        byte[] json = InternalStepRunner.testConfig(appId,
-                                                    zone,
-                                                    SystemName.Public,
-                                                    Map.of(zone, Map.of(ClusterSpec.Id.from("ai"),
-                                                                        URI.create("https://server/"))),
-                                                    Map.of(zone, List.of("facts")));
-        byte[] expected = InternalStepRunnerTest.class.getResourceAsStream("/testConfig.json").readAllBytes();
-        assertEquals(new String(SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlime(expected))),
-                     new String(json));
-    }
-
-    @Test
     public void generates_correct_services_xml_test() {
         assertFile("test_runner_services.xml-cd", new String(InternalStepRunner.servicesXml(SystemName.cd, Optional.of("d-2-12-75"))));
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/TestConfigSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/TestConfigSerializerTest.java
new file mode 100644
index 00000000000..877cfa3b59b
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/TestConfigSerializerTest.java
@@ -0,0 +1,36 @@
+package com.yahoo.vespa.hosted.controller.deployment;
+
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.vespa.config.SlimeUtils;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+import java.util.Map;
+
+import static com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester.appId;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author jonmv
+ */
+public class TestConfigSerializerTest {
+
+    @Test
+    public void testConfig() throws IOException {
+        ZoneId zone = JobType.systemTest.zone(SystemName.PublicCd);
+        byte[] json = new TestConfigSerializer(SystemName.PublicCd).testConfig(appId,
+                                                                               JobType.systemTest,
+                                                                               Map.of(zone, Map.of(ClusterSpec.Id.from("ai"),
+                                                                                                   URI.create("https://server/"))),
+                                                                               Map.of(zone, List.of("facts")));
+        byte[] expected = InternalStepRunnerTest.class.getResourceAsStream("/testConfig.json").readAllBytes();
+        assertEquals(new String(SlimeUtils.toJsonBytes(SlimeUtils.jsonToSlime(expected))),
+                     new String(json));
+    }
+
+}