Extend existing reservations on deploy

3 files changed, 56 insertions, 1 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index 12a0f64aee1..78be6fe57d1 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -17,6 +17,7 @@ import com.yahoo.vespa.curator.transaction.CuratorOperations;
 import com.yahoo.vespa.curator.transaction.CuratorTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.node.Agent;
+import com.yahoo.vespa.hosted.provision.node.History;
 import com.yahoo.vespa.hosted.provision.node.Status;
 
 import java.nio.charset.StandardCharsets;
@@ -192,7 +193,7 @@ public class CuratorDatabaseClient {
                                     newNodeStatus(node, toState),
                                     toState,
                                     toState.isAllocated() ? node.allocation() : Optional.empty(),
-                                    node.history().recordStateTransition(node.state(), toState, agent, clock.instant()),
+                                    recordStateTransition(node, toState, agent),
                                     node.type());
             curatorTransaction.add(CuratorOperations.delete(toPath(node).getAbsolute()))
                               .add(CuratorOperations.create(toPath(toState, newNode.hostname()).getAbsolute(), nodeSerializer.toJson(newNode)));
@@ -208,6 +209,16 @@ public class CuratorDatabaseClient {
         return writtenNodes;
     }
 
+    private History recordStateTransition(Node node, Node.State toState, Agent agent) {
+        History history = node.history();
+        // When a node is re-reserved we want to update the reservation instant, we do this by removing the existing
+        // event and recording a new one
+        if (node.state() == Node.State.reserved && toState == Node.State.reserved) {
+            history = history.without(History.Event.Type.reserved);
+        }
+        return history.recordStateTransition(node.state(), toState, agent, clock.instant());
+    }
+
     private Status newNodeStatus(Node node, Node.State toState) {
         if (node.state() != Node.State.failed && toState == Node.State.failed) return node.status().withIncreasedFailCount();
         if (node.state() == Node.State.failed && toState == Node.State.active) return node.status().withDecreasedFailCount(); // fail undo
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
index a809fd88d97..49d802c7058 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
@@ -71,6 +71,9 @@ public class GroupPreparer {
                     throw new OutOfCapacityException("Could not satisfy " + requestedNodes + " for " + cluster +
                                                      outOfCapacityDetails(allocation));
 
+                // Extend reservation for already reserved nodes
+                nodeRepository.reserve(nodeRepository.getNodes(application, Node.State.reserved));
+
                 // Carry out and return allocation
                 nodeRepository.reserve(allocation.reservableNodes());
                 nodeRepository.addDockerNodes(allocation.newNodes());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index c80637fcdad..b9a54b700a6 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -24,6 +24,8 @@ import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
+import com.yahoo.vespa.hosted.provision.maintenance.JobControl;
+import com.yahoo.vespa.hosted.provision.maintenance.ReservationExpirer;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.History;
 import com.yahoo.vespa.hosted.provision.persistence.NameResolver;
@@ -31,6 +33,7 @@ import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
 import org.junit.Ignore;
 import org.junit.Test;
 
+import java.time.Duration;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -682,6 +685,44 @@ public class ProvisioningTest {
         }
     }
 
+    @Test
+    public void application_deployment_extends_existing_reservations_on_deploy() {
+        ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east")));
+
+        ApplicationId application = tester.makeApplicationId();
+        tester.makeReadyNodes(2, "default");
+
+        // Deploy fails with out of capacity
+        try {
+            prepare(application, 2, 0, 2, 0,
+                    "default", tester);
+            fail("Expected exception");
+        } catch (OutOfCapacityException ignored) {}
+        assertEquals("Reserved a subset of required nodes", 2,
+                     tester.getNodes(application, Node.State.reserved).size());
+
+        // Enough nodes become available
+        tester.makeReadyNodes(2, "default");
+
+        // Deploy is retried after a few minutes
+        tester.clock().advance(Duration.ofMinutes(2));
+        SystemState state = prepare(application, 2, 0, 2, 0,
+                                    "default", tester);
+        assertEquals("Reserved required nodes", 4,
+                     tester.getNodes(application, Node.State.reserved).size());
+
+        // Over 10 minutes pass since first reservation. First set of reserved nodes are not expired
+        tester.clock().advance(Duration.ofMinutes(8).plus(Duration.ofSeconds(1)));
+        ReservationExpirer expirer = new ReservationExpirer(tester.nodeRepository(), tester.clock(),
+                                                            Duration.ofMinutes(10),
+                                                            new JobControl(tester.nodeRepository().database()));
+        expirer.run();
+        assertEquals("Nodes remain reserved", 4,
+                     tester.getNodes(application, Node.State.reserved).size());
+        tester.activate(application, state.allHosts);
+        assertEquals(4, tester.getNodes(application, Node.State.active).size());
+    }
+
     private void assertCorrectFlavorPreferences(boolean largeIsStock) {
         FlavorConfigBuilder b = new FlavorConfigBuilder();
         b.addFlavor("large", 4., 8., 100, Flavor.Type.BARE_METAL).cost(10).stock(largeIsStock);