Merge pull request #29081 from vespa-engine/jonmv/actually-set-architecture-for-admin-nodes-from-flag

Actually set arch from flag for admin nodes

6 files changed, 85 insertions, 15 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
index 16affbd7b0e..c8f088509c5 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
@@ -13,6 +13,7 @@ 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.NodeResources.DiskSpeed;
 import com.yahoo.config.provision.ProvisionLogger;
 
 import java.util.ArrayList;
@@ -234,7 +235,7 @@ public class InMemoryProvisioner implements HostProvisioner {
     // Minimal capacity policies
     private NodeResources decideResources(NodeResources resources) {
         if (defaultNodeResources.isUnspecified()) return resources;
-        return resources.withUnspecifiedNumbersFrom(defaultNodeResources);
+        return resources.withUnspecifiedFieldsFrom(defaultNodeResources);
     }
 
     private List<HostSpec> allocateHostGroup(ClusterSpec clusterGroup, NodeResources requestedResourcesOrUnspecified,
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java
index c1296c6708e..a0c48200ff6 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java
@@ -278,7 +278,7 @@ public class NodeResources {
         return new NodeResources(vcpu, memoryGb, diskGb, bandwidthGbps, diskSpeed, storageType, architecture, gpuResources);
     }
 
-    public NodeResources withUnspecifiedNumbersFrom(NodeResources fullySpecified) {
+    public NodeResources withUnspecifiedFieldsFrom(NodeResources fullySpecified) {
         var resources = this;
         if (resources.vcpuIsUnspecified())
             resources = resources.withVcpu(fullySpecified.vcpu());
@@ -288,6 +288,13 @@ public class NodeResources {
             resources = resources.withDiskGb(fullySpecified.diskGb());
         if (resources.bandwidthGbpsIsUnspecified())
             resources = resources.withBandwidthGbps(fullySpecified.bandwidthGbps());
+        if (resources.diskSpeed() == DiskSpeed.any)
+            resources = resources.with(fullySpecified.diskSpeed());
+        if (resources.storageType() == StorageType.any)
+            resources = resources.with(fullySpecified.storageType());
+        if (resources.architecture() == Architecture.any)
+            resources = resources.with(fullySpecified.architecture());
+        assert fullySpecified.gpuResources() == GpuResources.zero : "Not handled";
         return resources;
     }
 
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java
index 27b659fe1c6..65ec070d744 100644
--- a/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/NodeResourcesTest.java
@@ -1,22 +1,27 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.provision;
 
+import com.yahoo.config.provision.NodeResources.Architecture;
+import com.yahoo.config.provision.NodeResources.DiskSpeed;
+import com.yahoo.config.provision.NodeResources.GpuResources;
+import com.yahoo.config.provision.NodeResources.StorageType;
 import org.junit.jupiter.api.Test;
 
 import java.util.function.Supplier;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author bratseth
  */
-public class NodeResourcesTest {
+class NodeResourcesTest {
 
     @Test
-    public void testCost() {
+    void testCost() {
         assertEquals(5.408, new NodeResources(32, 128, 1200, 1).cost(), 0.0001);
     }
 
@@ -81,7 +86,40 @@ public class NodeResourcesTest {
     }
 
     @Test
-    public void testJoiningResources() {
+    void testSpecifyFully() {
+        NodeResources empty = new NodeResources(0, 0, 0, 0).with(DiskSpeed.any);
+
+        assertEquals(0, empty.withUnspecifiedFieldsFrom(empty).vcpu());
+        assertEquals(3, empty.withUnspecifiedFieldsFrom(empty.withVcpu(3)).vcpu());
+        assertEquals(2, empty.withVcpu(2).withUnspecifiedFieldsFrom(empty.withVcpu(3)).vcpu());
+
+        assertEquals(0, empty.withUnspecifiedFieldsFrom(empty).memoryGb());
+        assertEquals(3, empty.withUnspecifiedFieldsFrom(empty.withMemoryGb(3)).memoryGb());
+        assertEquals(2, empty.withMemoryGb(2).withUnspecifiedFieldsFrom(empty.withMemoryGb(3)).memoryGb());
+
+        assertEquals(0, empty.withUnspecifiedFieldsFrom(empty).diskGb());
+        assertEquals(3, empty.withUnspecifiedFieldsFrom(empty.withDiskGb(3)).diskGb());
+        assertEquals(2, empty.withDiskGb(2).withUnspecifiedFieldsFrom(empty.withDiskGb(3)).diskGb());
+
+        assertEquals(0, empty.withUnspecifiedFieldsFrom(empty).bandwidthGbps());
+        assertEquals(3, empty.withUnspecifiedFieldsFrom(empty.withBandwidthGbps(3)).bandwidthGbps());
+        assertEquals(2, empty.withBandwidthGbps(2).withUnspecifiedFieldsFrom(empty.withBandwidthGbps(3)).bandwidthGbps());
+
+        assertEquals(Architecture.any, empty.withUnspecifiedFieldsFrom(empty).architecture());
+        assertEquals(Architecture.arm64, empty.withUnspecifiedFieldsFrom(empty.with(Architecture.arm64)).architecture());
+        assertEquals(Architecture.x86_64, empty.with(Architecture.x86_64).withUnspecifiedFieldsFrom(empty.with(Architecture.arm64)).architecture());
+
+        assertEquals(DiskSpeed.any, empty.withUnspecifiedFieldsFrom(empty).diskSpeed());
+        assertEquals(DiskSpeed.fast, empty.withUnspecifiedFieldsFrom(empty.with(DiskSpeed.fast)).diskSpeed());
+        assertEquals(DiskSpeed.slow, empty.with(DiskSpeed.slow).withUnspecifiedFieldsFrom(empty.with(DiskSpeed.fast)).diskSpeed());
+
+        assertEquals(StorageType.any, empty.withUnspecifiedFieldsFrom(empty).storageType());
+        assertEquals(StorageType.local, empty.withUnspecifiedFieldsFrom(empty.with(StorageType.local)).storageType());
+        assertEquals(StorageType.remote, empty.with(StorageType.remote).withUnspecifiedFieldsFrom(empty.with(StorageType.local)).storageType());
+    }
+
+    @Test
+    void testJoiningResources() {
         var resources = new NodeResources(1, 2, 3, 1,
                 NodeResources.DiskSpeed.fast,
                 NodeResources.StorageType.local,
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 f9ac7367778..1e9adea4e95 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
@@ -9,6 +9,7 @@ import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.NodeResources.DiskSpeed;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.StringFlag;
@@ -90,7 +91,10 @@ public class CapacityPolicies {
     }
 
     public NodeResources specifyFully(NodeResources resources, ClusterSpec clusterSpec, ApplicationId applicationId) {
-        return resources.withUnspecifiedNumbersFrom(defaultResources(clusterSpec, applicationId));
+        NodeResources amended = resources.withUnspecifiedFieldsFrom(defaultResources(clusterSpec, applicationId).with(DiskSpeed.any));
+        // TODO jonmv: remove this after all apps are 8.248.8 or above; architecture for admin nodes was not picked up before this.
+        if (clusterSpec.vespaVersion().isBefore(Version.fromString("8.248.8"))) amended = amended.with(resources.architecture());
+        return amended;
     }
 
     private NodeResources defaultResources(ClusterSpec clusterSpec, ApplicationId applicationId) {
@@ -149,7 +153,9 @@ public class CapacityPolicies {
         return Architecture.valueOf(adminClusterNodeArchitecture.with(INSTANCE_ID, instance.serializedForm()).value());
     }
 
-    /** Returns the resources for the newest version not newer than that requested in the cluster spec. */
+    /**
+     * Returns the resources for the newest version not newer than that requested in the cluster spec.
+     */
     static NodeResources versioned(ClusterSpec spec, Map<Version, NodeResources> resources) {
         return requireNonNull(new TreeMap<>(resources).floorEntry(spec.vespaVersion()),
                               "no default resources applicable for " + spec + " among: " + resources)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java
index 9204e3ddb0b..be2b2ca896a 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java
@@ -200,7 +200,7 @@ public class DynamicProvisioningTester {
                                             int nodeCount, int groupCount,
                                             double approxCpu, double approxMemory, double approxDisk,
                                             Autoscaling autoscaling) {
-        assertTrue("Resources are present: " + message + " (" + autoscaling + ": " + autoscaling.status() + ")",
+        assertTrue("Resources should be present: " + message + " (" + autoscaling + ": " + autoscaling.status() + ")",
                    autoscaling.resources().isPresent());
         var resources = autoscaling.resources().get();
         assertResources(message, nodeCount, groupCount, approxCpu, approxMemory, approxDisk, resources);
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 d25dfd44b1e..8ba9fbdf6d2 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
@@ -17,6 +17,9 @@ import com.yahoo.config.provision.HostFilter;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.NodeAllocationException;
 import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.NodeResources.Architecture;
+import com.yahoo.config.provision.NodeResources.DiskSpeed;
+import com.yahoo.config.provision.NodeResources.StorageType;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.ParentHostUnavailableException;
 import com.yahoo.config.provision.RegionName;
@@ -411,8 +414,8 @@ public class ProvisioningTest {
                                         tester);
             assertEquals(6, state.allHosts.size());
             tester.activate(application, state.allHosts);
-            assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == NodeResources.DiskSpeed.any));
-            assertTrue(tester.nodeRepository().nodes().list().owner(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == NodeResources.DiskSpeed.any));
+            assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == DiskSpeed.any));
+            assertTrue(tester.nodeRepository().nodes().list().owner(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == DiskSpeed.any));
         }
 
         {
@@ -423,8 +426,8 @@ public class ProvisioningTest {
                                         tester);
             assertEquals(8, state.allHosts.size());
             tester.activate(application, state.allHosts);
-            assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == NodeResources.DiskSpeed.fast));
-            assertTrue(tester.nodeRepository().nodes().list().owner(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == NodeResources.DiskSpeed.fast));
+            assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == DiskSpeed.fast));
+            assertTrue(tester.nodeRepository().nodes().list().owner(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == DiskSpeed.fast));
         }
 
         {
@@ -434,8 +437,8 @@ public class ProvisioningTest {
                                         tester);
             assertEquals(8, state.allHosts.size());
             tester.activate(application, state.allHosts);
-            assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == NodeResources.DiskSpeed.any));
-            assertTrue(tester.nodeRepository().nodes().list().owner(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == NodeResources.DiskSpeed.any));
+            assertTrue(state.allHosts.stream().allMatch(host -> host.requestedResources().get().diskSpeed() == DiskSpeed.any));
+            assertTrue(tester.nodeRepository().nodes().list().owner(application).stream().allMatch(node -> node.allocation().get().requestedResources().diskSpeed() == DiskSpeed.any));
         }
     }
 
@@ -1074,10 +1077,25 @@ public class ProvisioningTest {
     }
 
     @Test
+    public void no_arm64_architecture() {
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build();
+
+        NodeResources nodeResources = new NodeResources(1, 4, 10, 4, DiskSpeed.fast, StorageType.any, Architecture.x86_64);
+        tester.makeReadyHosts(4, nodeResources);
+        tester.activateTenantHosts();
+
+        ApplicationId application = ProvisioningTester.applicationId();
+        assertTrue(assertThrows(NodeAllocationException.class,
+                                () -> prepare(application, 1, 1, 1, 1, nodeResources.with(Architecture.arm64), tester))
+                           .getMessage().startsWith("Could not satisfy request"));
+
+    }
+
+    @Test
     public void arm64_architecture() {
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build();
 
-        NodeResources nodeResources = new NodeResources(1, 4, 10, 4, NodeResources.DiskSpeed.any, NodeResources.StorageType.any, NodeResources.Architecture.arm64);
+        NodeResources nodeResources = new NodeResources(1, 4, 10, 4, DiskSpeed.fast, StorageType.any, Architecture.arm64);
         tester.makeReadyHosts(4, nodeResources);
         tester.activateTenantHosts();