Merge pull request #5578 from vespa-engine/mpolden/health-status-use-rotation-name

Use rotation FQDN when checking health status

6 files changed, 24 insertions, 17 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 baf03485beb..9ccfdd3642e 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
@@ -363,7 +363,7 @@ public class ApplicationController {
     private LockedApplication withRotation(LockedApplication application, ZoneId zone) {
         if (zone.environment() == Environment.prod && application.deploymentSpec().globalServiceId().isPresent()) {
             try (RotationLock rotationLock = rotationRepository.lock()) {
-                Rotation rotation = rotationRepository.getRotation(application, rotationLock);
+                Rotation rotation = rotationRepository.getOrAssignRotation(application, rotationLock);
                 application = application.with(rotation.id());
                 store(application); // store assigned rotation even if deployment fails
 
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 f63dc52fb1e..b99a7ec1fcd 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
@@ -29,6 +29,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.persistence.ControllerDb;
 import com.yahoo.vespa.hosted.controller.persistence.ControllerDbProxy;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
+import com.yahoo.vespa.hosted.controller.rotation.Rotation;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 import com.yahoo.vespa.hosted.rotation.config.RotationsConfig;
@@ -154,8 +155,8 @@ public class Controller extends AbstractComponent {
 
     public ZoneRegistry zoneRegistry() { return zoneRegistry; }
 
-    public Map<String, RotationStatus> getHealthStatus(String hostname) {
-        return globalRoutingService.getHealthStatus(hostname);
+    public Map<String, RotationStatus> rotationStatus(Rotation rotation) {
+        return globalRoutingService.getHealthStatus(rotation.name());
     }
 
     // TODO: Model the response properly
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index aebd23f2386..ab369a1baa5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -411,12 +411,10 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
             deploymentObject.setString("environment", deployment.zone().environment().value());
             deploymentObject.setString("region", deployment.zone().region().value());
             deploymentObject.setString("instance", application.id().instance().value()); // pointless
-            if (application.rotation().isPresent()) {
-                Map<String, RotationStatus> rotationHealthStatus = application.rotation()
-                                                                              .map(rotation -> controller.getHealthStatus(rotation.dnsName()))
-                                                                              .orElse(Collections.emptyMap());
+            controller.applications().rotationRepository().getRotation(application).ifPresent(rotation -> {
+                Map<String, RotationStatus> rotationHealthStatus = controller.rotationStatus(rotation);
                 setRotationStatus(deployment, rotationHealthStatus, deploymentObject);
-            }
+            });
 
             if (recurseOverDeployments(request)) // List full deployment information when recursive.
                 toSlime(deploymentObject, new DeploymentId(application.id(), deployment.zone()), deployment, request);
@@ -572,11 +570,14 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         Slime slime = new Slime();
         Cursor response = slime.setObject();
 
-        Map<String, RotationStatus> rotationHealthStatus = controller.getHealthStatus(application.rotation().get().dnsName());
-        for (String rotationEndpoint : rotationHealthStatus.keySet()) {
+        Map<String, RotationStatus> rotationStatus = controller.applications().rotationRepository()
+                                                               .getRotation(application)
+                                                               .map(controller::rotationStatus)
+                                                               .orElseGet(Collections::emptyMap);
+        for (String rotationEndpoint : rotationStatus.keySet()) {
             if (rotationEndpoint.contains(toDns(environment)) && rotationEndpoint.contains(toDns(region))) {
                 Cursor bcpStatusObject = response.setObject("bcpStatus");
-                bcpStatusObject.setString("rotationStatus", rotationHealthStatus.getOrDefault(rotationEndpoint, RotationStatus.UNKNOWN).name());
+                bcpStatusObject.setString("rotationStatus", rotationStatus.getOrDefault(rotationEndpoint, RotationStatus.UNKNOWN).name());
             }
         }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java
index c0d3fd4758e..08aedcd12b0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepository.java
@@ -13,6 +13,7 @@ import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -45,6 +46,11 @@ public class RotationRepository {
         return new RotationLock(curator.lockRotations());
     }
 
+    /** Get rotation for given application */
+    public Optional<Rotation> getRotation(Application application) {
+        return application.rotation().map(r -> allRotations.get(r.id()));
+    }
+
     /**
      * Returns a rotation for the given application
      *
@@ -54,7 +60,7 @@ public class RotationRepository {
      * @param application The application requesting a rotation
      * @param lock Lock which must be acquired by the caller
      */
-    public Rotation getRotation(Application application, RotationLock lock) {
+    public Rotation getOrAssignRotation(Application application, RotationLock lock) {
         if (application.rotation().isPresent()) {
             return allRotations.get(application.rotation().get().id());
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java
index c73bf9ac34e..f13c65a06fe 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java
@@ -68,7 +68,7 @@ public class RotationTest {
         assertEquals(URI.create("http://app1.tenant1.global.vespa.yahooapis.com:4080/"),
                      application.rotation().get().url());
         try (RotationLock lock = repository.lock()) {
-            Rotation rotation = repository.getRotation(tester.applications().require(application.id()), lock);
+            Rotation rotation = repository.getOrAssignRotation(tester.applications().require(application.id()), lock);
             assertEquals(expected, rotation);
         }
 
@@ -93,7 +93,7 @@ public class RotationTest {
         application = tester.applications().require(application.id());
 
         try (RotationLock lock = repository.lock()) {
-            Rotation rotation = repository.getRotation(application, lock);
+            Rotation rotation = repository.getOrAssignRotation(application, lock);
             Rotation assignedRotation = new Rotation(new RotationId("foo-1"), "foo-1.com");
             assertEquals(assignedRotation, rotation);
         }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index effa8f17d52..7fc305d397f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.provision.persistence;
 import com.google.common.util.concurrent.UncheckedTimeoutException;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationLockException;
-import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.log.LogLevel;
@@ -217,9 +216,9 @@ public class CuratorDatabaseClient {
         return node.status();
     }
 
-    /** In automated test environments, nodes need to be reused quickly to achieve fast test turnaronud time */
+    /** In automated test environments, nodes need to be reused quickly to achieve fast test turnaround time */
     private boolean needsFastNodeReuse(Zone zone) {
-        return zone.environment() == Environment.staging || zone.environment() == Environment.test;
+        return zone.environment().isTest();
     }
 
     /**