Merge pull request #27256 from vespa-engine/mpolden/prefer-latest-gen

Prefer latest generation with fallback to older

7 files changed, 120 insertions, 86 deletions

diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java
index ccce8c2f4b9..cfe740069db 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java
@@ -121,7 +121,7 @@ public class TestPackageTest {
     }
 
     @Test
-    void testTestPacakgeAssembly() throws IOException {
+    void testTestPackageAssembly() throws IOException {
         byte[] bundleZip = ApplicationPackage.filesZip(Map.of("components/foo-tests.jar", testsJar("SystemTest", "ProductionTest"),
                                                               "artifacts/key", new byte[0]));
         TestPackage bundleTests = new TestPackage(() -> new ByteArrayInputStream(bundleZip),
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java
index 1533780e694..3f9e8ef4407 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java
@@ -24,18 +24,17 @@ import com.yahoo.vespa.hosted.provision.NodeMutex;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.History;
+import com.yahoo.vespa.hosted.provision.provisioning.HostProvisionRequest;
 import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
 import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing;
 import com.yahoo.vespa.hosted.provision.provisioning.NodeCandidate;
 import com.yahoo.vespa.hosted.provision.provisioning.NodePrioritizer;
 import com.yahoo.vespa.hosted.provision.provisioning.NodeSpec;
-import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost;
 
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.EnumSet;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -212,9 +211,10 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer {
             Version osVersion = nodeRepository().osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion);
             List<Integer> provisionIndices = nodeRepository().database().readProvisionIndices(count);
             List<Node> hosts = new ArrayList<>();
-            hostProvisioner.provisionHosts(provisionIndices, NodeType.host, nodeResources, ApplicationId.defaultId(), osVersion,
-                                           HostSharing.shared, Optional.empty(), Optional.empty(),
-                                           nodeRepository().zone().cloud().account(),
+            HostProvisionRequest request = new HostProvisionRequest(provisionIndices, NodeType.host, nodeResources, ApplicationId.defaultId(), osVersion,
+                                                                    HostSharing.shared, Optional.empty(), Optional.empty(),
+                                                                    nodeRepository().zone().cloud().account(), false);
+            hostProvisioner.provisionHosts(request,
                                            provisionedHosts -> {
                                                hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(Duration.ZERO)).toList());
                                                nodeRepository().nodes().addNodes(hosts, Agent.HostCapacityMaintainer);
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 5ae5a4f09d9..3823f4906c2 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
@@ -99,7 +99,7 @@ public class GroupPreparer {
                 Version osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion);
                 NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get();
                 List<Node> hosts = new ArrayList<>();
-                Consumer<List<ProvisionedHost>> provisionedHostsConsumer = provisionedHosts -> {
+                Consumer<List<ProvisionedHost>> whenProvisioned = provisionedHosts -> {
                     hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(requestedNodes.hostTTL())).toList());
                     nodeRepository.nodes().addNodes(hosts, Agent.application);
 
@@ -110,12 +110,18 @@ public class GroupPreparer {
                             .toList();
                     allocation.offer(candidates);
                 };
-
                 try {
-                    hostProvisioner.get().provisionHosts(
-                            allocation.provisionIndices(deficit.count()), hostType, deficit.resources(), application,
-                            osVersion, sharing, Optional.of(cluster.type()), Optional.of(cluster.id()),
-                            requestedNodes.cloudAccount(), provisionedHostsConsumer);
+                    HostProvisionRequest request = new HostProvisionRequest(allocation.provisionIndices(deficit.count()),
+                                                                            hostType,
+                                                                            deficit.resources(),
+                                                                            application,
+                                                                            osVersion,
+                                                                            sharing,
+                                                                            Optional.of(cluster.type()),
+                                                                            Optional.of(cluster.id()),
+                                                                            requestedNodes.cloudAccount(),
+                                                                            false);
+                    hostProvisioner.get().provisionHosts(request, whenProvisioned);
                 } catch (NodeAllocationException e) {
                     // Mark the nodes that were written to ZK in the consumer for deprovisioning. While these hosts do
                     // not exist, we cannot remove them from ZK here because other nodes may already have been
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisionRequest.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisionRequest.java
new file mode 100644
index 00000000000..f7b9c9016b1
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisionRequest.java
@@ -0,0 +1,63 @@
+// Copyright Yahoo. 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.CloudAccount;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.NodeResources;
+import com.yahoo.config.provision.NodeType;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * A host provisioning request. This contains the details required to provision a host.
+ *
+ * @param indices                 List of unique provision indices which will be used to generate the node hostnames
+ *                                on the form of <code>[prefix][index].[domain]</code>.
+ * @param type                    The host type to provision.
+ * @param resources               The resources needed per node - the provisioned host may be significantly larger.
+ * @param owner                   ID of the application that will own the provisioned host.
+ * @param osVersion               The OS version to use. If this version does not exist, implementations may choose a suitable
+ *                                fallback version.
+ * @param sharing                 Puts requirements on sharing or exclusivity of the host to be provisioned.
+ * @param clusterType             The cluster we are provisioning for, or empty if we are provisioning hosts
+ *                                to be shared by multiple cluster nodes.
+ * @param clusterId               The ID of the cluster we are provisioning for, or empty if we are provisioning hosts
+ *                                to be shared by multiple cluster nodes.
+ * @param cloudAccount            The cloud account to use.
+ * @param requireLatestGeneration Whether to require the latest generation when choosing a flavor. Latest generation will
+ *                                always be preferred, but setting this to true disallows falling back to an older
+ *                                generation.
+ * @author mpolden
+ */
+public record HostProvisionRequest(List<Integer> indices,
+                                   NodeType type,
+                                   NodeResources resources,
+                                   ApplicationId owner,
+                                   Version osVersion,
+                                   HostProvisioner.HostSharing sharing,
+                                   Optional<ClusterSpec.Type> clusterType,
+                                   Optional<ClusterSpec.Id> clusterId,
+                                   CloudAccount cloudAccount,
+                                   boolean requireLatestGeneration) {
+
+    public HostProvisionRequest(List<Integer> indices, NodeType type, NodeResources resources,
+                                ApplicationId owner, Version osVersion, HostProvisioner.HostSharing sharing,
+                                Optional<ClusterSpec.Type> clusterType, Optional<ClusterSpec.Id> clusterId,
+                                CloudAccount cloudAccount, boolean requireLatestGeneration) {
+        this.indices = List.copyOf(Objects.requireNonNull(indices));
+        this.type = Objects.requireNonNull(type);
+        this.resources = Objects.requireNonNull(resources);
+        this.owner = Objects.requireNonNull(owner);
+        this.osVersion = Objects.requireNonNull(osVersion);
+        this.sharing = Objects.requireNonNull(sharing);
+        this.clusterType = Objects.requireNonNull(clusterType);
+        this.clusterId = Objects.requireNonNull(clusterId);
+        this.cloudAccount = Objects.requireNonNull(cloudAccount);
+        this.requireLatestGeneration = requireLatestGeneration;
+    }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
index 7c17c22334f..d678f4c8173 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java
@@ -1,19 +1,12 @@
 // Copyright Yahoo. 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.CloudAccount;
-import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.HostEvent;
 import com.yahoo.config.provision.NodeAllocationException;
-import com.yahoo.config.provision.NodeResources;
-import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.Node;
 
-import java.time.Duration;
 import java.util.List;
-import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
 
@@ -40,35 +33,14 @@ public interface HostProvisioner {
     /**
      * Schedule provisioning of a given number of hosts.
      *
-     * @param provisionIndices list of unique provision indices which will be used to generate the node hostnames
-     *                         on the form of <code>[prefix][index].[domain]</code>
-     * @param hostType the host type to provision
-     * @param resources the resources needed per node - the provisioned host may be significantly larger
-     * @param applicationId id of the application that will own the provisioned host
-     * @param osVersion the OS version to use. If this version does not exist, implementations may choose a suitable
-     *                  fallback version.
-     * @param sharing puts requirements on sharing or exclusivity of the host to be provisioned.
-     * @param clusterType the cluster we are provisioning for, or empty if we are provisioning hosts
-     *                    to be shared by multiple cluster nodes
-     * @param clusterId the id of the cluster we are provisioning for, or empty if we are provisioning hosts
-     *                    to be shared by multiple cluster nodes
-     * @param cloudAccount the cloud account to use
-     * @param provisionedHostConsumer consumer of {@link ProvisionedHost}s describing the provisioned nodes,
-     *                                the {@link Node} returned from {@link ProvisionedHost#generateHost} must be
-     *                                written to ZK immediately in case the config server goes down while waiting
-     *                                for the provisioning to finish.
+     * @param request         details of the host provision request.
+     * @param whenProvisioned consumer of {@link ProvisionedHost}s describing the provisioned nodes,
+     *                        the {@link Node} returned from {@link ProvisionedHost#generateHost} must be
+     *                        written to ZK immediately in case the config server goes down while waiting
+     *                        for the provisioning to finish.
      * @throws NodeAllocationException if the cloud provider cannot satisfy the request
      */
-    void provisionHosts(List<Integer> provisionIndices,
-                        NodeType hostType,
-                        NodeResources resources,
-                        ApplicationId applicationId,
-                        Version osVersion,
-                        HostSharing sharing,
-                        Optional<ClusterSpec.Type> clusterType,
-                        Optional<ClusterSpec.Id> clusterId,
-                        CloudAccount cloudAccount,
-                        Consumer<List<ProvisionedHost>> provisionedHostConsumer) throws NodeAllocationException;
+    void provisionHosts(HostProvisionRequest request, Consumer<List<ProvisionedHost>> whenProvisioned) throws NodeAllocationException;
 
     /**
      * Continue provisioning of given list of Nodes.
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 7f0d201b3e4..e6f2758ff7f 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
@@ -31,6 +31,7 @@ import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Used to manage a list of nodes during the node reservation process to fulfill the nodespec.
@@ -73,7 +74,7 @@ class NodeAllocation {
     /** The number of nodes that just now was changed to retired */
     private int wasRetiredJustNow = 0;
 
-    /** The node indexes to verify uniqueness of each members index */
+    /** The node indexes to verify uniqueness of each member's index */
     private final Set<Integer> indexes = new HashSet<>();
 
     /** The next membership index to assign to a new node */
@@ -93,11 +94,11 @@ class NodeAllocation {
         this.nodeRepository = nodeRepository;
         this.nodeResourceLimits = new NodeResourceLimits(nodeRepository);
         this.requiredHostFlavor = Optional.of(PermanentFlags.HOST_FLAVOR.bindTo(nodeRepository.flagSource())
-                        .with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm())
-                        .with(FetchVector.Dimension.CLUSTER_TYPE, cluster.type().name())
-                        .with(FetchVector.Dimension.CLUSTER_ID, cluster.id().value())
-                        .value())
-                .filter(s -> !s.isBlank());
+                                                                        .with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm())
+                                                                        .with(FetchVector.Dimension.CLUSTER_TYPE, cluster.type().name())
+                                                                        .with(FetchVector.Dimension.CLUSTER_ID, cluster.id().value())
+                                                                        .value())
+                                          .filter(s -> !s.isBlank());
     }
 
     /**
@@ -379,8 +380,8 @@ class NodeAllocation {
      * @return the final list of nodes
      */
     List<Node> finalNodes() {
-        int wantToRetireCount = (int) nodes.values().stream().filter(NodeCandidate::wantToRetire).count();
-        int currentRetiredCount = (int) nodes.values().stream().filter(node -> node.allocation().get().membership().retired()).count();
+        int wantToRetireCount = (int) matching(NodeCandidate::wantToRetire).count();
+        int currentRetiredCount = (int) matching(node -> node.allocation().get().membership().retired()).count();
         int deltaRetiredCount = requestedNodes.idealRetiredCount(nodes.size(), wantToRetireCount, currentRetiredCount);
 
         if (deltaRetiredCount > 0) { // retire until deltaRetiredCount is 0
@@ -415,24 +416,21 @@ class NodeAllocation {
             nodes.put(candidate.toNode().hostname(), candidate);
         }
 
-        return nodes.values().stream().map(n -> n.toNode()).toList();
+        return nodes.values().stream().map(NodeCandidate::toNode).toList();
     }
 
     List<Node> reservableNodes() {
         // Include already reserved nodes to extend reservation period and to potentially update their cluster spec.
         EnumSet<Node.State> reservableStates = EnumSet.of(Node.State.inactive, Node.State.ready, Node.State.reserved);
-        return nodesFilter(n -> ! n.isNew && reservableStates.contains(n.state()));
+        return matching(n -> ! n.isNew && reservableStates.contains(n.state())).toList();
     }
 
     List<Node> newNodes() {
-        return nodesFilter(n -> n.isNew);
+        return matching(node -> node.isNew).toList();
     }
 
-    private List<Node> nodesFilter(Predicate<NodeCandidate> predicate) {
-        return nodes.values().stream()
-                .filter(predicate)
-                .map(n -> n.toNode())
-                .toList();
+    private Stream<Node> matching(Predicate<NodeCandidate> predicate) {
+        return nodes.values().stream().filter(predicate).map(NodeCandidate::toNode);
     }
 
     /** Returns the number of nodes accepted this far */
@@ -487,8 +485,8 @@ class NodeAllocation {
         outsideRealLimits("node real resources is outside limits"),
         violatesParentHostPolicy("node violates parent host policy"),
         incompatibleResources("node resources are incompatible"),
-        hardRequest("node is requested to retire"),
-        softRequest("node is requested to retire (soft)"),
+        hardRequest("node is requested and required to retire"),
+        softRequest("node is requested to retire"),
         violatesExclusivity("node violates host exclusivity"),
         violatesHostFlavor("node violates host flavor"),
         none("");
@@ -499,7 +497,7 @@ class NodeAllocation {
             this.description = description;
         }
 
-        /** Human readable description of this cause */
+        /** Human-readable description of this cause */
         public String description() {
             return description;
         }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java
index 5e3cdaec216..779926a2c83 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java
@@ -1,8 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision.testutils;
 
-import com.yahoo.component.Version;
-import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Flavor;
@@ -16,11 +14,11 @@ import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.IP;
 import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException;
 import com.yahoo.vespa.hosted.provision.provisioning.HostIpConfig;
+import com.yahoo.vespa.hosted.provision.provisioning.HostProvisionRequest;
 import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
 import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator;
 import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost;
 
-import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -65,34 +63,31 @@ public class MockHostProvisioner implements HostProvisioner {
     }
 
     @Override
-    public void provisionHosts(List<Integer> provisionIndices, NodeType hostType, NodeResources resources,
-                               ApplicationId applicationId, Version osVersion, HostSharing sharing,
-                               Optional<ClusterSpec.Type> clusterType, Optional<ClusterSpec.Id> clusterId,
-                               CloudAccount cloudAccount, Consumer<List<ProvisionedHost>> provisionedHostsConsumer) {
-        Flavor hostFlavor = hostFlavors.get(clusterType.orElse(ClusterSpec.Type.content));
+    public void provisionHosts(HostProvisionRequest request, Consumer<List<ProvisionedHost>> whenProvisioned) {
+        Flavor hostFlavor = hostFlavors.get(request.clusterType().orElse(ClusterSpec.Type.content));
         if (hostFlavor == null)
             hostFlavor = flavors.stream()
-                                .filter(f -> sharing == HostSharing.exclusive ? compatible(f, resources)
-                                                                              : f.resources().satisfies(resources))
+                                .filter(f -> request.sharing() == HostSharing.exclusive ? compatible(f, request.resources())
+                                                                              : f.resources().satisfies(request.resources()))
                                 .findFirst()
-                                .orElseThrow(() -> new NodeAllocationException("No host flavor matches " + resources, true));
+                                .orElseThrow(() -> new NodeAllocationException("No host flavor matches " + request.resources(), true));
 
         List<ProvisionedHost> hosts = new ArrayList<>();
-        for (int index : provisionIndices) {
-            String hostHostname = hostType == NodeType.host ? "host" + index : hostType.name() + index;
-            hosts.add(new ProvisionedHost("id-of-" + hostType.name() + index,
+        for (int index : request.indices()) {
+            String hostHostname = request.type() == NodeType.host ? "host" + index : request.type().name() + index;
+            hosts.add(new ProvisionedHost("id-of-" + request.type().name() + index,
                                           hostHostname,
                                           hostFlavor,
-                                          hostType,
-                                          sharing == HostSharing.exclusive ? Optional.of(applicationId) : Optional.empty(),
+                                          request.type(),
+                                          request.sharing() == HostSharing.exclusive ? Optional.of(request.owner()) : Optional.empty(),
                                           Optional.empty(),
-                                          createHostnames(hostType, hostFlavor, index),
-                                          resources,
-                                          osVersion,
-                                          cloudAccount));
+                                          createHostnames(request.type(), hostFlavor, index),
+                                          request.resources(),
+                                          request.osVersion(),
+                                          request.cloudAccount()));
         }
         provisionedHosts.addAll(hosts);
-        provisionedHostsConsumer.accept(hosts);
+        whenProvisioned.accept(hosts);
     }
 
     @Override