Merge pull request #2494 from yahoo/hakon/adds-safe-setting-of-wanted-state-down

Safely set storage node to DOWN

5 files changed, 256 insertions, 46 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
index df0d2f78c61..c9d9645735c 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
@@ -6,10 +6,13 @@ import com.yahoo.vdslib.state.Node;
 import com.yahoo.vdslib.state.NodeState;
 import com.yahoo.vdslib.state.NodeType;
 import com.yahoo.vdslib.state.State;
+import com.yahoo.vespa.clustercontroller.core.hostinfo.HostInfo;
+import com.yahoo.vespa.clustercontroller.core.hostinfo.Metrics;
 import com.yahoo.vespa.clustercontroller.core.hostinfo.StorageNode;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest;
 
 import java.util.List;
+import java.util.Optional;
 
 /**
  * Checks if a node can be upgraded.
@@ -17,6 +20,7 @@ import java.util.List;
  * @author dybis
  */
 public class NodeStateChangeChecker {
+    public static final String BUCKETS_METRIC_NAME = "vds.datastored.alldisks.buckets";
 
     private final int minStorageNodesUp;
     private double minRatioOfStorageNodesUp;
@@ -95,6 +99,11 @@ public class NodeStateChangeChecker {
                     "Requested node type: " + node.getType().toString());
         }
 
+        NodeInfo nodeInfo = clusterInfo.getStorageNodeInfo(node.getIndex());
+        if (nodeInfo == null) {
+            return Result.createDisallowed("Unknown node " + node);
+        }
+
         // If the new state and description equals the existing, we're done. This is done for 2 cases:
         // - We can short-circuit setting of a new wanted state, which e.g. hits ZooKeeper.
         // - We ensure that clients that have previously set the wanted state, continue
@@ -106,35 +115,73 @@ public class NodeStateChangeChecker {
 
         switch (newState.getState()) {
             case UP:
-                return canSetStateUp(node, oldState.getState());
+                return canSetStateUp(nodeInfo, oldState.getState());
             case MAINTENANCE:
-                return canSetStateMaintenance(node, clusterState);
+                return canSetStateMaintenanceTemporarily(node, clusterState);
+            case DOWN:
+                return canSetStateDownPermanently(nodeInfo, clusterState);
             default:
-                return Result.createDisallowed("Safe only supports state UP and MAINTENANCE, you tried: " + newState);
+                return Result.createDisallowed("Destination node state unsupported in safe mode: " + newState);
         }
     }
 
-    private Result canSetStateUp(Node node, State oldState) {
+    private Result canSetStateDownPermanently(NodeInfo nodeInfo, ClusterState clusterState) {
+        State reportedState = nodeInfo.getReportedState().getState();
+        if (reportedState != State.UP) {
+            return Result.createDisallowed("Reported state (" + reportedState
+                    + ") is not UP, so no bucket data is available");
+        }
+
+        State currentState = clusterState.getNodeState(nodeInfo.getNode()).getState();
+        if (currentState != State.RETIRED) {
+            return Result.createDisallowed("Only retired nodes are allowed to be set to DOWN in safe mode - is "
+                    + currentState);
+        }
+
+        Result thresholdCheckResult = checkUpThresholds(clusterState);
+        if (!thresholdCheckResult.settingWantedStateIsAllowed()) {
+            return thresholdCheckResult;
+        }
+
+        HostInfo hostInfo = nodeInfo.getHostInfo();
+        Integer hostInfoNodeVersion = hostInfo.getClusterStateVersionOrNull();
+        int clusterControllerVersion = clusterState.getVersion();
+        if (hostInfoNodeVersion == null || hostInfoNodeVersion != clusterControllerVersion) {
+            return Result.createDisallowed("Cluster controller at version " + clusterControllerVersion
+                    + " got info for storage node " + nodeInfo.getNodeIndex() + " at a different version "
+                    + hostInfoNodeVersion);
+        }
+
+        Optional<Metrics.Value> bucketsMetric = hostInfo.getMetrics().getValue(BUCKETS_METRIC_NAME);
+        if (!bucketsMetric.isPresent() || bucketsMetric.get().getLast() == null) {
+            return Result.createDisallowed("Missing last value of the " + BUCKETS_METRIC_NAME +
+                    " metric for storage node " + nodeInfo.getNodeIndex());
+        }
+
+        long lastBuckets = bucketsMetric.get().getLast();
+        if (lastBuckets > 0) {
+            return Result.createDisallowed("The storage node manages " + lastBuckets + " buckets");
+        }
+
+        return Result.allowSettingOfWantedState();
+    }
+
+    private Result canSetStateUp(NodeInfo nodeInfo, State oldState) {
         if (oldState != State.MAINTENANCE) {
             return Result.createDisallowed("Refusing to set wanted state to up when it is currently in " + oldState);
         }
 
-        if (clusterInfo.getNodeInfo(node).getReportedState().getState() != State.UP) {
+        if (nodeInfo.getReportedState().getState() != State.UP) {
             return Result.createDisallowed("Refuse to set wanted state to UP, " +
                     "since the reported state is not UP (" +
-                    clusterInfo.getNodeInfo(node).getReportedState().getState() + ")");
+                    nodeInfo.getReportedState().getState() + ")");
         }
 
         return Result.allowSettingOfWantedState();
     }
 
-    private Result canSetStateMaintenance(Node node, ClusterState clusterState) {
-        NodeInfo nodeInfo = clusterInfo.getNodeInfo(node);
-        if (nodeInfo == null) {
-            return Result.createDisallowed("Unknown node " + node);
-        }
-        NodeState currentState = clusterState.getNodeState(node);
-        if (currentState.getState() == State.DOWN) {
+    private Result canSetStateMaintenanceTemporarily(Node node, ClusterState clusterState) {
+        if (clusterState.getNodeState(node).getState() == State.DOWN) {
             return Result.allowSettingOfWantedState();
         }
 
@@ -148,13 +195,9 @@ public class NodeStateChangeChecker {
             return ongoingChanges;
         }
 
-        if (clusterInfo.getStorageNodeInfo().size() < minStorageNodesUp) {
-            return Result.createDisallowed("There are only " + clusterInfo.getStorageNodeInfo().size() +
-                    " storage nodes up, while config requires at least " + minStorageNodesUp);
-        }
-        Result fractionCheck = isFractionHighEnough(clusterState);
-        if (!fractionCheck.settingWantedStateIsAllowed()) {
-            return fractionCheck;
+        Result thresholdCheckResult = checkUpThresholds(clusterState);
+        if (!thresholdCheckResult.settingWantedStateIsAllowed()) {
+            return thresholdCheckResult;
         }
 
         return Result.allowSettingOfWantedState();
@@ -182,17 +225,22 @@ public class NodeStateChangeChecker {
         return upNodesCount;
     }
 
-    private Result isFractionHighEnough(ClusterState clusterState) {
+    private Result checkUpThresholds(ClusterState clusterState) {
+        if (clusterInfo.getStorageNodeInfo().size() < minStorageNodesUp) {
+            return Result.createDisallowed("There are only " + clusterInfo.getStorageNodeInfo().size() +
+                    " storage nodes up, while config requires at least " + minStorageNodesUp);
+        }
+
         final int nodesCount = clusterInfo.getStorageNodeInfo().size();
         final int upNodesCount = contentNodesWithAvailableNodeState(clusterState);
 
         if (nodesCount == 0) {
-            return Result.createDisallowed("No storage nodes in cluster state, not safe to restart.");
+            return Result.createDisallowed("No storage nodes in cluster state");
         }
         if (((double)upNodesCount) / nodesCount < minRatioOfStorageNodesUp) {
-            return Result.createDisallowed("Not enough storage nodes running, running: " + upNodesCount
-                    + " total storage nodes " +  nodesCount +
-                    " required fraction " + minRatioOfStorageNodesUp);
+            return Result.createDisallowed("Not enough storage nodes running: " + upNodesCount
+                    + " of " +  nodesCount + " storage nodes are up which is less that the required fraction of "
+                    + minRatioOfStorageNodesUp);
         }
         return Result.allowSettingOfWantedState();
     }
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java
index b2924516f26..4770bb40545 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/hostinfo/Metrics.java
@@ -6,6 +6,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * Keeper for Metrics for HostInfo.
@@ -13,7 +14,17 @@ import java.util.List;
  */
 public class Metrics {
 
-    public List<Metric> getValues() { return Collections.unmodifiableList(metricsList); }
+    public Optional<Value> getValue(String name) {
+        for (Metric metric : metricsList) {
+            if (name.equals(metric.getName())) {
+                return Optional.ofNullable(metric.getValue());
+            }
+        }
+
+        return Optional.empty();
+    }
+
+    public List<Metric> getMetrics() { return Collections.unmodifiableList(metricsList); }
 
     public static class Metric {
         private final String name;
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java
index 8958bce8ccd..308b189841d 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/PartitionStateRequest.java
@@ -39,7 +39,7 @@ public class PartitionStateRequest extends Request<Response.PartitionResponse> {
     }
 
     private static void fillInMetrics(Metrics metrics, Response.PartitionResponse result) {
-        for (Metrics.Metric metric: metrics.getValues()) {
+        for (Metrics.Metric metric: metrics.getMetrics()) {
             fillInMetricValue(metric.getName(), metric.getValue(), result);
         }
     }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java
index 5b53e524102..0d2f12dfdcd 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeCheckerTest.java
@@ -20,9 +20,13 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
-import static org.hamcrest.core.StringContains.containsString;
 import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.*;
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -36,8 +40,9 @@ public class NodeStateChangeCheckerTest {
     private static final Node nodeDistributor = new Node(NodeType.DISTRIBUTOR, 1);
     private static final Node nodeStorage = new Node(NodeType.STORAGE, 1);
 
-    private static final NodeState upNodeState = new NodeState(NodeType.STORAGE, State.UP);
-    public static final NodeState maintenanceNodeState = createNodeState(State.MAINTENANCE, "Orchestrator");
+    private static final NodeState UP_NODE_STATE = new NodeState(NodeType.STORAGE, State.UP);
+    public static final NodeState MAINTENANCE_NODE_STATE = createNodeState(State.MAINTENANCE, "Orchestrator");
+    public static final NodeState DOWN_NODE_STATE = createNodeState(State.DOWN, "RetireEarlyExpirer");
 
     private static NodeState createNodeState(State state, String description) {
         return new NodeState(NodeType.STORAGE, state).setDescription(description);
@@ -122,17 +127,30 @@ public class NodeStateChangeCheckerTest {
         NodeState newState = new NodeState(NodeType.STORAGE, State.INITIALIZING);
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeDistributor, defaultAllUpClusterState(), SetUnitStateRequest.Condition.FORCE,
-                upNodeState, newState);
+                UP_NODE_STATE, newState);
         assertTrue(result.settingWantedStateIsAllowed());
         assertTrue(!result.wantedStateAlreadySet());
     }
 
     @Test
+    public void testUnknownStorageNode() {
+        ContentCluster cluster = createCluster(createNodes(4));
+        NodeStateChangeChecker nodeStateChangeChecker = new NodeStateChangeChecker(
+                5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo());
+        NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
+                new Node(NodeType.STORAGE, 10), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
+                UP_NODE_STATE, MAINTENANCE_NODE_STATE);
+        assertFalse(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+        assertThat(result.getReason(), is("Unknown node storage.10"));
+    }
+
+    @Test
     public void testSafeSetStateDistributors() {
         NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(createCluster(createNodes(1)));
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeDistributor, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
-                upNodeState, maintenanceNodeState);
+                UP_NODE_STATE, MAINTENANCE_NODE_STATE);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
         assertThat(result.getReason(), containsString("Safe-set of node state is only supported for storage nodes"));
@@ -146,7 +164,7 @@ public class NodeStateChangeCheckerTest {
                 5 /* min storage nodes */, minRatioOfStorageNodesUp, requiredRedundancy, cluster.clusterInfo());
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
-                upNodeState, maintenanceNodeState);
+                UP_NODE_STATE, MAINTENANCE_NODE_STATE);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
         assertThat(result.getReason(), is("There are only 4 storage nodes up, while config requires at least 5"));
@@ -167,7 +185,7 @@ public class NodeStateChangeCheckerTest {
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
-                maintenanceNodeState, upNodeState);
+                MAINTENANCE_NODE_STATE, UP_NODE_STATE);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
@@ -187,7 +205,7 @@ public class NodeStateChangeCheckerTest {
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, stateWithNodeDown, SetUnitStateRequest.Condition.SAFE,
-                maintenanceNodeState, upNodeState);
+                MAINTENANCE_NODE_STATE, UP_NODE_STATE);
         assertTrue(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
@@ -200,7 +218,7 @@ public class NodeStateChangeCheckerTest {
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
-                new NodeState(NodeType.STORAGE, State.DOWN), upNodeState);
+                new NodeState(NodeType.STORAGE, State.DOWN), UP_NODE_STATE);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
         assertThat(result.getReason(), is("Refusing to set wanted state to up when it is currently in Down"));
@@ -214,7 +232,7 @@ public class NodeStateChangeCheckerTest {
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
-                upNodeState, maintenanceNodeState);
+                UP_NODE_STATE, MAINTENANCE_NODE_STATE);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
         assertThat(result.getReason(), is("Distributor 0 says storage node 1 " +
@@ -229,7 +247,7 @@ public class NodeStateChangeCheckerTest {
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 new Node(NodeType.STORAGE, 3), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
-                upNodeState, maintenanceNodeState);
+                UP_NODE_STATE, MAINTENANCE_NODE_STATE);
         assertTrue(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
@@ -253,7 +271,7 @@ public class NodeStateChangeCheckerTest {
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 new Node(NodeType.STORAGE, 1), defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE,
-                upNodeState, maintenanceNodeState);
+                UP_NODE_STATE, MAINTENANCE_NODE_STATE);
         assertTrue(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
@@ -265,7 +283,7 @@ public class NodeStateChangeCheckerTest {
         cluster.clusterInfo().getStorageNodeInfo(1).setReportedState(new NodeState(NodeType.STORAGE, State.UP), 0);
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
-                nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, upNodeState, maintenanceNodeState);
+                nodeStorage, defaultAllUpClusterState(), SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
         assertThat(result.getReason(), is("Distributor node (0) has not reported any cluster state version yet."));
@@ -332,7 +350,7 @@ public class NodeStateChangeCheckerTest {
         }
 
         return nodeStateChangeChecker.evaluateTransition(
-                nodeStorage, clusterState, SetUnitStateRequest.Condition.SAFE, upNodeState, maintenanceNodeState);
+                nodeStorage, clusterState, SetUnitStateRequest.Condition.SAFE, UP_NODE_STATE, MAINTENANCE_NODE_STATE);
     }
 
     private void setAllNodesUp(ContentCluster cluster, HostInfo distributorHostInfo) {
@@ -402,11 +420,144 @@ public class NodeStateChangeCheckerTest {
 
         NodeStateChangeChecker.Result result = nodeStateChangeChecker.evaluateTransition(
                 nodeStorage, stateWithNodeDown, SetUnitStateRequest.Condition.SAFE,
-                upNodeState, maintenanceNodeState);
+                UP_NODE_STATE, MAINTENANCE_NODE_STATE);
         assertFalse(result.settingWantedStateIsAllowed());
         assertFalse(result.wantedStateAlreadySet());
     }
 
+    @Test
+    public void testDownDisallowedByNonRetiredState() {
+        NodeStateChangeChecker.Result result = evaluateDownTransition(
+                defaultAllUpClusterState(),
+                State.UP,
+                currentClusterStateVersion,
+                0);
+        assertFalse(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+        assertEquals("Only retired nodes are allowed to be set to DOWN in safe mode - is Up", result.getReason());
+    }
+
+    @Test
+    public void testDownDisallowedByBuckets() {
+        NodeStateChangeChecker.Result result = evaluateDownTransition(
+                retiredClusterStateSuffix(),
+                State.UP,
+                currentClusterStateVersion,
+                1);
+        assertFalse(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+        assertEquals("The storage node manages 1 buckets", result.getReason());
+    }
+
+    @Test
+    public void testDownDisallowedByReportedState() {
+        NodeStateChangeChecker.Result result = evaluateDownTransition(
+                retiredClusterStateSuffix(),
+                State.INITIALIZING,
+                currentClusterStateVersion,
+                0);
+        assertFalse(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+        assertEquals("Reported state (Initializing) is not UP, so no bucket data is available", result.getReason());
+    }
+
+    @Test
+    public void testDownDisallowedByVersionMismatch() {
+        NodeStateChangeChecker.Result result = evaluateDownTransition(
+                retiredClusterStateSuffix(),
+                State.UP,
+                currentClusterStateVersion - 1,
+                0);
+        assertFalse(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+        assertEquals("Cluster controller at version 2 got info for storage node 1 at a different version 1",
+                result.getReason());
+    }
+
+    @Test
+    public void testAllowedToSetDown() {
+        NodeStateChangeChecker.Result result = evaluateDownTransition(
+                retiredClusterStateSuffix(),
+                State.UP,
+                currentClusterStateVersion,
+                0);
+        assertTrue(result.settingWantedStateIsAllowed());
+        assertFalse(result.wantedStateAlreadySet());
+    }
+
+    private NodeStateChangeChecker.Result evaluateDownTransition(
+            ClusterState clusterState,
+            State reportedState,
+            int hostInfoClusterStateVersion,
+            int lastAlldisksBuckets) {
+        ContentCluster cluster = createCluster(createNodes(4));
+        NodeStateChangeChecker nodeStateChangeChecker = createChangeChecker(cluster);
+
+        StorageNodeInfo nodeInfo = cluster.clusterInfo().getStorageNodeInfo(nodeStorage.getIndex());
+        nodeInfo.setReportedState(new NodeState(NodeType.STORAGE, reportedState), 0);
+        nodeInfo.setHostInfo(createHostInfoWithMetrics(hostInfoClusterStateVersion, lastAlldisksBuckets));
+
+        return nodeStateChangeChecker.evaluateTransition(
+                nodeStorage, clusterState, SetUnitStateRequest.Condition.SAFE,
+                UP_NODE_STATE, DOWN_NODE_STATE);
+    }
+
+    private ClusterState retiredClusterStateSuffix() {
+        return clusterState(String.format("version:%d distributor:4 storage:4 .%d.s:r",
+                currentClusterStateVersion,
+                nodeStorage.getIndex()));
+    }
+
+    private static HostInfo createHostInfoWithMetrics(int clusterStateVersion, int lastAlldisksBuckets) {
+        return HostInfo.createHostInfo(String.format("{\n" +
+                        "  \"metrics\":\n" +
+                        "  {\n" +
+                        "    \"snapshot\":\n" +
+                        "    {\n" +
+                        "      \"from\":1494940706,\n" +
+                        "      \"to\":1494940766\n" +
+                        "    },\n" +
+                        "    \"values\":\n" +
+                        "    [\n" +
+                        "      {\n" +
+                        "        \"name\":\"vds.datastored.alldisks.buckets\",\n" +
+                        "        \"description\":\"buckets managed\",\n" +
+                        "        \"values\":\n" +
+                        "        {\n" +
+                        "          \"average\":262144.0,\n" +
+                        "          \"count\":1,\n" +
+                        "          \"rate\":0.016666,\n" +
+                        "          \"min\":262144,\n" +
+                        "          \"max\":262144,\n" +
+                        "          \"last\":%d\n" +
+                        "        },\n" +
+                        "        \"dimensions\":\n" +
+                        "        {\n" +
+                        "        }\n" +
+                        "      },\n" +
+                        "      {\n" +
+                        "        \"name\":\"vds.datastored.alldisks.docs\",\n" +
+                        "        \"description\":\"documents stored\",\n" +
+                        "        \"values\":\n" +
+                        "        {\n" +
+                        "          \"average\":154689587.0,\n" +
+                        "          \"count\":1,\n" +
+                        "          \"rate\":0.016666,\n" +
+                        "          \"min\":154689587,\n" +
+                        "          \"max\":154689587,\n" +
+                        "          \"last\":154689587\n" +
+                        "        },\n" +
+                        "        \"dimensions\":\n" +
+                        "        {\n" +
+                        "        }\n" +
+                        "      }\n" +
+                        "    ]\n" +
+                        "  },\n" +
+                        "  \"cluster-state-version\":%d\n" +
+                        "}",
+                lastAlldisksBuckets, clusterStateVersion));
+    }
+
     private List<ConfiguredNode> createNodes(int count) {
         List<ConfiguredNode> nodes = new ArrayList<>();
         for (int i = 0; i < count; i++)
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java
index d24b45817e0..fd93df4b2dc 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/hostinfo/HostInfoTest.java
@@ -32,7 +32,7 @@ public class HostInfoTest {
         HostInfo hostInfo = HostInfo.createHostInfo("{}");
         assertThat(hostInfo.getVtag().getVersionOrNull(), is(nullValue()));
         assertThat(hostInfo.getDistributor().getStorageNodes().size(), is(0));
-        assertThat(hostInfo.getMetrics().getValues().size(), is(0));
+        assertThat(hostInfo.getMetrics().getMetrics().size(), is(0));
         assertThat(hostInfo.getClusterStateVersionOrNull(), is(nullValue()));
     }
 
@@ -51,7 +51,7 @@ public class HostInfoTest {
         assertThat(storageNodeList.get(0).getOpsLatenciesOrNull().getPut().getCount(), is(16L));
         assertThat(storageNodeList.get(1).getOpsLatenciesOrNull().getPut().getCount(), is(18L));
         assertThat(storageNodeList.get(0).getOpsLatenciesOrNull().getPut().getLatencyMsSum(), is(15L));
-        List<Metrics.Metric> metrics = hostInfo.getMetrics().getValues();
+        List<Metrics.Metric> metrics = hostInfo.getMetrics().getMetrics();
         assertThat(metrics.size(), is(2));
         Metrics.Value value = metrics.get(0).getValue();
         assertThat(value.getLast(), is(5095L));
@@ -70,7 +70,7 @@ public class HostInfoTest {
             }
             HostInfo hostInfo = HostInfo.createHostInfo(json);
             // Check a value so not all code is removed by optimizer.
-            if (hostInfo.getMetrics().getValues().size() == -1) return;
+            if (hostInfo.getMetrics().getMetrics().size() == -1) return;
         }
         long end = System.currentTimeMillis();
         System.out.println("Should take about 1.5 ms on fast machine, actually " + (end - start) / 10. + " ms.");