Split isSuspended and isHealthy, and show only healthy content clusters

4 files changed, 27 insertions, 19 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 13e3b0a0a89..b92b6bc12f5 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
@@ -249,16 +249,13 @@ public class ApplicationController {
 
     public ApplicationStore applicationStore() {  return applicationStore; }
 
-    /** Returns all content clusters in all current deployments of the given application. */
-    public Map<ZoneId, List<String>> contentClustersByZone(Collection<DeploymentId> ids) {
+    /** Returns all currently reachable content clusters among the given deployments. */
+    public Map<ZoneId, List<String>> reachableContentClustersByZone(Collection<DeploymentId> ids) {
         Map<ZoneId, List<String>> clusters = new TreeMap<>(Comparator.comparing(ZoneId::value));
         for (DeploymentId id : ids)
-            try {
+            if (isHealthy(id))
                 clusters.put(id.zoneId(), List.copyOf(configServer.getContentClusters(id)));
-            }
-            catch (RuntimeException e) {
-                log.log(Level.WARNING, "Failed getting content clusters for " + id + ": " + Exceptions.toMessageString(e));
-            }
+
         return Collections.unmodifiableMap(clusters);
     }
 
@@ -745,6 +742,20 @@ public class ApplicationController {
     }
 
     /**
+     * Asks the config server whether this deployment is currently healthy, i.e., serving traffic as usual.
+     * If this cannot be ascertained, we must assumed it is not.
+     */
+    public boolean isHealthy(DeploymentId deploymentId) {
+        try {
+            return ! isSuspended(deploymentId); // consider adding checks again global routing status, etc.?
+        }
+        catch (RuntimeException e) {
+            log.log(Level.WARNING, "Failed getting suspension status of " + deploymentId + ": " + Exceptions.toMessageString(e));
+            return false;
+        }
+    }
+
+    /**
      * Asks the config server whether this deployment is currently <i>suspended</i>:
      * Not in a state where it should receive traffic.
      */
@@ -752,14 +763,11 @@ public class ApplicationController {
         try {
             return configServer.isSuspended(deploymentId);
         }
-        catch (RuntimeException e) {
-            if (   e instanceof ConfigServerException
-                && ((ConfigServerException) e).getErrorCode() == ConfigServerException.ErrorCode.NOT_FOUND)
+        catch (ConfigServerException e) {
+            if (e.getErrorCode() == ConfigServerException.ErrorCode.NOT_FOUND)
                 return false; // If the application wasn't found, it's not suspended.
 
-            // Otherwise, assume it is, as the deployment may not be in a working state.
-            log.log(Level.WARNING, "Failed getting suspension status of " + deploymentId + ": " + Exceptions.toMessageString(e));
-            return true;
+            throw e;
         }
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index 2bdfd9758d5..c602ebd856a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -285,7 +285,7 @@ public class DeploymentTrigger {
                     status.jobSteps().get(job).readyAt(status.application().require(job.application().instance()).change())
                           .filter(readyAt -> ! clock.instant().isBefore(readyAt))
                           .filter(__ -> ! status.jobs().get(job).get().isRunning())
-                          .filter(__ -> ! (job.type().isProduction() && isSuspendedInAnotherZone(status.application(), job)))
+                          .filter(__ -> ! (job.type().isProduction() && isUnhealthyInAnotherZone(status.application(), job)))
                           .ifPresent(readyAt -> {
                               jobs.add(deploymentJob(status.application().require(job.application().instance()),
                                                      versions,
@@ -297,11 +297,11 @@ public class DeploymentTrigger {
         return Collections.unmodifiableList(jobs);
     }
 
-    /** Returns whether the application is suspended in any production zone. */
-    private boolean isSuspendedInAnotherZone(Application application, JobId job) {
+    /** Returns whether the application is healthy in all other production zones. */
+    private boolean isUnhealthyInAnotherZone(Application application, JobId job) {
         for (Deployment deployment : application.require(job.application().instance()).productionDeployments().values()) {
             if (   ! deployment.zone().equals(job.type().zone(controller.system()))
-                &&   controller.applications().isSuspended(new DeploymentId(job.application(), deployment.zone())))
+                && ! controller.applications().isHealthy(new DeploymentId(job.application(), deployment.zone())))
                 return true;
         }
         return false;
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 0d47d738bed..373967df6c4 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
@@ -578,7 +578,7 @@ public class InternalStepRunner implements StepRunner {
                                                         id.type(),
                                                         true,
                                                         endpoints,
-                                                        controller.applications().contentClustersByZone(deployments));
+                                                        controller.applications().reachableContentClustersByZone(deployments));
         controller.jobController().cloud().startTests(getTesterDeploymentId(id), suite, config);
         return Optional.of(running);
     }
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 117ac9bbf4b..8b567bbe299 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
@@ -1842,7 +1842,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                                                                       type,
                                                                       false,
                                                                       controller.routing().zoneEndpointsOf(deployments),
-                                                                      controller.applications().contentClustersByZone(deployments)));
+                                                                      controller.applications().reachableContentClustersByZone(deployments)));
     }
 
     private static SourceRevision toSourceRevision(Inspector object) {