Merge pull request #8449 from vespa-engine/jvenstad/cache-orchestrator-host-statuses-for-reads

Jvenstad/cache orchestrator host statuses for reads

5 files changed, 27 insertions, 24 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
index 39a2787ca9b..42dbcdf7a86 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
@@ -25,6 +25,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -34,7 +35,7 @@ import java.util.stream.Collectors;
 public class MetricsReporter extends Maintainer {
 
     private final Metric metric;
-    private final Orchestrator orchestrator;
+    private final Function<HostName, Optional<HostStatus>> orchestrator;
     private final ServiceMonitor serviceMonitor;
     private final Map<Map<String, String>, Metric.Context> contextMap = new HashMap<>();
     private final Supplier<Integer> pendingRedeploymentsSupplier;
@@ -48,7 +49,7 @@ public class MetricsReporter extends Maintainer {
                            JobControl jobControl) {
         super(nodeRepository, interval, jobControl);
         this.metric = metric;
-        this.orchestrator = orchestrator;
+        this.orchestrator = orchestrator.getNodeStatuses();
         this.serviceMonitor = serviceMonitor;
         this.pendingRedeploymentsSupplier = pendingRedeploymentsSupplier;
     }
@@ -129,13 +130,9 @@ public class MetricsReporter extends Maintainer {
                 node.status().hardwareDivergence().isPresent() ? 1 : 0,
                 context);
 
-        try {
-            HostStatus status = orchestrator.getNodeStatus(new HostName(node.hostname()));
-            boolean allowedToBeDown = status == HostStatus.ALLOWED_TO_BE_DOWN;
-            metric.set("allowedToBeDown", allowedToBeDown ? 1 : 0, context);
-        } catch (HostNameNotFoundException e) {
-            // Ignore
-        }
+        orchestrator.apply(new HostName(node.hostname()))
+                    .map(status -> status == HostStatus.ALLOWED_TO_BE_DOWN ? 1 : 0)
+                    .ifPresent(allowedToBeDown -> metric.set("allowedToBeDown", allowedToBeDown, context));
 
         long numberOfServices;
         HostName hostName = new HostName(node.hostname());
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
index 2a2ca6bfd87..5b942497be8 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
@@ -22,7 +22,9 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URI;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
+import java.util.function.Function;
 
 /**
 * @author bratseth
@@ -40,7 +42,7 @@ class NodesResponse extends HttpResponse {
 
     private final NodeFilter filter;
     private final boolean recursive;
-    private final Orchestrator orchestrator;
+    private final Function<HostName, Optional<HostStatus>> orchestrator;
     private final NodeRepository nodeRepository;
     private final Slime slime;
     private final NodeSerializer serializer = new NodeSerializer();
@@ -52,7 +54,7 @@ class NodesResponse extends HttpResponse {
         this.nodeParentUrl = toNodeParentUrl(request);
         filter = NodesApiHandler.toNodeFilter(request);
         this.recursive = request.getBooleanProperty("recursive");
-        this.orchestrator = orchestrator;
+        this.orchestrator = orchestrator.getNodeStatuses();
         this.nodeRepository = nodeRepository;
 
         slime = new Slime();
@@ -158,11 +160,9 @@ class NodesResponse extends HttpResponse {
             object.setLong("currentRestartGeneration", node.allocation().get().restartGeneration().current());
             object.setString("wantedDockerImage", nodeRepository.dockerImage().withTag(node.allocation().get().membership().cluster().vespaVersion()).asString());
             object.setString("wantedVespaVersion", node.allocation().get().membership().cluster().vespaVersion().toFullString());
-            try {
-                object.setBool("allowedToBeDown", 
-                               orchestrator.getNodeStatus(new HostName(node.hostname())) == HostStatus.ALLOWED_TO_BE_DOWN);
-            }
-            catch (HostNameNotFoundException e) {/* ok */ }
+            orchestrator.apply(new HostName(node.hostname()))
+                        .map(status -> status == HostStatus.ALLOWED_TO_BE_DOWN)
+                        .ifPresent(allowedToBeDown -> object.setBool("allowedToBeDown", allowedToBeDown));
         }
         object.setLong("rebootGeneration", node.status().reboot().wanted());
         object.setLong("currentRebootGeneration", node.status().reboot().current());
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
index 70750dd6672..96ec0349fb2 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
@@ -11,7 +11,9 @@ import com.yahoo.vespa.orchestrator.status.HostStatus;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
+import java.util.function.Function;
 
 /**
  * @author bratseth
@@ -32,6 +34,11 @@ public class OrchestratorMock implements Orchestrator {
     }
 
     @Override
+    public Function<HostName, Optional<HostStatus>> getNodeStatuses() {
+        return hostName -> Optional.of(getNodeStatus(hostName));
+    }
+
+    @Override
     public void setNodeStatus(HostName hostName, HostStatus state) {}
 
     @Override
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ServiceMonitorStub.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ServiceMonitorStub.java
index 8215dc8ecd0..b9c5df9d999 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ServiceMonitorStub.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/ServiceMonitorStub.java
@@ -62,7 +62,11 @@ public class ServiceMonitorStub implements ServiceMonitor {
     }
 
     @Override
-    public Map<ApplicationInstanceReference, ApplicationInstance> getAllApplicationInstances() {
+    public ServiceModel getServiceModelSnapshot() {
+        return new ServiceModel(getAllApplicationInstances());
+    }
+
+    private Map<ApplicationInstanceReference, ApplicationInstance> getAllApplicationInstances() {
         // Convert apps information to the response payload to return
         Map<ApplicationInstanceReference, ApplicationInstance> status = new HashMap<>();
         for (Map.Entry<ApplicationId, MockDeployer.ApplicationContext> app : apps.entrySet()) {
@@ -84,8 +88,4 @@ public class ServiceMonitorStub implements ServiceMonitor {
         return status;
     }
 
-    @Override
-    public ServiceModel getServiceModelSnapshot() {
-        return new ServiceModel(getAllApplicationInstances());
-    }
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java
index 1e502439aeb..57b942d85e4 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/monitoring/MetricsReporterTest.java
@@ -38,7 +38,6 @@ import java.util.Optional;
 import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -90,7 +89,7 @@ public class MetricsReporterTest {
 
         Orchestrator orchestrator = mock(Orchestrator.class);
         ServiceMonitor serviceMonitor = mock(ServiceMonitor.class);
-        when(orchestrator.getNodeStatus(any())).thenReturn(HostStatus.NO_REMARKS);
+        when(orchestrator.getNodeStatuses()).thenReturn(hostName -> Optional.of(HostStatus.NO_REMARKS));
         ServiceModel serviceModel = mock(ServiceModel.class);
         when(serviceMonitor.getServiceModelSnapshot()).thenReturn(serviceModel);
         when(serviceModel.getServiceInstancesByHostName()).thenReturn(Collections.emptyMap());
@@ -137,7 +136,7 @@ public class MetricsReporterTest {
 
         Orchestrator orchestrator = mock(Orchestrator.class);
         ServiceMonitor serviceMonitor = mock(ServiceMonitor.class);
-        when(orchestrator.getNodeStatus(any())).thenReturn(HostStatus.NO_REMARKS);
+        when(orchestrator.getNodeStatuses()).thenReturn(hostName -> Optional.of(HostStatus.NO_REMARKS));
         ServiceModel serviceModel = mock(ServiceModel.class);
         when(serviceMonitor.getServiceModelSnapshot()).thenReturn(serviceModel);
         when(serviceModel.getServiceInstancesByHostName()).thenReturn(Collections.emptyMap());