Merge pull request #17360 from vespa-engine/mpolden/query-deprovisioned

Add includeDeprovisioned parameter to node repository client

11 files changed, 18 insertions, 45 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java
index 72592e16bfd..6aaf568d84b 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java
@@ -36,36 +36,21 @@ public interface NodeRepository {
 
     void deleteNode(ZoneId zone, String hostname);
 
-    void setState(ZoneId zone, NodeState nodeState, String nodename);
+    void setState(ZoneId zone, NodeState nodeState, String hostname);
 
     NodeRepositoryNode getNode(ZoneId zone, String hostname);
 
+    // TODO: Migrate any callers to list() and remove this method
     NodeList listNodes(ZoneId zone);
 
-    NodeList listNodes(ZoneId zone, ApplicationId application);
-
-    NodeList listNodes(ZoneId zone, List<HostName> hostnames);
-
     /** List all nodes in given zone */
-    default List<Node> list(ZoneId zone) {
-        return listNodes(zone).nodes().stream()
-                              .map(NodeRepository::toNode)
-                              .collect(Collectors.toUnmodifiableList());
-    }
+    List<Node> list(ZoneId zone, boolean includeDeprovisioned);
 
     /** List all nodes in zone having given hostnames */
-    default List<Node> list(ZoneId zone, List<HostName> hostnames) {
-        return listNodes(zone, hostnames).nodes().stream()
-                                         .map(NodeRepository::toNode)
-                                         .collect(Collectors.toUnmodifiableList());
-    }
+    List<Node> list(ZoneId zone, List<HostName> hostnames);
 
     /** List all nodes in zone owned by given application */
-    default List<Node> list(ZoneId zone, ApplicationId application) {
-        return listNodes(zone, application).nodes().stream()
-                                           .map(NodeRepository::toNode)
-                                           .collect(Collectors.toUnmodifiableList());
-    }
+    List<Node> list(ZoneId zone, ApplicationId application);
 
     /** List all nodes in states, in zone owned by given application */
     default List<Node> list(ZoneId zone, ApplicationId application, Set<Node.State> states) {
@@ -111,7 +96,7 @@ public interface NodeRepository {
     /** Checks whether the zone has the spare capacity to remove the given hosts */
     boolean isReplaceable(ZoneId zoneId, List<HostName> hostNames);
 
-    private static Node toNode(NodeRepositoryNode node) {
+    static Node toNode(NodeRepositoryNode node) {
         var application = Optional.ofNullable(node.getOwner())
                                   .map(owner -> ApplicationId.from(owner.getTenant(), owner.getApplication(),
                                                                    owner.getInstance()));
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java
index 26243a28ee0..5714e3d5f6d 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/ProvisionResource.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.hosted.controller.api.integration.noderepository;
 
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepoStats;
 
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
@@ -44,7 +43,7 @@ public interface ProvisionResource {
 
     @GET
     @Path("/node/")
-    NodeList listNodes(@QueryParam("recursive") boolean recursive);
+    NodeList listNodes(@QueryParam("recursive") boolean recursive, @QueryParam("includeDeprovisioned") boolean includeDeprovisioned);
 
     @GET
     @Path("/node/")
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
index 5ab2ca4a5d6..d923db936cb 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
@@ -54,7 +54,7 @@ public class CloudEventReporter extends ControllerMaintainer {
     /** Deprovision any host affected by given event */
     private void deprovisionAffectedHosts(String region, CloudEvent event) {
         for (var zone : zonesByCloudNativeRegion.get(region)) {
-            for (var node : nodeRepository.list(zone.getId())) {
+            for (var node : nodeRepository.list(zone.getId(), false)) {
                 if (!affects(node, event)) continue;
                 log.info("Retiring and deprovisioning " + node.hostname().value() + " in " + zone.getId() +
                          ": Affected by maintenance event " + event.instanceEventId);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdater.java
index 8e7a364b5f3..87326bdac42 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdater.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdater.java
@@ -43,7 +43,7 @@ public class HostSwitchUpdater extends ControllerMaintainer {
         int nodesUpdated = 0;
         try {
             for (var zone : controller().zoneRegistry().zones().controllerUpgraded().all().ids()) {
-                for (var node : nodeRepository.list(zone)) {
+                for (var node : nodeRepository.list(zone, false)) {
                     if (!node.type().isHost()) continue;
                     NodeEntity nodeEntity = nodeEntities.get(registeredHostnameOf(node));
                     if (!shouldUpdate(node, nodeEntity)) continue;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
index fd375c80218..b40f2232504 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
@@ -94,7 +94,7 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
                 .ofCloud(CloudName.from("aws"))
                 .reachable().zones().stream()
                 .map(ZoneApi::getId)
-                .map(zoneId -> createResourceSnapshotsFromNodes(zoneId, nodeRepository.list(zoneId)))
+                .map(zoneId -> createResourceSnapshotsFromNodes(zoneId, nodeRepository.list(zoneId, false)))
                 .flatMap(Collection::stream)
                 .collect(Collectors.toList());
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java
index 634e5ba10ce..c7bf7e765ed 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java
@@ -43,7 +43,7 @@ public class ResourceTagMaintainer extends ControllerMaintainer {
 
     private Map<HostName, Optional<ApplicationId>> getTenantOfParentHosts(ZoneId zoneId) {
         return controller().serviceRegistry().configServer().nodeRepository()
-                .list(zoneId)
+                .list(zoneId, false)
                 .stream()
                 .filter(node -> node.type().isHost())
                 .collect(Collectors.toMap(
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java
index 5d92166d758..b6468464a0b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java
@@ -48,7 +48,7 @@ public class CostCalculator {
         Map<Property, ResourceAllocation> allocationByProperty = new HashMap<>();
         var nodes = controller.zoneRegistry().zones()
                               .reachable().in(Environment.prod).ofCloud(cloudName).zones().stream()
-                              .flatMap(zone -> uncheck(() -> nodeRepository.list(zone.getId()).stream()))
+                              .flatMap(zone -> uncheck(() -> nodeRepository.list(zone.getId(), false).stream()))
                               .filter(node -> node.owner().isPresent() && !node.owner().get().tenant().equals(SystemApplication.TENANT))
                               .collect(Collectors.toList());
         var totalAllocation = ResourceAllocation.ZERO;
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 2eabc5dc21b..bcede747405 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
@@ -33,7 +33,6 @@ import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import java.util.function.Function;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
@@ -162,7 +161,7 @@ public class NodeRepositoryMock implements NodeRepository {
     }
 
     @Override
-    public void setState(ZoneId zone, NodeState nodeState, String nodename) {
+    public void setState(ZoneId zone, NodeState nodeState, String hostname) {
         throw new UnsupportedOperationException();
     }
 
@@ -177,17 +176,7 @@ public class NodeRepositoryMock implements NodeRepository {
     }
 
     @Override
-    public NodeList listNodes(ZoneId zone, ApplicationId application) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public NodeList listNodes(ZoneId zone, List<HostName> hostnames) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public List<Node> list(ZoneId zone) {
+    public List<Node> list(ZoneId zone, boolean includeDeprovisioned) {
         return List.copyOf(nodeRepository.getOrDefault(zone, Map.of()).values());
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
index d14d4014b48..680743055c9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
@@ -128,7 +128,7 @@ public class CloudEventReporterTest {
     }
 
     private Set<String> getHostnames(ZoneId zoneId) {
-        return tester.configServer().nodeRepository().list(zoneId)
+        return tester.configServer().nodeRepository().list(zoneId, false)
                 .stream()
                 .map(node -> node.hostname().value())
                 .collect(Collectors.toSet());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdaterTest.java
index 4dcacb3934b..50c96b19d30 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdaterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/HostSwitchUpdaterTest.java
@@ -90,7 +90,7 @@ public class HostSwitchUpdaterTest {
     private static List<Node> allNodes(ControllerTester tester) {
         List<Node> nodes = new ArrayList<>();
         for (var zone : tester.zoneRegistry().zones().controllerUpgraded().all().ids()) {
-            nodes.addAll(tester.serviceRegistry().configServer().nodeRepository().list(zone));
+            nodes.addAll(tester.serviceRegistry().configServer().nodeRepository().list(zone, false));
         }
         return nodes;
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
index 1b43fe2aaa5..d42342b57fb 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
@@ -357,7 +357,7 @@ public class MetricsReporterTest {
         tester.configServer().setOsVersion(version0, SystemApplication.tenantHost.id(), zone);
         tester.configServer().setOsVersion(version0, SystemApplication.configServerHost.id(), zone);
         runAll(statusUpdater, reporter);
-        List<Node> hosts = tester.configServer().nodeRepository().list(zone);
+        List<Node> hosts = tester.configServer().nodeRepository().list(zone, false);
         assertOsChangeDuration(Duration.ZERO, hosts);
 
         var targets = List.of(Version.fromString("8.1"), Version.fromString("8.2"));
@@ -381,7 +381,7 @@ public class MetricsReporterTest {
                          tester.configServer().nodeRepository().list(zone, SystemApplication.tenantHost.id()).stream()
                                .map(Node::wantedOsVersion).min(Comparator.naturalOrder()).get());
             assertTrue("No nodes are suspended", tester.controller().serviceRegistry().configServer()
-                                                       .nodeRepository().list(zone).stream()
+                                                       .nodeRepository().list(zone, false).stream()
                                                        .noneMatch(node -> node.serviceState() == Node.ServiceState.allowedDown));
 
             // Another 30 minutes pass