Support in-place node resize in certain cases

4 files changed, 293 insertions, 23 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index 1b8f5f27a97..ab51fb13414 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -4,8 +4,9 @@ package com.yahoo.vespa.hosted.provision.provisioning;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeFlavors;
+import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.Zone;
@@ -110,18 +111,18 @@ class NodeAllocation {
                 if (requestedNodes.considerRetiring()) {
                     boolean wantToRetireNode = false;
                     if (violatesParentHostPolicy(this.nodes, offered)) wantToRetireNode = true;
-                    if ( ! hasCompatibleFlavor(offered)) wantToRetireNode = true;
+                    if ( ! hasCompatibleFlavor(node)) wantToRetireNode = true;
                     if (offered.status().wantToRetire()) wantToRetireNode = true;
                     if (requestedNodes.isExclusive() && ! hostsOnly(application.tenant(), offered.parentHostname()))
                         wantToRetireNode = true;
-                    if (( ! saturated() && hasCompatibleFlavor(offered)) || acceptToRetire(offered))
-                        accepted.add(acceptNode(node, wantToRetireNode));
+                    if (( ! saturated() && hasCompatibleFlavor(node)) || acceptToRetire(node))
+                        accepted.add(acceptNode(node, wantToRetireNode, node.isResizable));
                 }
                 else {
-                    accepted.add(acceptNode(node, false));
+                    accepted.add(acceptNode(node, false, false));
                 }
             }
-            else if ( ! saturated() && hasCompatibleFlavor(offered)) {
+            else if ( ! saturated() && hasCompatibleFlavor(node)) {
                 if ( violatesParentHostPolicy(this.nodes, offered)) {
                     ++rejectedWithClashingParentHost;
                     continue;
@@ -141,7 +142,7 @@ class NodeAllocation {
                                                         ClusterMembership.from(cluster, highestIndex.add(1)),
                                                         requestedNodes.resources().orElse(node.node.flavor().resources()),
                                                         clock.instant());
-                accepted.add(acceptNode(node, false));
+                accepted.add(acceptNode(node, false, false));
             }
         }
 
@@ -209,25 +210,32 @@ class NodeAllocation {
      * initialized. (In the other case, where a container node is not desired because we have enough nodes we
      * do want to remove it immediately to get immediate feedback on how the size reduction works out.)
      */
-    private boolean acceptToRetire(Node node) {
-        if (node.state() != Node.State.active) return false;
-        if (! node.allocation().get().membership().cluster().group().equals(cluster.group())) return false;
+    private boolean acceptToRetire(PrioritizableNode node) {
+        if (node.node.state() != Node.State.active) return false;
+        if (! node.node.allocation().get().membership().cluster().group().equals(cluster.group())) return false;
 
         return cluster.type().isContent() ||
                (cluster.type() == ClusterSpec.Type.container && !hasCompatibleFlavor(node));
     }
 
-    private boolean hasCompatibleFlavor(Node node) {
-        return requestedNodes.isCompatible(node.flavor(), flavors);
+    private boolean hasCompatibleFlavor(PrioritizableNode node) {
+        return requestedNodes.isCompatible(node.node.flavor(), flavors) || node.isResizable;
     }
 
-    private Node acceptNode(PrioritizableNode prioritizableNode, boolean wantToRetire) {
+    private Node acceptNode(PrioritizableNode prioritizableNode, boolean wantToRetire, boolean resize) {
         Node node = prioritizableNode.node;
 
         if (node.allocation().isPresent()) // Record the currently requested resources
             node = node.with(node.allocation().get().withRequestedResources(requestedNodes.resources().orElse(node.flavor().resources())));
 
         if (! wantToRetire) {
+            if (resize) {
+                NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources();
+                node = node.with(new Flavor(requestedNodes.resources().get()
+                        .with(hostResources.diskSpeed())
+                        .with(hostResources.storageType())));
+            }
+
             if (node.state() != Node.State.active) // reactivated node - make sure its not retired
                 node = node.unretire();
 
@@ -313,7 +321,7 @@ class NodeAllocation {
         }
         else if (deltaRetiredCount < 0) { // unretire until deltaRetiredCount is 0
             for (PrioritizableNode node : byIncreasingIndex(nodes)) {
-                if ( node.node.allocation().get().membership().retired() && hasCompatibleFlavor(node.node)) {
+                if ( node.node.allocation().get().membership().retired() && hasCompatibleFlavor(node)) {
                     node.node = node.node.unretire();
                     if (++deltaRetiredCount == 0) break;
                 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
index 1e77f33a99c..c7182571f31 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
@@ -45,6 +45,8 @@ public class NodePrioritizer {
     private final NameResolver nameResolver;
     private final boolean isDocker;
     private final boolean isAllocatingForReplacement;
+    private final boolean isTopologyChange;
+    private final int currentClusterSize;
     private final Set<Node> spareHosts;
 
     NodePrioritizer(LockedNodeList allNodes, ApplicationId appId, ClusterSpec clusterSpec, NodeSpec nodeSpec,
@@ -58,6 +60,18 @@ public class NodePrioritizer {
         this.spareHosts = findSpareHosts(allNodes, capacity, spares);
 
         NodeList nodesInCluster = allNodes.owner(appId).type(clusterSpec.type()).cluster(clusterSpec.id());
+        long currentGroups = nodesInCluster.state(Node.State.active).stream()
+                .flatMap(node -> node.allocation()
+                        .flatMap(alloc -> alloc.membership().cluster().group().map(ClusterSpec.Group::index))
+                        .stream())
+                .distinct()
+                .count();
+        this.isTopologyChange = currentGroups != wantedGroups;
+
+        this.currentClusterSize = (int) nodesInCluster.state(Node.State.active).stream()
+                .map(node -> node.allocation().flatMap(alloc -> alloc.membership().cluster().group()))
+                .filter(clusterSpec.group()::equals)
+                .count();
 
         this.isAllocatingForReplacement = isReplacement(
                 nodesInCluster.size(),
@@ -195,10 +209,15 @@ public class NodePrioritizer {
                 .newNode(isNewNode);
 
         allNodes.parentOf(node).ifPresent(parent -> {
-            builder.parent(parent).freeParentCapacity(capacity.freeCapacityOf(parent, false));
-            if (spareHosts.contains(parent)) {
+            NodeResources parentCapacity = capacity.freeCapacityOf(parent, false);
+            builder.parent(parent).freeParentCapacity(parentCapacity);
+
+            if (!isNewNode)
+                builder.resizable(requestedNodes.canResize(
+                        node.flavor().resources(), parentCapacity, isTopologyChange, currentClusterSize));
+
+            if (spareHosts.contains(parent))
                 builder.violatesSpares(true);
-            }
         });
 
         return builder.build();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
index c0b67956716..45fc1e6934c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
@@ -5,7 +5,6 @@ import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.vespa.hosted.provision.Node;
 
 import java.util.Objects;
 import java.util.Optional;
@@ -57,6 +56,15 @@ public interface NodeSpec {
     /** Returns the resources requested by this or empty if none are explicitly requested */
     Optional<NodeResources> resources();
 
+    /**
+     * Returns true if a node with given current resources and current spare host resources can be resized
+     * in-place to resources in this spec.
+     */
+    default boolean canResize(NodeResources currentNodeResources, NodeResources currentSpareHostResources,
+                              boolean hasTopologyChange, int currentClusterSize) {
+        return false;
+    }
+
     static NodeSpec from(int nodeCount, NodeResources resources, boolean exclusive, boolean canFail) {
         return new CountNodeSpec(nodeCount, resources, exclusive, canFail);
     }
@@ -126,11 +134,17 @@ public interface NodeSpec {
         }
 
         @Override
-        public Node assignRequestedFlavor(Node node) {
-            // Docker nodes can change flavor in place - disabled - see below
-            // if (requestedFlavorCanBeAchievedByResizing(node.flavor()))
-            //    return node.with(requestedFlavor);
-            return node;
+        public boolean canResize(NodeResources currentNodeResources, NodeResources currentSpareHostResources,
+                                 boolean hasTopologyChange, int currentClusterSize) {
+            // Never allow in-place resize when also changing topology or decreasing cluster size
+            if (hasTopologyChange || count < currentClusterSize) return false;
+
+            // Do not allow increasing cluster size and decreasing node resources at the same time
+            if (count > currentClusterSize && !requestedNodeResources.satisfies(currentNodeResources.justNumbers()))
+                return false;
+
+            // Otherwise, allowed as long as the host can satisfy the new requested resources
+            return currentSpareHostResources.add(currentNodeResources.justNumbers()).satisfies(requestedNodeResources);
         }
 
         @Override
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java
new file mode 100644
index 00000000000..b0aa0ed514e
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java
@@ -0,0 +1,229 @@
+// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.provisioning;
+
+import com.yahoo.component.Version;
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.HostSpec;
+import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.config.provision.OutOfCapacityException;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.Zone;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeList;
+import org.junit.Test;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast;
+import static com.yahoo.config.provision.NodeResources.StorageType.local;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * If there is no change in cluster size or topology, any increase in node resource allocation is fine as long as:
+ * a. We have the necessary spare resources available on the all hosts used in the cluster
+ * b. We have the necessary spare resources available on a subset of the hosts used in the cluster AND
+ *    also have available capacity to migrate the remaining nodes to different hosts.
+ * c. Any decrease in node resource allocation is fine.
+ *
+ * If there is an increase in cluster size, this can be combined with increase in resource allocations given there is
+ * available resources and new nodes.
+ *
+ * No other changes should be supported at this time, due to risks in complexity and possibly unknowns.
+ * Specifically, the following is intentionally not supported by the above changes:
+ *  a. Decrease in resource allocation combined with cluster size increase
+ *  b. Change in resource allocation combined with cluster size reduction
+ *  c. Change in resource allocation combined with cluster topology changes
+ *
+ * @author freva
+ */
+public class InPlaceResizeProvisionTest {
+    private static final NodeResources smallResources = new NodeResources(2, 4, 8, 1, NodeResources.DiskSpeed.any, NodeResources.StorageType.any);
+    private static final NodeResources mediumResources = new NodeResources(4, 8, 16, 1, NodeResources.DiskSpeed.any, NodeResources.StorageType.any);
+    private static final NodeResources largeResources = new NodeResources(8, 16, 32, 1, NodeResources.DiskSpeed.any, NodeResources.StorageType.any);
+
+    private static final ClusterSpec container1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container1"), Version.fromString("7.157.9"), false);
+    private static final ClusterSpec container2 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("container2"), Version.fromString("7.157.9"), false);
+    private static final ClusterSpec content1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("content1"), Version.fromString("7.157.9"), false);
+
+    private final ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();
+    private final ApplicationId infraApp = tester.makeApplicationId();
+    private final ApplicationId app = tester.makeApplicationId();
+
+    @Test
+    public void single_group_same_cluster_size_resource_increase() {
+        addParentHosts(4, largeResources.with(fast).with(local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate();
+        assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, largeResources).activate();
+        assertClusterSizeAndResources(container1, 4, new NodeResources(8, 16, 32, 1, fast, local));
+        assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size());
+    }
+
+    @Test
+    public void single_group_same_cluster_size_resource_decrease() {
+        addParentHosts(4, mediumResources.with(fast).with(local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate();
+        assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, smallResources).activate();
+        assertClusterSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local));
+        assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size());
+    }
+
+    @Test
+    public void two_groups_with_resources_increase_and_decrease() {
+        addParentHosts(8, mediumResources.with(fast).with(local));
+
+        new PrepareHelper(tester, app)
+                .prepare(container1, 4, 1, smallResources)
+                .prepare(container2, 4, 1, mediumResources)
+                .activate();
+        Set<String> container1Hostnames = listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet());
+        assertClusterSizeAndResources(container1, 4, new NodeResources(2, 4, 8, 1, fast, local));
+        assertClusterSizeAndResources(container2, 4, new NodeResources(4, 8, 16, 1, fast, local));
+
+        new PrepareHelper(tester, app)
+                .prepare(container1, 4, 1, mediumResources)
+                .prepare(container2, 4, 1, smallResources)
+                .activate();
+        assertEquals(container1Hostnames, listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet()));
+        assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local));
+        assertClusterSizeAndResources(container2, 4, new NodeResources(2, 4, 8, 1, fast, local));
+        assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size());
+    }
+
+    @Test
+    public void cluster_size_and_resources_increase() {
+        addParentHosts(6, largeResources.with(fast).with(local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate();
+        Set<String> initialHostnames = listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet());
+
+        new PrepareHelper(tester, app)
+                .prepare(container1, 6, 1, largeResources).activate();
+        assertTrue(listCluster(container1).stream().map(Node::hostname).collect(Collectors.toSet()).containsAll(initialHostnames));
+        assertClusterSizeAndResources(container1, 6, new NodeResources(8, 16, 32, 1, fast, local));
+        assertEquals("No nodes are retired", 0, tester.getNodes(app, Node.State.active).retired().size());
+    }
+
+    @Test
+    public void partial_in_place_resource_increase() {
+        addParentHosts(4, new NodeResources(8, 16, 32, 8, fast, local));
+
+        // Allocate 2 nodes for one app that leaves exactly enough capacity for mediumResources left on the host
+        new PrepareHelper(tester, tester.makeApplicationId()).prepare(container1, 2, 1, mediumResources).activate();
+
+        // Allocate 4 nodes for another app. After this, 2 hosts should be completely full
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate();
+
+        // Attempt to increase resources of the other app
+        try {
+            new PrepareHelper(tester, app).prepare(container1, 4, 1, largeResources);
+            fail("Expected to fail due to out of capacity");
+        } catch (OutOfCapacityException ignored) { }
+
+        // Add 2 more parent host, now we should be able to do the same deployment that failed earlier
+        // 2 of the nodes will be increased in-place and 2 will be allocated to the new hosts.
+        addParentHosts(2, new NodeResources(8, 16, 32, 8, fast, local));
+
+        Set<String> initialHostnames = listCluster(container1).stream().map(Node::hostname)
+                .collect(Collectors.collectingAndThen(Collectors.toSet(), HashSet::new));
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, largeResources).activate();
+        NodeList appNodes = tester.getNodes(app, Node.State.active);
+        assertEquals(6, appNodes.size()); // 4 nodes with large resources + 2 retired nodes with medium resources
+        appNodes.forEach(node -> {
+            if (node.allocation().get().membership().retired())
+                assertEquals(new NodeResources(4, 8, 16, 1, fast, local), node.flavor().resources());
+            else
+                assertEquals(new NodeResources(8, 16, 32, 1, fast, local), node.flavor().resources());
+            initialHostnames.remove(node.hostname());
+        });
+        assertTrue("All initial nodes should still be allocated to the application", initialHostnames.isEmpty());
+    }
+
+    @Test(expected = OutOfCapacityException.class)
+    public void cannot_inplace_decrease_resources_while_increasing_cluster_size() {
+        addParentHosts(6, mediumResources.with(fast).with(local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate();
+        assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local));
+
+        new PrepareHelper(tester, app).prepare(container1, 6, 1, smallResources);
+    }
+
+    @Test(expected = OutOfCapacityException.class)
+    public void cannot_inplace_change_resources_while_decreasing_cluster_size() {
+        addParentHosts(4, largeResources.with(fast).with(local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate();
+        assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local));
+
+        new PrepareHelper(tester, app).prepare(container1, 2, 1, smallResources);
+    }
+
+    @Test(expected = OutOfCapacityException.class)
+    public void cannot_inplace_change_resources_while_changing_topology() {
+        addParentHosts(4, largeResources.with(fast).with(local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 1, mediumResources).activate();
+        assertClusterSizeAndResources(container1, 4, new NodeResources(4, 8, 16, 1, fast, local));
+
+        new PrepareHelper(tester, app).prepare(container1, 4, 2, smallResources);
+    }
+
+    private void addParentHosts(int count, NodeResources resources) {
+        tester.makeReadyNodes(count, resources, NodeType.host, 4);
+        tester.prepareAndActivateInfraApplication(infraApp, NodeType.host);
+    }
+
+    private void assertClusterSizeAndResources(ClusterSpec cluster, int clusterSize, NodeResources resources) {
+        NodeList nodes = listCluster(cluster);
+        nodes.forEach(node -> assertEquals(node.toString(), node.flavor().resources(), resources));
+        assertEquals(clusterSize, nodes.size());
+    }
+
+    private NodeList listCluster(ClusterSpec cluster) {
+        return tester.getNodes(app, Node.State.active)
+                .filter(node -> node.allocation().get().membership().cluster().satisfies(cluster));
+    }
+
+    private static class PrepareHelper {
+        private final Set<HostSpec> preparedNodes = new HashSet<>();
+        private final ProvisioningTester tester;
+        private final ApplicationId application;
+        private boolean activated = false;
+
+        private PrepareHelper(ProvisioningTester tester, ApplicationId application) {
+            this.tester = tester;
+            this.application = application;
+        }
+
+        private PrepareHelper prepare(ClusterSpec cluster, int nodeCount, int groups, NodeResources resources) {
+            return prepare(cluster, nodeCount, groups, false, resources);
+        }
+
+        private PrepareHelper prepare(ClusterSpec cluster, int nodeCount, int groups, boolean required, NodeResources resources) {
+            if (this.activated) throw new IllegalArgumentException("Deployment was already activated");
+            preparedNodes.addAll(tester.prepare(application, cluster, nodeCount, groups, required, resources));
+            return this;
+        }
+
+        private Collection<HostSpec> activate() {
+            if (this.activated) throw new IllegalArgumentException("Deployment was already activated");
+            Collection<HostSpec> activated = tester.activate(application, preparedNodes);
+            this.activated = true;
+            return activated;
+        }
+    }
+}