diff options
5 files changed, 151 insertions, 48 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..659edc96b77 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,31 @@ 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<NodeRepositoryNode, 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(node -> + allNodes.stream() + .filter(parent -> parent.getHostname().equals(node.getParentHostname())) + .findFirst().orElseThrow() + )); // 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 +52,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 +61,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 +80,23 @@ public class ChangeManagementAssessor { return assessment; }).collect(Collectors.toList()); + + var hostAssessments = prParentHost.entrySet().stream().map((entry) -> { + HostAssessment hostAssessment = new HostAssessment(); + hostAssessment.hostName = entry.getKey().getHostname(); + hostAssessment.switchName = entry.getKey().getSwitchHostname(); + 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 +113,43 @@ 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 String switchName; + 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..cd9c467582c 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,14 @@ 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.setString("switchName", assessment.switchName); + 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..8a91be2b2a3 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 @@ -8,7 +8,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeOwne import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; import com.yahoo.vespa.hosted.controller.integration.NodeRepositoryMock; -import org.junit.Assert; import org.junit.Test; import java.util.ArrayList; @@ -16,6 +15,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + 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 @@ -44,19 +46,23 @@ public class ChangeManagementAssessorTest { // Add an not impacted hosts allNodesInZone.add(createNode("node4", "host2", "myapp", "default", 0 )); + // Add tenant hosts + allNodesInZone.add(createHost("host1", "switch1")); + allNodesInZone.add(createHost("host2", "switch1")); + // 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 @@ -82,19 +88,33 @@ public class ChangeManagementAssessorTest { allNodesInZone.add(createNode("node5", "host4", "myapp", "myman", 5 )); allNodesInZone.add(createNode("node6", "host4", "myapp", "myman", 6 )); + // Add tenant hosts + allNodesInZone.add(createHost("host1", "switch1")); + allNodesInZone.add(createHost("host2", "switch1")); + // 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()); + assertTrue(hostAssessments.stream().anyMatch(hostAssessment -> + hostAssessment.hostName.equals("host1") && + hostAssessment.switchName.equals("switch1") && + hostAssessment.numberOfChildren == 2 && + hostAssessment.numberOfProblematicChildren == 2 + )); } private NodeOwner createOwner(String tenant, String application, String instance) { @@ -125,4 +145,11 @@ public class ChangeManagementAssessorTest { return node; } + + private NodeRepositoryNode createHost(String hostname, String switchName) { + NodeRepositoryNode node = new NodeRepositoryNode(); + node.setHostname(hostname); + node.setSwitchHostname(switchName); + return node; + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java index 6f5aa776fef..359f34f5740 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/changemanagement/ChangeManagementApiHandlerTest.java @@ -54,6 +54,8 @@ public class ChangeManagementApiHandlerTest extends ControllerContainerTest { nodes.add(createNode("node2", "host1", "myapp", "default", 0 )); nodes.add(createNode("node3", "host1", "myapp", "default", 0 )); nodes.add(createNode("node4", "host2", "myapp", "default", 0 )); + nodes.add(createHost("host1", "switch1")); + nodes.add(createHost("host2", "switch2")); return nodes; } @@ -85,4 +87,12 @@ public class ChangeManagementApiHandlerTest extends ControllerContainerTest { return node; } + + private NodeRepositoryNode createHost(String hostname, String switchName) { + NodeRepositoryNode node = new NodeRepositoryNode(); + node.setHostname(hostname); + node.setSwitchHostname(switchName); + return node; + } + } 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..cfe98ab906b 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,13 @@ "impact": "na" } ], - "hosts": [] + "hosts": [ + { + "hostname": "host1", + "switchName": "switch1", + "numberOfChildren": 3, + "numberOfProblematicChildren": 3 + } + ] } }
\ No newline at end of file |