Don't propagate unspecified resources

13 files changed, 64 insertions, 16 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
index 2755030e2b3..849ea03665b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
@@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.NodeResourceLimits;
 
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 /**
  * @author bratseth
@@ -141,7 +142,6 @@ public class AllocatableClusterResources {
                                                              Limits applicationLimits,
                                                              NodeList hosts,
                                                              NodeRepository nodeRepository) {
-        var capacityPolicies = new CapacityPolicies(nodeRepository);
         var systemLimits = new NodeResourceLimits(nodeRepository);
         boolean exclusive = clusterSpec.isExclusive();
         if ( !clusterSpec.isExclusive() && !nodeRepository.zone().getCloud().dynamicProvisioning()) {
@@ -150,7 +150,9 @@ public class AllocatableClusterResources {
             advertisedResources = systemLimits.enlargeToLegal(advertisedResources, clusterSpec.type(), exclusive); // Ask for something legal
             advertisedResources = applicationLimits.cap(advertisedResources); // Overrides other conditions, even if it will then fail
             var realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources, exclusive); // What we'll really get
-            if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) return Optional.empty();
+            if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type()))
+                return Optional.empty();
+
             if (matchesAny(hosts, advertisedResources))
                     return Optional.of(new AllocatableClusterResources(wantedResources.with(realResources),
                                                                        advertisedResources,
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
index d727757b07e..30432c1c078 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
@@ -67,13 +67,11 @@ public class AllocationOptimizer {
                                                                                limits, target, current, clusterModel));
                 var allocatableResources = AllocatableClusterResources.from(next, current.clusterSpec(), limits,
                                                                             hosts, nodeRepository);
-
                 if (allocatableResources.isEmpty()) continue;
                 if (bestAllocation.isEmpty() || allocatableResources.get().preferableTo(bestAllocation.get()))
                     bestAllocation = allocatableResources;
             }
         }
-
         return bestAllocation;
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java
index 8c421443a65..1c3d3f5c489 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java
@@ -20,6 +20,7 @@ public class NodeResourcesSerializer {
     private static final String storageTypeKey = "storageType";
 
     static void toSlime(NodeResources resources, Cursor resourcesObject) {
+        if (resources.isUnspecified()) return;
         resourcesObject.setDouble(vcpuKey, resources.vcpu());
         resourcesObject.setDouble(memoryKey, resources.memoryGb());
         resourcesObject.setDouble(diskKey, resources.diskGb());
@@ -29,6 +30,7 @@ public class NodeResourcesSerializer {
     }
 
     static NodeResources resourcesFromSlime(Inspector resources) {
+        if ( ! resources.field(vcpuKey).valid()) return NodeResources.unspecified();
         return new NodeResources(resources.field(vcpuKey).asDouble(),
                                  resources.field(memoryKey).asDouble(),
                                  resources.field(diskKey).asDouble(),
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java
index 0d32b21016c..8c358301b85 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java
@@ -113,7 +113,8 @@ class Activator {
             var cluster = modified.cluster(clusterEntry.getKey()).get();
             var previousResources = oldNodes.cluster(clusterEntry.getKey()).toResources();
             var currentResources = clusterEntry.getValue().toResources();
-            if ( ! previousResources.justNumbers().equals(currentResources.justNumbers())) {
+            if ( previousResources.nodeResources().isUnspecified()
+                 || ! previousResources.justNumbers().equals(currentResources.justNumbers())) {
                 cluster = cluster.with(ScalingEvent.create(previousResources, currentResources, generation, at));
             }
             if (cluster.targetResources().isPresent()
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
index 2b9c5396724..4088d717a67 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java
@@ -57,6 +57,7 @@ public class CapacityPolicies {
 
     private NodeResources decideNodeResources(NodeResources target, boolean required) {
         if (required) return target;
+        if (target.isUnspecified()) return target; // Cannot be modified
 
         // Dev does not cap the cpu or network of containers since usage is spotty: Allocate just a small amount exclusively
         if (zone.environment() == Environment.dev && !zone.getCloud().dynamicProvisioning())
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 ba46f0a9535..2d93763c631 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,7 +71,7 @@ public class GroupPreparer {
         // Try preparing in memory without global unallocated lock. Most of the time there should be no changes and we
         // can return nodes previously allocated.
         NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes,
-                indices::probeNext, wantedGroups, allNodesAndHosts);
+                                                           indices::probeNext, wantedGroups, allNodesAndHosts);
         if (probeAllocation.fulfilledAndNoChanges()) {
             List<Node> acceptedNodes = probeAllocation.finalNodes();
             surplusActiveNodes.removeAll(acceptedNodes);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
index 4f0ae688b1c..62ac1f0d0e6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
@@ -238,7 +238,6 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat
 
     private double skewWith(NodeResources resources) {
         if (parent.isEmpty()) return 0;
-
         NodeResources free = freeParentCapacity.justNumbers().subtract(resources.justNumbers());
         return Node.skew(parent.get().flavor().resources(), free);
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
index 5dac2004931..7d15a2b30b1 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
@@ -109,7 +109,8 @@ public class NodeRepositoryProvisioner implements Provisioner {
         }
         else {
             groups = 1; // type request with multiple groups is not supported
-            resources = requested.minResources().nodeResources();
+            resources = requested.minResources().nodeResources().isUnspecified() ? capacityPolicies.defaultNodeResources(cluster.type())
+                                                                                 : requested.minResources().nodeResources();
             nodeSpec = NodeSpec.from(requested.type());
         }
         return asSortedHosts(preparer.prepare(application, cluster, nodeSpec, groups), resources);
@@ -163,12 +164,20 @@ public class NodeRepositoryProvisioner implements Provisioner {
         boolean firstDeployment = nodes.isEmpty();
         AllocatableClusterResources currentResources =
                 firstDeployment // start at min, preserve current resources otherwise
-                ? new AllocatableClusterResources(requested.minResources(), clusterSpec, nodeRepository)
+                ? new AllocatableClusterResources(initialResourcesFrom(requested, clusterSpec), clusterSpec, nodeRepository)
                 : new AllocatableClusterResources(nodes.asList(), nodeRepository);
         var clusterModel = new ClusterModel(application, cluster, clusterSpec, nodes, nodeRepository.metricsDb(), nodeRepository.clock());
         return within(Limits.of(requested), currentResources, firstDeployment, clusterModel);
     }
 
+    private ClusterResources initialResourcesFrom(Capacity requested, ClusterSpec clusterSpec) {
+        var initial = requested.minResources();
+        if (initial.nodeResources().isUnspecified())
+            initial = initial.with(capacityPolicies.defaultNodeResources(clusterSpec.type()));
+        return initial;
+    }
+
+
     /** Make the minimal adjustments needed to the current resources to stay within the limits */
     private ClusterResources within(Limits limits,
                                     AllocatableClusterResources current,
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
index 282b0d96cf4..b12368b2834 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
@@ -10,7 +10,6 @@ import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.NodesAndHosts;
-import com.yahoo.vespa.hosted.provision.node.Nodes;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -25,13 +24,11 @@ import java.util.stream.Collectors;
  */
 class Preparer {
 
-    private final NodeRepository nodeRepository;
     private final GroupPreparer groupPreparer;
     private final Optional<LoadBalancerProvisioner> loadBalancerProvisioner;
 
     public Preparer(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner,
                     Optional<LoadBalancerProvisioner> loadBalancerProvisioner) {
-        this.nodeRepository = nodeRepository;
         this.loadBalancerProvisioner = loadBalancerProvisioner;
         this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner);
     }
@@ -69,9 +66,10 @@ class Preparer {
 
         for (int groupIndex = 0; groupIndex < wantedGroups; groupIndex++) {
             ClusterSpec clusterGroup = cluster.with(Optional.of(ClusterSpec.Group.from(groupIndex)));
-            GroupPreparer.PrepareResult result = groupPreparer.prepare(
-                    application, clusterGroup, requestedNodes.fraction(wantedGroups),
-                    surplusNodes, indices, wantedGroups, allNodesAndHosts);
+            GroupPreparer.PrepareResult result = groupPreparer.prepare(application, clusterGroup,
+                                                                       requestedNodes.fraction(wantedGroups),
+                                                                       surplusNodes, indices, wantedGroups,
+                                                                       allNodesAndHosts);
             allNodesAndHosts = result.allNodesAndHosts; // Might have changed
             List<Node> accepted = result.prepared;
             if (requestedNodes.rejectNonActiveParent()) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
index 7ade2cdf8c4..1eebcff0903 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
@@ -779,6 +779,27 @@ public class AutoscalingTest {
                                tester.autoscale(application1, cluster1, capacity).target());
     }
 
+    @Test
+    public void test_autoscaling_in_dev_with_required_unspecified_resources() {
+        NodeResources resources = NodeResources.unspecified();
+        ClusterResources min = new ClusterResources( 1, 1, resources);
+        ClusterResources max = new ClusterResources(3, 1, resources);
+        Capacity capacity = Capacity.from(min, max, true, true);
+
+        AutoscalingTester tester = new AutoscalingTester(Environment.dev,
+                                                         new NodeResources(10, 16, 100, 2));
+        ApplicationId application1 = tester.applicationId("application1");
+        ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1");
+
+        tester.deploy(application1, cluster1, capacity);
+        tester.addQueryRateMeasurements(application1, cluster1.id(),
+                                        500, t -> 100.0);
+        tester.addCpuMeasurements(1.0f, 1f, 10, application1);
+        tester.assertResources("We scale up even in dev because resources are required",
+                               3, 1, 1.5,  8, 50,
+                               tester.autoscale(application1, cluster1, capacity).target());
+    }
+
     /**
      * This calculator subtracts the memory tax when forecasting overhead, but not when actually
      * returning information about nodes. This is allowed because the forecast is a *worst case*.
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java
index 6d5677d0911..ac5eec27a9d 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java
@@ -230,6 +230,22 @@ public class AutoscalingMaintainerTest {
                            .size());
     }
 
+    @Test
+    public void test_cd_test_not_specifying_node_resources() {
+        ApplicationId app1 = AutoscalingMaintainerTester.makeApplicationId("app1");
+        ClusterSpec cluster1 = AutoscalingMaintainerTester.containerClusterSpec();
+        ClusterResources resources = new ClusterResources( 2, 1, NodeResources.unspecified());
+        var capacity = Capacity.from(resources);
+        var tester = new AutoscalingMaintainerTester(new Zone(SystemName.cd, Environment.prod, RegionName.from("us-east3")),
+                                                     new MockDeployer.ApplicationContext(app1, cluster1, capacity));
+        tester.deploy(app1, cluster1, capacity); // Deploy should succeed and allocate the nodes
+        assertEquals(2,
+                     tester.nodeRepository().nodes().list(Node.State.active)
+                           .owner(app1)
+                           .cluster(cluster1.id())
+                           .size());
+    }
+
     private void autoscale(boolean down, Duration completionTime, Duration expectedWindow,
                            ManualClock clock, ApplicationId application, ClusterSpec cluster,
                            AutoscalingMaintainerTester tester) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
index 316655e11fb..7ce26354739 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
@@ -458,7 +458,7 @@ public class DynamicProvisioningMaintainerTest {
 
         // Provision config servers
         for (int i = 0; i < provisionedHosts.size(); i++) {
-            tester.makeReadyChildren(1, i + 1, NodeResources.unspecified(), hostType.childNodeType(),
+            tester.makeReadyChildren(1, i + 1, new NodeResources(1.5, 8, 50, 0.3), hostType.childNodeType(),
                     provisionedHosts.get(i).hostname(), (nodeIndex) -> "cfg" + nodeIndex);
         }
         tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
index d1ec1018023..c478840780f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
@@ -493,6 +493,7 @@ public class ProvisioningTester {
     public List<Node> makeReadyNodes(int n, Flavor flavor, Optional<TenantName> reservedTo, NodeType type, int ipAddressPoolSize, boolean dualStack) {
         List<Node> nodes = makeProvisionedNodes(n, flavor, reservedTo, type, ipAddressPoolSize, dualStack);
         nodes = nodeRepository.nodes().deallocate(nodes, Agent.system, getClass().getSimpleName());
+        nodes.forEach(node -> { if (node.resources().isUnspecified()) throw new IllegalArgumentException(); });
         return nodeRepository.nodes().setReady(nodes, Agent.system, getClass().getSimpleName());
     }