From 7df5396bef81b7cda3c1428182a89072cc2ae70d Mon Sep 17 00:00:00 2001 From: Ola Aunrønning Date: Thu, 18 Mar 2021 16:01:49 +0100 Subject: Consider whether all hosts in change request is replaceable --- .../maintenance/ChangeManagementAssessor.java | 17 +++++++++++++---- .../changemanagement/ChangeManagementApiHandler.java | 2 +- .../controller/integration/NodeRepositoryMock.java | 5 +++++ .../maintenance/ChangeManagementAssessorTest.java | 9 ++++++--- 4 files changed, 25 insertions(+), 8 deletions(-) (limited to 'controller-server') 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 5226f8b39bd..1bfdbc55f26 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 @@ -1,5 +1,6 @@ package com.yahoo.vespa.hosted.controller.maintenance; +import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository; @@ -28,15 +29,15 @@ public class ChangeManagementAssessor { public String impact; } - public ChangeManagementAssessor(Controller controller) { - this.nodeRepository = controller.serviceRegistry().configServer().nodeRepository(); + public ChangeManagementAssessor(NodeRepository nodeRepository) { + this.nodeRepository = nodeRepository; } public List assessment(List impactedHostnames, ZoneId zone) { return assessmentInner(impactedHostnames, nodeRepository.listNodes(zone).nodes(), zone); } - static List assessmentInner(List impactedHostnames, List allNodes, ZoneId zone) { + List assessmentInner(List impactedHostnames, List allNodes, ZoneId zone) { // Get all active nodes running on the impacted hosts List containerNodes = allNodes.stream() .filter(nodeRepositoryNode -> nodeRepositoryNode.getState() == NodeState.active) //TODO look at more states? @@ -45,6 +46,13 @@ public class ChangeManagementAssessor { // Group nodes pr cluster Map> prCluster = containerNodes.stream().collect(Collectors.groupingBy(ChangeManagementAssessor::clusterKey)); + boolean allHostsReplacable = nodeRepository.isReplaceable( + zone, + impactedHostnames.stream() + .map(HostName::from) + .collect(Collectors.toList()) + ); + // Report assessment pr cluster return prCluster.entrySet().stream().map((entry) -> { String key = entry.getKey(); @@ -64,10 +72,11 @@ public class ChangeManagementAssessor { assessment.groupsTotal = totalStats[1]; assessment.groupsImpact = impactedStats[1]; + // TODO check upgrade policy assessment.upgradePolicy = "na"; // TODO do some heuristic on suggestion action - assessment.suggestedAction = "nothing"; + assessment.suggestedAction = allHostsReplacable ? "Retire all hosts" : "nothing"; // TODO do some heuristic on impact assessment.impact = "na"; 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 ad12b7d6fa9..09114a03522 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 @@ -36,7 +36,7 @@ public class ChangeManagementApiHandler extends AuditLoggingRequestHandler { public ChangeManagementApiHandler(LoggingRequestHandler.Context ctx, Controller controller) { super(ctx, controller.auditLogger()); - this.assessor = new ChangeManagementAssessor(controller); + this.assessor = new ChangeManagementAssessor(controller.serviceRegistry().configServer().nodeRepository()); this.controller = controller; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java index a84e7242495..85e82bb3fcd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java @@ -284,6 +284,11 @@ public class NodeRepositoryMock implements NodeRepository { throw new UnsupportedOperationException(); } + @Override + public boolean isReplaceable(ZoneId zoneId, List hostNames) { + return false; + } + public Optional osUpgradeBudget(ZoneId zone, NodeType type, Version version) { return Optional.ofNullable(osUpgradeBudgets.get(Objects.hash(zone, type, version))); } 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 f3cb996c048..5ec14ee6156 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 @@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeMemb import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeOwner; 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; @@ -16,6 +17,8 @@ import java.util.List; public class ChangeManagementAssessorTest { + private ChangeManagementAssessor changeManagementAssessor = new ChangeManagementAssessor(new NodeRepositoryMock()); + @Test public void empty_input_variations() { ZoneId zone = ZoneId.from("prod", "eu-trd"); @@ -24,7 +27,7 @@ public class ChangeManagementAssessorTest { // Both zone and hostnames are empty List assessments - = ChangeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); + = changeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); Assert.assertEquals(0, assessments.size()); } @@ -42,7 +45,7 @@ public class ChangeManagementAssessorTest { // Make Assessment List assessments - = ChangeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); + = changeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); // Assess the assessment :-o Assert.assertEquals(1, assessments.size()); @@ -80,7 +83,7 @@ public class ChangeManagementAssessorTest { // Make Assessment List assessments - = ChangeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); + = changeManagementAssessor.assessmentInner(hostNames, allNodesInZone, zone); // Assess the assessment :-o Assert.assertEquals(1, assessments.size()); //One cluster is impacted -- cgit v1.2.3