Assessor includes host assessments

4 files changed, 114 insertions, 47 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessor.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessor.java
index 00e8c508695..104e8be4566 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessor.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessor.java
@@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepo
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
@@ -17,35 +18,27 @@ public class ChangeManagementAssessor {
 
     private final NodeRepository nodeRepository;
 
-    public static class Assessment {
-        public String app;
-        public String zone;
-        public String cluster;
-        public long clusterImpact;
-        public long clusterSize;
-        public long groupsImpact;
-        public long groupsTotal;
-        public String upgradePolicy;
-        public String suggestedAction;
-        public String impact;
-    }
-
     public ChangeManagementAssessor(NodeRepository nodeRepository) {
         this.nodeRepository = nodeRepository;
     }
 
-    public List<Assessment> assessment(List<String> impactedHostnames, ZoneId zone) {
+    public Assessment assessment(List<String> impactedHostnames, ZoneId zone) {
         return assessmentInner(impactedHostnames, nodeRepository.listNodes(zone).nodes(), zone);
     }
 
-    List<Assessment> assessmentInner(List<String> impactedHostnames, List<NodeRepositoryNode> allNodes, ZoneId zone) {
-        // Get all active nodes running on the impacted hosts
-        List<NodeRepositoryNode> containerNodes = allNodes.stream()
+    Assessment assessmentInner(List<String> impactedHostnames, List<NodeRepositoryNode> allNodes, ZoneId zone) {
+
+        // Group impacted application nodes by parent host
+        Map<String, List<NodeRepositoryNode>> prParentHost = allNodes.stream()
                 .filter(nodeRepositoryNode -> nodeRepositoryNode.getState() == NodeState.active) //TODO look at more states?
-                .filter(node -> impactedHostnames.contains(node.getParentHostname() == null ? "" : node.getParentHostname())).collect(Collectors.toList());
+                .filter(node -> impactedHostnames.contains(node.getParentHostname() == null ? "" : node.getParentHostname()))
+                .collect(Collectors.groupingBy(NodeRepositoryNode::getParentHostname));
 
         // Group nodes pr cluster
-        Map<String, List<NodeRepositoryNode>> prCluster = containerNodes.stream().collect(Collectors.groupingBy(ChangeManagementAssessor::clusterKey));
+        Map<String, List<NodeRepositoryNode>> prCluster = prParentHost.values()
+                .stream()
+                .flatMap(Collection::stream)
+                .collect(Collectors.groupingBy(ChangeManagementAssessor::clusterKey));
 
         boolean allHostsReplacable = nodeRepository.isReplaceable(
                 zone,
@@ -55,7 +48,7 @@ public class ChangeManagementAssessor {
         );
 
         // Report assessment pr cluster
-        return prCluster.entrySet().stream().map((entry) -> {
+        var clusterAssessments = prCluster.entrySet().stream().map((entry) -> {
             String key = entry.getKey();
             List<NodeRepositoryNode> nodes = entry.getValue();
             String app = Arrays.stream(key.split(":")).limit(3).collect(Collectors.joining(":"));
@@ -64,7 +57,7 @@ public class ChangeManagementAssessor {
             long[] totalStats = clusterStats(key, allNodes);
             long[] impactedStats = clusterStats(key, nodes);
 
-            Assessment assessment = new Assessment();
+            ClusterAssessment assessment = new ClusterAssessment();
             assessment.app = app;
             assessment.zone = zone.value();
             assessment.cluster = cluster;
@@ -83,6 +76,22 @@ public class ChangeManagementAssessor {
 
             return assessment;
         }).collect(Collectors.toList());
+
+        var hostAssessments = prParentHost.entrySet().stream().map((entry) -> {
+            HostAssessment hostAssessment = new HostAssessment();
+            hostAssessment.hostName = entry.getKey();
+            hostAssessment.numberOfChildren = entry.getValue().size();
+
+            //TODO: Some better heuristic for what's considered problematic
+            hostAssessment.numberOfProblematicChildren = (int) entry.getValue().stream()
+                    .mapToInt(node -> prCluster.get(clusterKey(node)).size())
+                    .filter(i -> i > 1)
+                    .count();
+
+            return hostAssessment;
+        }).collect(Collectors.toList());
+
+        return new Assessment(clusterAssessments, hostAssessments);
     }
 
     private static String clusterKey(NodeRepositoryNode node) {
@@ -99,4 +108,42 @@ public class ChangeManagementAssessor {
         long groups = clusterNodes.stream().map(nodeRepositoryNode -> nodeRepositoryNode.getMembership() != null ? nodeRepositoryNode.getMembership().group : "").distinct().count();
         return new long[] { clusterNodes.size(), groups};
     }
+
+    public static class Assessment {
+        List<ClusterAssessment> clusterAssessments;
+        List<HostAssessment> hostAssessments;
+
+        Assessment(List<ClusterAssessment> clusterAssessments, List<HostAssessment> hostAssessments) {
+            this.clusterAssessments = clusterAssessments;
+            this.hostAssessments = hostAssessments;
+        }
+
+        public List<ClusterAssessment> getClusterAssessments() {
+            return clusterAssessments;
+        }
+
+        public List<HostAssessment> getHostAssessments() {
+            return hostAssessments;
+        }
+    }
+
+    public static class ClusterAssessment {
+        public String app;
+        public String zone;
+        public String cluster;
+        public long clusterImpact;
+        public long clusterSize;
+        public long groupsImpact;
+        public long groupsTotal;
+        public String upgradePolicy;
+        public String suggestedAction;
+        public String impact;
+    }
+
+    public static class HostAssessment {
+        public String hostName;
+        public int numberOfChildren;
+        public int numberOfProblematicChildren;
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java
index b522469f179..6f37c2fbf35 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandler.java
@@ -134,7 +134,7 @@ public class ChangeManagementApiHandler extends AuditLoggingRequestHandler {
     }
 
     private Slime doAssessment(List<String> hostNames, ZoneId zoneId) {
-        List<ChangeManagementAssessor.Assessment> assessments = assessor.assessment(hostNames, zoneId);
+        ChangeManagementAssessor.Assessment assessments = assessor.assessment(hostNames, zoneId);
 
         Slime slime = new Slime();
         Cursor root = slime.setObject();
@@ -148,7 +148,7 @@ public class ChangeManagementApiHandler extends AuditLoggingRequestHandler {
         // Assessment on the cluster level
         Cursor clustersCursor = assessmentCursor.setArray("clusters");
 
-        assessments.forEach(assessment -> {
+        assessments.getClusterAssessments().forEach(assessment -> {
             Cursor oneCluster = clustersCursor.addObject();
             oneCluster.setString("app", assessment.app);
             oneCluster.setString("zone", assessment.zone);
@@ -162,8 +162,13 @@ public class ChangeManagementApiHandler extends AuditLoggingRequestHandler {
             oneCluster.setString("impact", assessment.impact);
         });
 
-        // Assessment on the host level - TODO
-        assessmentCursor.setArray("hosts");
+        Cursor hostsCursor = assessmentCursor.setArray("hosts");
+        assessments.getHostAssessments().forEach(assessment -> {
+            Cursor hostObject = hostsCursor.addObject();
+            hostObject.setString("hostname", assessment.hostName);
+            hostObject.setLong("numberOfChildren", assessment.numberOfChildren);
+            hostObject.setLong("numberOfProblematicChildren", assessment.numberOfProblematicChildren);
+        });
 
         return slime;
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessorTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessorTest.java
index a4fb4669237..80c5b9fbbaa 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessorTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ChangeManagementAssessorTest.java
@@ -16,6 +16,8 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import static org.junit.Assert.assertEquals;
+
 public class ChangeManagementAssessorTest {
 
     private ChangeManagementAssessor changeManagementAssessor = new ChangeManagementAssessor(new NodeRepositoryMock());
@@ -27,9 +29,9 @@ public class ChangeManagementAssessorTest {
         List<NodeRepositoryNode> allNodesInZone = new ArrayList<>();
 
         // Both zone and hostnames are empty
-        List<ChangeManagementAssessor.Assessment> assessments
+        ChangeManagementAssessor.Assessment assessment
                 = changeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone);
-        Assert.assertEquals(0, assessments.size());
+        assertEquals(0, assessment.getClusterAssessments().size());
     }
 
     @Test
@@ -45,18 +47,18 @@ public class ChangeManagementAssessorTest {
         allNodesInZone.add(createNode("node4", "host2", "myapp", "default", 0 ));
 
         // Make Assessment
-        List<ChangeManagementAssessor.Assessment> assessments
-                = changeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone);
+        List<ChangeManagementAssessor.ClusterAssessment> assessments
+                = changeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone).getClusterAssessments();
 
         // Assess the assessment :-o
-        Assert.assertEquals(1, assessments.size());
-        Assert.assertEquals(3, assessments.get(0).clusterImpact);
-        Assert.assertEquals(4, assessments.get(0).clusterSize);
-        Assert.assertEquals(1, assessments.get(0).groupsImpact);
-        Assert.assertEquals(1, assessments.get(0).groupsTotal);
-        Assert.assertEquals("content:default", assessments.get(0).cluster);
-        Assert.assertEquals("mytenant:myapp:default", assessments.get(0).app);
-        Assert.assertEquals("prod.eu-trd", assessments.get(0).zone);
+        assertEquals(1, assessments.size());
+        assertEquals(3, assessments.get(0).clusterImpact);
+        assertEquals(4, assessments.get(0).clusterSize);
+        assertEquals(1, assessments.get(0).groupsImpact);
+        assertEquals(1, assessments.get(0).groupsTotal);
+        assertEquals("content:default", assessments.get(0).cluster);
+        assertEquals("mytenant:myapp:default", assessments.get(0).app);
+        assertEquals("prod.eu-trd", assessments.get(0).zone);
     }
 
     @Test
@@ -83,18 +85,25 @@ public class ChangeManagementAssessorTest {
         allNodesInZone.add(createNode("node6", "host4", "myapp", "myman", 6 ));
 
         // Make Assessment
-        List<ChangeManagementAssessor.Assessment> assessments
+        ChangeManagementAssessor.Assessment assessment
                 = changeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone);
 
         // Assess the assessment :-o
-        Assert.assertEquals(1, assessments.size()); //One cluster is impacted
-        Assert.assertEquals(3, assessments.get(0).clusterImpact);
-        Assert.assertEquals(6, assessments.get(0).clusterSize);
-        Assert.assertEquals(1, assessments.get(0).groupsImpact);
-        Assert.assertEquals(2, assessments.get(0).groupsTotal);
-        Assert.assertEquals("content:default", assessments.get(0).cluster);
-        Assert.assertEquals("mytenant:myapp:default", assessments.get(0).app);
-        Assert.assertEquals("prod.eu-trd", assessments.get(0).zone);
+        List<ChangeManagementAssessor.ClusterAssessment> clusterAssessments = assessment.getClusterAssessments();
+        assertEquals(1, clusterAssessments.size()); //One cluster is impacted
+        assertEquals(3, clusterAssessments.get(0).clusterImpact);
+        assertEquals(6, clusterAssessments.get(0).clusterSize);
+        assertEquals(1, clusterAssessments.get(0).groupsImpact);
+        assertEquals(2, clusterAssessments.get(0).groupsTotal);
+        assertEquals("content:default", clusterAssessments.get(0).cluster);
+        assertEquals("mytenant:myapp:default", clusterAssessments.get(0).app);
+        assertEquals("prod.eu-trd", clusterAssessments.get(0).zone);
+
+        List<ChangeManagementAssessor.HostAssessment> hostAssessments = assessment.getHostAssessments();
+        assertEquals(2, hostAssessments.size());
+        assertEquals("host1", hostAssessments.get(0).hostName);
+        assertEquals(2, hostAssessments.get(0).numberOfChildren);
+        assertEquals(2, hostAssessments.get(0).numberOfProblematicChildren);
     }
 
     private NodeOwner createOwner(String tenant, String application, String instance) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json
index a9b0dacb968..e8bdd1a62a8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/responses/initial.json
@@ -15,6 +15,12 @@
         "impact": "na"
       }
     ],
-    "hosts": []
+    "hosts": [
+      {
+        "hostname": "host1",
+        "numberOfChildren": 3,
+        "numberOfProblematicChildren": 3
+      }
+    ]
   }
 }
\ No newline at end of file