Merge pull request #21746 from vespa-engine/hmusum/cleanup-and-more-testing-of-admin-cluster-architecture

Cleanup and more testing of specifying node architecture for admin cl…

5 files changed, 47 insertions, 22 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index e3446002888..91514b89b3e 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -24,6 +24,8 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 
+import static com.yahoo.config.provision.NodeResources.Architecture;
+
 /**
  * Model context containing state provided to model factories.
  *
@@ -118,7 +120,8 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"arnej"}) default boolean avoidRenamingSummaryFeatures() { return false; }
         @ModelFeatureFlag(owners = {"bjorncs", "baldersheim"}) default boolean mergeGroupingResultInSearchInvoker() { return false; }
         @ModelFeatureFlag(owners = {"arnej"}) default boolean experimentalSdParsing() { return false; }
-        @ModelFeatureFlag(owners = {"hmusum"}) default String adminClusterNodeArchitecture() { return "x86_64"; } // TODO: Cluster controllers only for now
+        @ModelFeatureFlag(owners = {"hmusum"}) default String adminClusterNodeArchitecture() { return adminClusterArchitecture().name(); } // TODO: Remove when 7.564 is oldest version in use
+        @ModelFeatureFlag(owners = {"hmusum"}) default Architecture adminClusterArchitecture() { return Architecture.getDefault(); }
     }
 
     /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
index 575080d3025..3e251a621ab 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
@@ -20,6 +20,8 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
+import static com.yahoo.config.provision.NodeResources.Architecture;
+
 /**
  * A test-only Properties class
  *
@@ -81,7 +83,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private List<String> environmentVariables = List.of();
     private boolean avoidRenamingSummaryFeatures = false;
     private boolean experimentalSdParsing = false;
-    private String adminClusterNodeResourcesArchitecture = "x86_64";
+    private Architecture adminClusterNodeResourcesArchitecture = Architecture.getDefault();
 
     @Override public ModelContext.FeatureFlags featureFlags() { return this; }
     @Override public boolean multitenant() { return multitenant; }
@@ -143,7 +145,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public List<String> environmentVariables() { return environmentVariables; }
     @Override public boolean avoidRenamingSummaryFeatures() { return this.avoidRenamingSummaryFeatures; }
     @Override public boolean experimentalSdParsing() { return this.experimentalSdParsing; }
-    @Override public String adminClusterNodeArchitecture() { return adminClusterNodeResourcesArchitecture; }
+    @Override public Architecture adminClusterArchitecture() { return adminClusterNodeResourcesArchitecture; }
 
     public TestProperties maxUnCommittedMemory(int maxUnCommittedMemory) {
         this.maxUnCommittedMemory = maxUnCommittedMemory;
@@ -391,8 +393,8 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
         return this;
     }
 
-    public TestProperties setAdminClusterNodeResourcesArchitecture(String architectureFunction) {
-        this.adminClusterNodeResourcesArchitecture = architectureFunction;
+    public TestProperties setAdminClusterNodeResourcesArchitecture(Architecture architecture) {
+        this.adminClusterNodeResourcesArchitecture = architecture;
         return this;
     }
 
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index 91aff3935ab..582ab18f904 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -48,6 +48,9 @@ import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import static com.yahoo.config.model.test.TestUtil.joinLines;
+import static com.yahoo.config.provision.NodeResources.Architecture;
+import static com.yahoo.config.provision.NodeResources.DiskSpeed;
+import static com.yahoo.config.provision.NodeResources.StorageType;
 import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 import static com.yahoo.vespa.model.search.NodeResourcesTuning.GB;
 import static com.yahoo.vespa.model.search.NodeResourcesTuning.reservedMemoryGb;
@@ -1474,10 +1477,12 @@ public class ModelProvisioningTest {
     }
 
     @Test
-    public void testUseArm64NodesForClusterControllers() {
+    public void testUseArm64NodesForAdminCluster() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>" +
                         "<services>" +
+                        "   <admin version='4.0'>" +
+                        "   </admin>" +
                         "   <container version='1.0' id='container'>" +
                         "      <nodes count='2'>" +
                         "         <resources vcpu='2' memory='8Gb' disk='30Gb'/>" +
@@ -1495,25 +1500,35 @@ public class ModelProvisioningTest {
 
         VespaModelTester tester = new VespaModelTester();
         tester.setHosted(true);
-        tester.setAdminClusterArchitecture("arm64");
+        tester.setAdminClusterArchitecture(Architecture.arm64);
+        tester.useDedicatedNodeForLogserver(true);
         tester.addHosts(new NodeResources(13.5, 100,         1000, 0.3), 6);
         tester.addHosts(new NodeResources(85,   200, 1000_000_000, 0.3), 20);
-        tester.addHosts(new NodeResources(0.5, 2, 10, 0.3, NodeResources.DiskSpeed.any, NodeResources.StorageType.any, NodeResources.Architecture.arm64), 3);
+        tester.addHosts(new NodeResources(0.5, 2, 10, 0.3, DiskSpeed.any, StorageType.any, Architecture.arm64), 4);
         VespaModel model = tester.createModel(services, true, true);
         List<HostResource> hosts = model.getRoot().hostSystem().getHosts();
-        assertEquals(7, hosts.size());
-        Set<HostResource> clusterControllerResources =
-                hosts.stream()
-                     .filter(host -> host.getHostInfo().getServices().stream()
-                                         .anyMatch(service -> service.getServiceType().equals("container-clustercontroller")))
-                     .collect(Collectors.toSet());
+        assertEquals(8, hosts.size());
+
+        Set<HostResource> clusterControllerResources = getHostResourcesForService(hosts, "container-clustercontroller");
         assertEquals(3, clusterControllerResources.size());
-        assertTrue(clusterControllerResources.stream().allMatch(host -> host.realResources().architecture().name().equals("arm64")));
+        assertTrue(clusterControllerResources.stream().allMatch(host -> host.realResources().architecture() == Architecture.arm64));
+
+        Set<HostResource> logserverResources = getHostResourcesForService(hosts, "logserver-container");
+        assertEquals(1, logserverResources.size());
+        assertTrue(logserverResources.stream().allMatch(host -> host.realResources().architecture() == Architecture.arm64));
 
-        // Other hosts should be x86_64
+        // Other hosts should be default
         assertTrue(hosts.stream()
                         .filter(host -> !clusterControllerResources.contains(host))
-                        .allMatch(host -> host.realResources().architecture().name().equals("x86_64")));
+                        .filter(host -> !logserverResources.contains(host))
+                        .allMatch(host -> host.realResources().architecture() == Architecture.getDefault()));
+    }
+
+    private Set<HostResource> getHostResourcesForService(List<HostResource> hosts, String service) {
+        return hosts.stream()
+                    .filter(host -> host.getHostInfo().getServices().stream()
+                                        .anyMatch(s -> s.getServiceType().equals(service)))
+                    .collect(Collectors.toSet());
     }
 
     @Test
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
index b9a5fde9301..41f9b4c7b55 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
@@ -30,6 +30,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
+import static com.yahoo.config.provision.NodeResources.Architecture;
 import static com.yahoo.vespa.model.test.utils.ApplicationPackageUtils.generateSchemas;
 
 /**
@@ -53,7 +54,7 @@ public class VespaModelTester {
     private final Map<NodeResources, Collection<Host>> hostsByResources = new HashMap<>();
     private ApplicationId applicationId = ApplicationId.defaultId();
     private boolean useDedicatedNodeForLogserver = false;
-    private String adminClusterArchitecture = "x86_64";
+    private Architecture adminClusterArchitecture = Architecture.getDefault();;
     private HostProvisioner provisioner;
 
     public VespaModelTester() {
@@ -102,7 +103,9 @@ public class VespaModelTester {
     public void setHosted(boolean hosted) { this.hosted = hosted; }
 
     /** Sets architecture to use for admin clusters. Default: x86_64 */
-    public void setAdminClusterArchitecture(String architecture) { this.adminClusterArchitecture = architecture; }
+    public void setAdminClusterArchitecture(Architecture architecture) {
+        this.adminClusterArchitecture = architecture;
+    }
 
     /** Sets the tenant, application name, and instance name of the model being built. */
     public void setApplicationId(String tenant, String applicationName, String instanceName) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index 1cfc7758def..718a25cc225 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -42,6 +42,7 @@ import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.function.ToIntFunction;
 
+import static com.yahoo.config.provision.NodeResources.Architecture;
 import static com.yahoo.vespa.config.server.ConfigServerSpec.fromConfig;
 import static com.yahoo.vespa.flags.FetchVector.Dimension.CLUSTER_TYPE;
 
@@ -211,7 +212,7 @@ public class ModelContextImpl implements ModelContext {
         private final boolean avoidRenamingSummaryFeatures;
         private final boolean mergeGroupingResultInSearchInvoker;
         private final boolean experimentalSdParsing;
-        private final String adminClusterNodeResourcesArchitecture;
+        private final Architecture adminClusterNodeResourcesArchitecture;
 
         public FeatureFlags(FlagSource source, ApplicationId appId, Version version) {
             this.defaultTermwiseLimit = flagValue(source, appId, version, Flags.DEFAULT_TERM_WISE_LIMIT);
@@ -261,7 +262,7 @@ public class ModelContextImpl implements ModelContext {
             this.avoidRenamingSummaryFeatures = flagValue(source, appId, version, Flags.AVOID_RENAMING_SUMMARY_FEATURES);
             this.mergeGroupingResultInSearchInvoker = flagValue(source, appId, version, Flags.MERGE_GROUPING_RESULT_IN_SEARCH_INVOKER);
             this.experimentalSdParsing = flagValue(source, appId, version, Flags.EXPERIMENTAL_SD_PARSING);
-            this.adminClusterNodeResourcesArchitecture = flagValue(source, appId, version, PermanentFlags.ADMIN_CLUSTER_NODE_ARCHITECTURE);
+            this.adminClusterNodeResourcesArchitecture = Architecture.valueOf(flagValue(source, appId, version, PermanentFlags.ADMIN_CLUSTER_NODE_ARCHITECTURE));
         }
 
         @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; }
@@ -313,7 +314,8 @@ public class ModelContextImpl implements ModelContext {
         @Override public boolean avoidRenamingSummaryFeatures() { return avoidRenamingSummaryFeatures; }
         @Override public boolean mergeGroupingResultInSearchInvoker() { return mergeGroupingResultInSearchInvoker; }
         @Override public boolean experimentalSdParsing() { return experimentalSdParsing; }
-        @Override public String adminClusterNodeArchitecture() { return adminClusterNodeResourcesArchitecture; }
+        @Override public String adminClusterNodeArchitecture() { return adminClusterArchitecture().name(); } // TODO: Remove when 7.564 is oldest version in use
+        @Override public Architecture adminClusterArchitecture() { return adminClusterNodeResourcesArchitecture; }
 
         private static <V> V flagValue(FlagSource source, ApplicationId appId, Version vespaVersion, UnboundFlag<? extends V, ?, ?> flag) {
             return flag.bindTo(source)