Always recycle failed Docker containers

2 files changed, 35 insertions, 20 deletions

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 79c0eff874a..52c5a13b44c 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
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.maintenance;
 
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.hosted.provision.Node;
@@ -49,16 +50,15 @@ public class FailedExpirer extends Expirer {
         List<Node> nodesToRecycle = new ArrayList<>();
         for (Node recycleCandidate : expired) {
             if (recycleCandidate.status().hardwareFailure().isPresent()) continue;
-            if (failCountIndicatesHwFail(zone) && recycleCandidate.status().failCount() >= 5) continue;
+            if (failCountIndicatesHwFail(zone, recycleCandidate) && recycleCandidate.status().failCount() >= 5) continue;
             nodesToRecycle.add(recycleCandidate);
         }
         nodeRepository.setDirty(nodesToRecycle);
     }
 
-    private boolean failCountIndicatesHwFail(Zone zone) {
-        if (zone.system() == SystemName.cd) {
-            return false;
-        }
+    private boolean failCountIndicatesHwFail(Zone zone, Node node) {
+        if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) return false;
+        if (zone.system() == SystemName.cd) return false;
         return zone.environment() == Environment.prod || zone.environment() == Environment.staging;
     }
 
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 35232e703c1..b69c7815de6 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
@@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.RegionName;
@@ -44,7 +45,7 @@ public class FailedExpirerTest {
 
     @Test
     public void ensure_failed_nodes_are_deallocated_in_prod() throws InterruptedException {
-        NodeRepository nodeRepository = failureScenarioIn(SystemName.main, Environment.prod);
+        NodeRepository nodeRepository = failureScenarioIn(SystemName.main, Environment.prod, "default");
 
         assertEquals(2, nodeRepository.getNodes(NodeType.tenant, Node.State.failed).size());
         assertEquals(1, nodeRepository.getNodes(NodeType.tenant, Node.State.dirty).size());
@@ -53,7 +54,7 @@ public class FailedExpirerTest {
 
     @Test
     public void ensure_failed_nodes_are_deallocated_in_dev() throws InterruptedException {
-        NodeRepository nodeRepository = failureScenarioIn(SystemName.main, Environment.dev);
+        NodeRepository nodeRepository = failureScenarioIn(SystemName.main, Environment.dev, "default");
 
         assertEquals(1, nodeRepository.getNodes(NodeType.tenant, Node.State.failed).size());
         assertEquals(2, nodeRepository.getNodes(NodeType.tenant, Node.State.dirty).size());
@@ -62,31 +63,45 @@ public class FailedExpirerTest {
 
     @Test
     public void ensure_failed_nodes_are_deallocated_in_cd() throws InterruptedException {
-        NodeRepository nodeRepository = failureScenarioIn(SystemName.cd, Environment.prod);
+        NodeRepository nodeRepository = failureScenarioIn(SystemName.cd, Environment.prod, "default");
 
         assertEquals(1, nodeRepository.getNodes(NodeType.tenant, Node.State.failed).size());
         assertEquals(2, nodeRepository.getNodes(NodeType.tenant, Node.State.dirty).size());
         assertEquals("node2", nodeRepository.getNodes(NodeType.tenant, Node.State.failed).get(0).hostname());
     }
 
-    private NodeRepository failureScenarioIn(SystemName system, Environment environment) {
+    @Test
+    public void ensure_failed_docker_nodes_are_deallocated() throws InterruptedException {
+        NodeRepository nodeRepository = failureScenarioIn(SystemName.main, Environment.prod, "docker");
+
+        assertEquals(1, nodeRepository.getNodes(NodeType.tenant, Node.State.failed).size());
+        assertEquals(2, nodeRepository.getNodes(NodeType.tenant, Node.State.dirty).size());
+        assertEquals("node2", nodeRepository.getNodes(NodeType.tenant, Node.State.failed).get(0).hostname());
+    }
+
+    private NodeRepository failureScenarioIn(SystemName system, Environment environment, String flavorName) {
         ManualClock clock = new ManualClock();
-        NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default");
+        NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default", flavorName);
         NodeRepository nodeRepository = new NodeRepository(nodeFlavors, curator, clock, Zone.defaultZone(),
                 new MockNameResolver().mockAnyLookup());
         NodeRepositoryProvisioner provisioner = new NodeRepositoryProvisioner(nodeRepository, nodeFlavors, Zone.defaultZone(), clock);
 
-        List<Node> nodes = new ArrayList<>(3);
-        nodes.add(nodeRepository.createNode("node1", "node1", Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.tenant));
-        nodes.add(nodeRepository.createNode("node2", "node2", Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.tenant));
-        nodes.add(nodeRepository.createNode("node3", "node3", Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.tenant));
-        nodeRepository.addNodes(nodes);
-
-        List<Node> hostNodes = new ArrayList<>(1);
-        hostNodes.add(nodeRepository.createNode("parent1", "parent1", Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host));
-        hostNodes.add(nodeRepository.createNode("parent2", "parent2", Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host));
+        Flavor defaultFlavor = nodeFlavors.getFlavorOrThrow("default");
+        List<Node> hostNodes = new ArrayList<>(3);
+        hostNodes.add(nodeRepository.createNode("parent1", "parent1", Optional.empty(), defaultFlavor, NodeType.host));
+        hostNodes.add(nodeRepository.createNode("parent2", "parent2", Optional.empty(), defaultFlavor, NodeType.host));
+        hostNodes.add(nodeRepository.createNode("parent3", "parent3", Optional.empty(), defaultFlavor, NodeType.host));
         nodeRepository.addNodes(hostNodes);
 
+        Flavor flavor = nodeFlavors.getFlavorOrThrow(flavorName);
+        List<Node> nodes = new ArrayList<>(3);
+        Optional<String> parentHost1 = flavorName.equals("docker") ? Optional.of("parent1") : Optional.empty();
+        Optional<String> parentHost2 = flavorName.equals("docker") ? Optional.of("parent2") : Optional.empty();
+        Optional<String> parentHost3 = flavorName.equals("docker") ? Optional.of("parent3") : Optional.empty();
+        nodes.add(nodeRepository.createNode("node1", "node1", parentHost1, flavor, NodeType.tenant));
+        nodes.add(nodeRepository.createNode("node2", "node2", parentHost2, flavor, NodeType.tenant));
+        nodes.add(nodeRepository.createNode("node3", "node3", parentHost3, flavor, NodeType.tenant));
+        nodeRepository.addNodes(nodes);
 
         // Set node1 to have failed 4 times before
         Node node1 = nodeRepository.getNode("node1").get();
@@ -106,7 +121,7 @@ public class FailedExpirerTest {
         nodeRepository.setReady(nodeRepository.setDirty(provisioned));
         ApplicationId applicationId = ApplicationId.from(TenantName.from("foo"), ApplicationName.from("bar"), InstanceName.from("fuz"));
         ClusterSpec cluster = ClusterSpec.requestVersion(ClusterSpec.Type.content, ClusterSpec.Id.from("test"), Optional.empty());
-        provisioner.prepare(applicationId, cluster, Capacity.fromNodeCount(3), 1, null);
+        provisioner.prepare(applicationId, cluster, Capacity.fromNodeCount(3, flavorName), 1, null);
         NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator));
         provisioner.activate(transaction, applicationId, ProvisioningTester.toHostSpecs(nodes));
         transaction.commit();