Check endpoints as part of install steps too

3 files changed, 50 insertions, 20 deletions

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 fdadae065d2..401c8a5becb 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
@@ -269,8 +269,14 @@ public class InternalStepRunner implements StepRunner {
 
         if (   nodesConverged(id.application(), id.type(), platform, logger)
             && servicesConverged(id.application(), id.type(), logger)) {
-            logger.log("Installation succeeded!");
-            return Optional.of(running);
+            if (endpointsAvailable(id.application(), id.type().zone(controller.system()), logger)) {
+                logger.log("Installation succeeded!");
+                return Optional.of(running);
+            }
+            else if (timedOut(deployment.get(), endpointTimeout)) {
+                logger.log(WARNING, "Endpoints failed to show up within " + endpointTimeout.toMinutes() + " minutes!");
+                return Optional.of(error);
+            }
         }
 
         if (timedOut(deployment.get(), installationTimeout)) {
@@ -293,8 +299,14 @@ public class InternalStepRunner implements StepRunner {
         logger.log("Checking installation of tester container ...");
         if (   nodesConverged(id.tester().id(), id.type(), platform, logger)
             && servicesConverged(id.tester().id(), id.type(), logger)) {
-            logger.log("Tester container successfully installed!");
-            return Optional.of(running);
+            if (endpointsAvailable(id.tester().id(), id.type().zone(controller.system()), logger)) {
+                logger.log("Tester container successfully installed!");
+                return Optional.of(running);
+            }
+            else if (timedOut(deployment.get(), endpointTimeout)) {
+                logger.log(WARNING, "Tester failed to show up within " + endpointTimeout.toMinutes() + " minutes!");
+                return Optional.of(error);
+            }
         }
 
         if (timedOut(deployment.get(), installationTimeout)) {
@@ -306,6 +318,19 @@ public class InternalStepRunner implements StepRunner {
         return Optional.empty();
     }
 
+    private boolean endpointsAvailable(ApplicationId id, ZoneId zoneId, DualLogger logger) {
+        logger.log("Attempting to find deployment endpoints ...");
+        Map<ZoneId, List<URI>> endpoints = deploymentEndpoints(id, Set.of(zoneId));
+        List<String> messages = new ArrayList<>();
+        messages.add("Found endpoints");
+        endpoints.forEach((zone, uris) -> {
+            messages.add("- " + zone);
+            uris.forEach(uri -> messages.add(" |-- " + uri));
+        });
+        logger.log(messages);
+        return endpoints.containsKey(zoneId);
+    }
+
     private boolean nodesConverged(ApplicationId id, JobType type, Version target, DualLogger logger) {
         List<Node> nodes = controller.configServer().nodeRepository().list(type.zone(controller.system()), id, ImmutableSet.of(active, reserved));
         List<String> statuses = nodes.stream()
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
index 096a41e5b3f..20757820a4b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
@@ -182,6 +182,7 @@ public class InternalDeploymentTester {
         if (type == JobType.stagingTest) { // Do the initial deployment and installation of the real application.
             assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.installInitialReal));
             tester.configServer().convergeServices(appId, zone);
+            setEndpoints(appId, zone);
             run.versions().sourcePlatform().ifPresent(version -> tester.configServer().nodeRepository().doUpgrade(deployment, Optional.empty(), version));
             runner.run();
             assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.installInitialReal));
@@ -193,6 +194,12 @@ public class InternalDeploymentTester {
         assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.installReal));
         tester.configServer().convergeServices(appId, zone);
         runner.run();
+        if (   ! (run.versions().sourceApplication().isPresent() && type.isProduction())
+            &&   type != JobType.stagingTest) {
+            assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.installReal));
+            setEndpoints(appId, zone);
+        }
+        runner.run();
         assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.installReal));
 
         assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.installTester));
@@ -201,17 +208,12 @@ public class InternalDeploymentTester {
         assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.installTester));
         tester.configServer().convergeServices(testerId.id(), zone);
         runner.run();
-        assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.installTester));
-
-        // All installation is complete. We now need endpoints, and the tests will then run, and cleanup finish.
-        assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.startTests));
+        assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.installTester));
         setEndpoints(testerId.id(), zone);
         runner.run();
-        if (!run.versions().sourceApplication().isPresent() || !type.isProduction()) {
-            assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.startTests));
-            setEndpoints(appId, zone);
-        }
-        runner.run();
+        assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.installTester));
+
+        // All installation is complete and endpoints are ready, so tests may begin.
         assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.startTests));
 
         assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.endTests));
@@ -222,7 +224,7 @@ public class InternalDeploymentTester {
         assertEquals(type.isProduction(), app().deployments().containsKey(zone));
         assertTrue(tester.configServer().nodeRepository().list(zone, testerId.id()).isEmpty());
 
-        if (!app().deployments().containsKey(zone))
+        if ( ! app().deployments().containsKey(zone))
             routing.removeEndpoints(deployment);
         routing.removeEndpoints(new DeploymentId(testerId.id(), zone));
     }
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 b53a7b39d61..02a4bfcdbe6 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
@@ -18,6 +18,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud;
+import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import org.junit.Before;
@@ -156,6 +157,10 @@ public class InternalStepRunnerTest {
     public void waitsForEndpointsAndTimesOut() {
         tester.newRun(JobType.systemTest);
 
+        // Tester fails to show up for staging tests, and the real deployment for system tests.
+        tester.setEndpoints(testerId.id(), JobType.systemTest.zone(tester.tester().controller().system()));
+        tester.setEndpoints(appId, JobType.stagingTest.zone(tester.tester().controller().system()));
+
         tester.runner().run();
         tester.configServer().convergeServices(appId, JobType.stagingTest.zone(tester.tester().controller().system()));
         tester.runner().run();
@@ -165,14 +170,10 @@ public class InternalStepRunnerTest {
         tester.configServer().convergeServices(testerId.id(), JobType.stagingTest.zone(tester.tester().controller().system()));
         tester.runner().run();
 
-        // Tester fails to show up for system tests, and the real deployment for staging tests.
-        tester.setEndpoints(appId, JobType.systemTest.zone(tester.tester().controller().system()));
-        tester.setEndpoints(testerId.id(), JobType.stagingTest.zone(tester.tester().controller().system()));
-
         tester.clock().advance(InternalStepRunner.endpointTimeout.plus(Duration.ofSeconds(1)));
         tester.runner().run();
-        assertEquals(failed, tester.jobs().last(appId, JobType.systemTest).get().steps().get(Step.startTests));
-        assertEquals(failed, tester.jobs().last(appId, JobType.stagingTest).get().steps().get(Step.startTests));
+        assertEquals(failed, tester.jobs().last(appId, JobType.systemTest).get().steps().get(Step.installReal));
+        assertEquals(failed, tester.jobs().last(appId, JobType.stagingTest).get().steps().get(Step.installTester));
     }
 
     @Test
@@ -180,6 +181,7 @@ public class InternalStepRunnerTest {
         tester.newRun(JobType.systemTest);
         tester.runner().run();
         tester.configServer().convergeServices(appId, JobType.systemTest.zone(tester.tester().controller().system()));
+        tester.setEndpoints(appId, JobType.systemTest.zone(tester.tester().controller().system()));
         tester.runner().run();
         assertEquals(succeeded, tester.jobs().last(appId, JobType.systemTest).get().steps().get(Step.installReal));
 
@@ -299,6 +301,7 @@ public class InternalStepRunnerTest {
 
         tester.runner().run(); // Job run order determined by JobType enum order per application.
         tester.configServer().convergeServices(appId, zone);
+        tester.setEndpoints(appId, zone);
         assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installReal));
         assertEquals(otherPackage.hash(), tester.configServer().application(appId).get().applicationPackage().hash());