Do not expire non-tenant non-host types from failed

3 files changed, 41 insertions, 16 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
index 7058917d351..2e17c2d6f67 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
@@ -9,6 +9,7 @@ import com.yahoo.config.provision.NodeType;
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
@@ -68,9 +69,10 @@ public class NodeList implements Iterable<Node> {
         return filter(node -> node.allocation().map(a -> a.owner().equals(application)).orElse(false));
     }
 
-    /** Returns the subset of nodes matching the given node type */
-    public NodeList nodeType(NodeType nodeType) {
-        return filter(node -> node.type() == nodeType);
+    /** Returns the subset of nodes matching the given node type(s) */
+    public NodeList nodeType(NodeType first, NodeType... rest) {
+        EnumSet<NodeType> nodeTypes = EnumSet.of(first, rest);
+        return filter(node -> nodeTypes.contains(node.type()));
     }
 
     /** Returns the subset of nodes that are parents */
@@ -87,9 +89,10 @@ public class NodeList implements Iterable<Node> {
         return childrenOf(parent.hostname());
     }
 
-    /** Returns the subset of nodes that are in a given state */
-    public NodeList state(Node.State state) {
-        return filter(node -> node.state() == state);
+    /** Returns the subset of nodes that are in a given state(s) */
+    public NodeList state(Node.State first, Node.State... rest) {
+        EnumSet<Node.State> nodeStates = EnumSet.of(first, rest);
+        return filter(node -> nodeStates.contains(node.state()));
     }
 
     /** Returns the parent nodes of the given child nodes */
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
index 3d8639a43be..4b086e1ea09 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java
@@ -4,9 +4,9 @@ package com.yahoo.vespa.hosted.provision.maintenance;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Flavor;
+import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
-import com.yahoo.document.datatypes.Array;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
@@ -77,7 +77,10 @@ public class FailedExpirer extends Maintainer {
 
     @Override
     protected void maintain() {
-        List<Node> remainingNodes = new ArrayList<>(nodeRepository.getNodes(Node.State.failed));
+        List<Node> remainingNodes = new ArrayList<>(nodeRepository.list()
+                .state(Node.State.failed)
+                .nodeType(NodeType.tenant, NodeType.host)
+                .asList());
 
         recycleIf(remainingNodes, node -> node.allocation().isEmpty());
         recycleIf(remainingNodes, node ->
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java
index 0da799eed30..6fa5afd0c20 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java
@@ -9,10 +9,10 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Flavor;
-import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.NodeFlavors;
+import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
@@ -33,10 +33,9 @@ import com.yahoo.vespa.hosted.provision.testutils.MockProvisionServiceProvider;
 import org.junit.Test;
 
 import java.time.Duration;
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.UUID;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -51,8 +50,7 @@ public class FailedExpirerTest {
 
     private static final ApplicationId tenantHostApplicationId = ApplicationId.from("vespa", "zone-app", "default");
     private static final ClusterSpec tenantHostApplicationClusterSpec =  ClusterSpec.request(
-            ClusterSpec.Type.container, ClusterSpec.Id.from("node-admin"), Version.fromString("6.42"), false,
-            Collections.emptySet());
+            ClusterSpec.Type.container, ClusterSpec.Id.from("node-admin"), Version.fromString("6.42"), false, Set.of());
     private static final Capacity tenantHostApplicationCapacity = Capacity.fromRequiredNodeType(NodeType.host);
 
     @Test
@@ -131,6 +129,27 @@ public class FailedExpirerTest {
     }
 
     @Test
+    public void ensure_non_tenant_nodes_and_hosts_are_not_recycled() {
+        FailureScenario scenario = new FailureScenario(SystemName.main, Environment.prod)
+                .withNode(NodeType.proxy, FailureScenario.defaultFlavor, "proxy1")
+                .withNode(NodeType.proxy, FailureScenario.defaultFlavor, "proxy2")
+                .withNode(NodeType.proxy, FailureScenario.defaultFlavor, "proxy3")
+                .setReady("proxy1", "proxy2", "proxy3")
+                .allocate( ApplicationId.from("vespa", "zone-app", "default"),
+                        ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("routing"), Version.fromString("6.42"), false, Set.of()),
+                        Capacity.fromRequiredNodeType(NodeType.proxy))
+                .failNode(1, "proxy1");
+
+        for (int i = 0; i < 10; i++) {
+            scenario.clock().advance(Duration.ofHours(2));
+            scenario.expirer().run();
+        }
+
+        scenario.assertNodesIn(Node.State.failed, "proxy1");
+        scenario.assertNodesIn(Node.State.active, "proxy2", "proxy3");
+    }
+
+    @Test
     public void ensure_failed_docker_nodes_are_deallocated() {
         FailureScenario scenario = new FailureScenario(SystemName.main, Environment.prod)
                 .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent1")
@@ -274,7 +293,7 @@ public class FailedExpirerTest {
         }
 
         public FailureScenario withNode(NodeType type, NodeResources flavor, String hostname, String parentHostname) {
-            nodeRepository.addNodes(Collections.singletonList(
+            nodeRepository.addNodes(List.of(
                     nodeRepository.createNode(UUID.randomUUID().toString(), hostname,
                                               Optional.ofNullable(parentHostname), new Flavor(flavor), type)
             ));
@@ -326,7 +345,7 @@ public class FailedExpirerTest {
                                                           ClusterSpec.Id.from("test"),
                                                           Version.fromString("6.42"),
                                                           false,
-                                                          Collections.emptySet());
+                                                          Set.of());
             Capacity capacity = Capacity.fromCount(hostname.length, Optional.of(flavor), false, true);
             return allocate(applicationId, clusterSpec, capacity);
         }
@@ -334,7 +353,7 @@ public class FailedExpirerTest {
         public FailureScenario allocate(ApplicationId applicationId, ClusterSpec clusterSpec, Capacity capacity) {
             List<HostSpec> preparedNodes = provisioner.prepare(applicationId, clusterSpec, capacity, 1, null);
             NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator));
-            provisioner.activate(transaction, applicationId, new HashSet<>(preparedNodes));
+            provisioner.activate(transaction, applicationId, Set.copyOf(preparedNodes));
             transaction.commit();
             return this;
         }