Merge pull request #16801 from vespa-engine/freva/clear-allocation

Remove allocation on expiry from dirty in dynamically provisioned zones

3 files changed, 78 insertions, 2 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java
index 22c8e49825d..f072891f210 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirer.java
@@ -23,14 +23,18 @@ import java.util.List;
  */
 public class DirtyExpirer extends Expirer {
 
+    private final boolean keepAllocationOnExpiry;
+
     DirtyExpirer(NodeRepository nodeRepository, Duration dirtyTimeout, Metric metric) {
         super(Node.State.dirty, History.Event.Type.deallocated, nodeRepository, dirtyTimeout, metric);
+        // Do not keep allocation in dynamically provisioned zones so that the hosts can be deprovisioned
+        this.keepAllocationOnExpiry = ! nodeRepository.zone().getCloud().dynamicProvisioning();
     }
 
     @Override
     protected void expire(List<Node> expired) {
         for (Node expiredNode : expired)
-            nodeRepository().nodes().fail(expiredNode.hostname(), Agent.DirtyExpirer, "Node is stuck in dirty");
+            nodeRepository().nodes().fail(expiredNode.hostname(), keepAllocationOnExpiry, Agent.DirtyExpirer, "Node is stuck in dirty");
     }
 
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
index 534115342f3..bb50d6fcc6f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
@@ -287,7 +287,11 @@ public class Nodes {
      * @throws NoSuchNodeException if the node is not found
      */
     public Node fail(String hostname, Agent agent, String reason) {
-        return move(hostname, true, Node.State.failed, agent, Optional.of(reason));
+        return fail(hostname, true, agent, reason);
+    }
+
+    public Node fail(String hostname, boolean keepAllocation, Agent agent, String reason) {
+        return move(hostname, keepAllocation, Node.State.failed, agent, Optional.of(reason));
     }
 
     /**
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirerTest.java
new file mode 100644
index 00000000000..cf5cfb93782
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirerTest.java
@@ -0,0 +1,68 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.maintenance;
+
+import com.yahoo.component.Version;
+import com.yahoo.config.provision.Cloud;
+import com.yahoo.config.provision.ClusterMembership;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.Flavor;
+import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.Zone;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.node.Agent;
+import com.yahoo.vespa.hosted.provision.node.Allocation;
+import com.yahoo.vespa.hosted.provision.node.Generation;
+import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester;
+import com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.List;
+import java.util.Optional;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author freva
+ */
+public class DirtyExpirerTest {
+
+    @Test
+    public void assert_allocation_after_expiry() {
+        assertAllocationAfterExpiry(true);
+        assertAllocationAfterExpiry(false);
+    }
+
+    private void assertAllocationAfterExpiry(boolean dynamicProvisioning) {
+        Zone zone = new Zone(Cloud.builder().dynamicProvisioning(dynamicProvisioning).build(), SystemName.main, Environment.prod, RegionName.from("us-east"));
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone)
+                .hostProvisioner(dynamicProvisioning ? new MockHostProvisioner(List.of()) : null)
+                .build();
+
+        Node node = Node.create("id", "node1.domain.tld", new Flavor(NodeResources.unspecified()), Node.State.dirty, NodeType.tenant)
+                .allocation(new Allocation(ProvisioningTester.applicationId(),
+                        ClusterMembership.from("container/default/0/0", Version.fromString("1.2.3"), Optional.empty()),
+                        NodeResources.unspecified(),
+                        Generation.initial(),
+                        false))
+                .build();
+
+        tester.nodeRepository().database().addNodesInState(List.of(node), node.state(), Agent.system);
+
+        Duration expiryTimeout = Duration.ofMinutes(30);
+        DirtyExpirer expirer = new DirtyExpirer(tester.nodeRepository(), expiryTimeout, new TestMetric());
+
+        assertEquals(Node.State.dirty, tester.nodeRepository().nodes().list().first().get().state());
+        expirer.run();
+        assertEquals(Node.State.dirty, tester.nodeRepository().nodes().list().first().get().state());
+
+        tester.clock().advance(expiryTimeout.plusSeconds(1));
+        expirer.run();
+        assertEquals(Node.State.failed, tester.nodeRepository().nodes().list().first().get().state());
+        assertEquals(dynamicProvisioning, tester.nodeRepository().nodes().list().first().get().allocation().isEmpty());
+    }
+
+}
\ No newline at end of file