Define suspended attribute on HostStatus, plus minor review fixes

5 files changed, 18 insertions, 12 deletions

diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfo.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfo.java
index 16db5685ae3..3dd597f2436 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfo.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfo.java
@@ -16,8 +16,8 @@ public class HostInfo {
     private final Optional<Instant> suspendedSince;
 
     public static HostInfo createSuspended(HostStatus status, Instant suspendedSince) {
-        if (status == HostStatus.NO_REMARKS) {
-            throw new IllegalArgumentException("NO_REMARKS is not a suspended-status");
+        if (!status.isSuspended()) {
+            throw new IllegalArgumentException(status + " is not a suspended-status");
         }
 
         return new HostInfo(status, Optional.of(suspendedSince));
@@ -35,8 +35,8 @@ public class HostInfo {
     public HostStatus status() { return status; }
 
     /**
-     * The instant the host status was set to != NO_REMARKS. Is preserved when transitioning
-     * between non-NO_REMARKS states. Returns empty if and only if NO_REMARKS.
+     * The instant the host status was set to a suspended status. Is preserved when transitioning
+     * between suspended statuses. Returns empty if and only if NO_REMARKS.
      */
     public Optional<Instant> suspendedSince() { return suspendedSince; }
 
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java
index 42773e22a74..cb3f9cb8afe 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java
@@ -22,7 +22,7 @@ public class HostInfos {
     /** Get all suspended hostnames. */
     public Set<HostName> suspendedHostsnames() {
         return hostInfos.entrySet().stream()
-                .filter(entry -> entry.getValue().status() != HostStatus.NO_REMARKS)
+                .filter(entry -> entry.getValue().status().isSuspended())
                 .map(entry -> entry.getKey())
                 .collect(Collectors.toSet());
     }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java
index f23917042e9..6764ffb48ea 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java
@@ -8,14 +8,19 @@ package com.yahoo.vespa.orchestrator.status;
  */
 public enum HostStatus {
     /** The services on the host is supposed to be up. */
-    NO_REMARKS,
+    NO_REMARKS(false),
 
     /** The services on the host is allowed to be down. */
-    ALLOWED_TO_BE_DOWN,
+    ALLOWED_TO_BE_DOWN(true),
 
     /**
      * Same as ALLOWED_TO_BE_DOWN, but in addition, it is expected
      * the host may be removed from its application at any moment.
      */
-    PERMANENTLY_DOWN;
+    PERMANENTLY_DOWN(true);
+
+    private final boolean suspended;
+
+    HostStatus(boolean suspended) { this.suspended = suspended; }
+    public boolean isSuspended() { return suspended; }
 }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
index ca6f3c524a2..ecf3a161c83 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
@@ -45,7 +45,6 @@ public class ZookeeperStatusService implements StatusService {
 
     final static String HOST_STATUS_BASE_PATH = "/vespa/host-status-service";
     final static String APPLICATION_STATUS_BASE_PATH = "/vespa/application-status-service";
-    final static String HOST_STATUS_CACHE_COUNTER_PATH = "/vespa/host-status-service-cache-counter";
 
     private final Curator curator;
     private final HostInfosCache hostInfosCache;
@@ -307,7 +306,9 @@ public class ZookeeperStatusService implements StatusService {
         // Once that's true we can stop writing to hosts-allowed-down, remove this code, and all
         // data in hosts-allowed-down can be removed.
         Set<HostName> legacyHostsDown = hostsDownFor(application);
-        Map<HostName, HostInfo> legacyHostInfos = legacyHostsDown.stream().collect(Collectors.toMap(
+        Map<HostName, HostInfo> legacyHostInfos = legacyHostsDown.stream()
+                .filter(hostname -> !hostInfos.containsKey(hostname))
+                .collect(Collectors.toMap(
                 hostname -> hostname,
                 hostname -> {
                     Stat stat = uncheck(() -> curator.framework()
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/json/WireHostInfo.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/json/WireHostInfo.java
index d0d2e523f50..c94384abec5 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/json/WireHostInfo.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/json/WireHostInfo.java
@@ -35,8 +35,8 @@ public class WireHostInfo {
     }
 
     public static byte[] serialize(HostInfo hostInfo) {
-        if (hostInfo.status() == HostStatus.NO_REMARKS) {
-            throw new IllegalArgumentException("Serialization of NO_REMARKS is not supported");
+        if (!hostInfo.status().isSuspended()) {
+            throw new IllegalArgumentException("Serialization of unsuspended status is not supported: " + hostInfo.status());
         }
 
         WireHostInfo wireHostInfo = new WireHostInfo();