Pass provision indexes to host provisioner

4 files changed, 35 insertions, 18 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index de8fb703462..2f100b00982 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -15,6 +15,7 @@ import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.transaction.Transaction;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.Lock;
+import com.yahoo.vespa.curator.recipes.CuratorCounter;
 import com.yahoo.vespa.curator.transaction.CuratorOperations;
 import com.yahoo.vespa.curator.transaction.CuratorTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
@@ -41,6 +42,7 @@ import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.toMap;
@@ -69,12 +71,14 @@ public class CuratorDatabaseClient {
     private final CuratorDatabase curatorDatabase;
     private final Clock clock;
     private final Zone zone;
+    private final CuratorCounter provisionIndexCounter;
 
     public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, Zone zone, boolean useCache) {
         this.nodeSerializer = new NodeSerializer(flavors);
         this.zone = zone;
         this.curatorDatabase = new CuratorDatabase(curator, root, useCache);
         this.clock = clock;
+        this.provisionIndexCounter = new CuratorCounter(curator, root.append("provisionIndexCounter").getAbsolute());
         initZK();
     }
 
@@ -92,6 +96,7 @@ public class CuratorDatabaseClient {
         curatorDatabase.create(firmwareCheckPath());
         curatorDatabase.create(loadBalancersRoot);
         curatorDatabase.create(flagsRoot);
+        provisionIndexCounter.initialize(100);
     }
 
     /**
@@ -515,4 +520,14 @@ public class CuratorDatabaseClient {
         return CuratorOperations.create(path.getAbsolute(), data);
     }
 
+    /** Returns a given number of unique provision indexes */
+    public List<Integer> getProvisionIndexes(int numIndexes) {
+        if (numIndexes < 1)
+            throw new IllegalArgumentException("numIndexes must be a positive integer, was " + numIndexes);
+
+        int firstProvisionIndex = (int) provisionIndexCounter.add(numIndexes) - numIndexes;
+        return IntStream.range(0, numIndexes)
+                .mapToObj(i -> firstProvisionIndex + i)
+                .collect(Collectors.toList());
+    }
 }
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 6ff683c1d7e..a8b48751d23 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
@@ -77,8 +77,11 @@ public class GroupPreparer {
                 allocation.offer(prioritizer.prioritize());
 
                 if (dynamicProvisioningEnabled) {
-                    List<ProvisionedHost> provisionedHosts = allocation.getFulfilledDockerDeficit().map(deficit ->
-                            hostProvisioner.get().provisionHosts(deficit.getCount(), deficit.getFlavor())).orElseGet(List::of);
+                    List<ProvisionedHost> provisionedHosts = allocation.getFulfilledDockerDeficit()
+                            .map(deficit -> hostProvisioner.get().provisionHosts(
+                                    nodeRepository.database().getProvisionIndexes(deficit.getCount()),
+                                    deficit.getFlavor()))
+                            .orElseGet(List::of);
 
                     // At this point we have started provisioning of the hosts, the first priority is to make sure that
                     // the returned hosts are added to the node-repo so that they are tracked by the provision maintainers
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 34fbaf94478..79296e58045 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
@@ -17,12 +17,13 @@ public interface HostProvisioner {
     /**
      * Schedule provisioning of a given number of hosts.
      *
-     * @param numHosts number of hosts to provision
+     * @param provisionIndexes List of unique provision indexes which will be used to generate the host hostnames
+     *                         on the form of <code>[prefix][index].[domain]</code>
      * @param nodeFlavor Vespa flavor of the node that will run on this host. The resulting provisioned host
      *                   will be of a flavor that is at least as big or bigger than this.
      * @return list of {@link ProvisionedHost} describing the provisioned hosts and nodes on them.
      */
-    List<ProvisionedHost> provisionHosts(int numHosts, Flavor nodeFlavor);
+    List<ProvisionedHost> provisionHosts(List<Integer> provisionIndexes, Flavor nodeFlavor);
 
     /**
      * Continue provisioning of given list of Nodes.
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java
index 38352e033dd..60e9289b9bf 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java
@@ -20,11 +20,9 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
-import java.util.stream.IntStream;
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyInt;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -50,13 +48,13 @@ public class DynamicDockerProvisionTest {
 
         mockHostProvisioner(hostProvisioner, tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("small"));
         List<HostSpec> hostSpec = tester.prepare(application1, clusterSpec("myContent.t1.a1"), 4, 1, flavor.canonicalName());
-        verify(hostProvisioner).provisionHosts(4, flavor);
+        verify(hostProvisioner).provisionHosts(List.of(100, 101, 102, 103), flavor);
 
         // Total of 8 nodes should now be in node-repo, 4 hosts in state provisioned, and 4 reserved nodes
         assertEquals(8, tester.nodeRepository().list().size());
         assertEquals(4, tester.nodeRepository().getNodes(NodeType.host, Node.State.provisioned).size());
         assertEquals(4, tester.nodeRepository().getNodes(NodeType.tenant, Node.State.reserved).size());
-        assertEquals(List.of("host-1-1", "host-2-1", "host-3-1", "host-4-1"),
+        assertEquals(List.of("host-100-1", "host-101-1", "host-102-1", "host-103-1"),
                 hostSpec.stream().map(HostSpec::hostname).collect(Collectors.toList()));
     }
 
@@ -70,7 +68,7 @@ public class DynamicDockerProvisionTest {
 
         mockHostProvisioner(hostProvisioner, tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("small"));
         tester.prepare(application, clusterSpec("myContent.t2.a2"), 2, 1, flavor.canonicalName());
-        verify(hostProvisioner).provisionHosts(2, flavor);
+        verify(hostProvisioner).provisionHosts(List.of(100, 101), flavor);
     }
 
     @Test
@@ -78,12 +76,13 @@ public class DynamicDockerProvisionTest {
         ApplicationId application = tester.makeApplicationId();
         Flavor flavor = tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("dockerSmall");
 
+        List<Integer> expectedProvisionIndexes = List.of(100, 101);
         mockHostProvisioner(hostProvisioner, tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("large"));
         tester.prepare(application, clusterSpec("myContent.t2.a2"), 2, 1, flavor.canonicalName());
-        verify(hostProvisioner).provisionHosts(2, flavor);
+        verify(hostProvisioner).provisionHosts(expectedProvisionIndexes, flavor);
 
         // Ready the provisioned hosts, add an IP addreses to pool and activate them
-        for (int i = 1; i < 3; i++) {
+        for (Integer i : expectedProvisionIndexes) {
             String hostname = "host-" + i;
             Node host = tester.nodeRepository().getNode(hostname).orElseThrow()
                     .withIpAddressPool(Set.of("::" + i + ":2")).withIpAddresses(Set.of("::" + i + ":0"));
@@ -95,7 +94,7 @@ public class DynamicDockerProvisionTest {
         mockHostProvisioner(hostProvisioner, tester.nodeRepository().getAvailableFlavors().getFlavorOrThrow("small"));
         tester.prepare(application, clusterSpec("another-id"), 2, 1, flavor.canonicalName());
         // Verify there was only 1 call to provision hosts (during the first prepare)
-        verify(hostProvisioner).provisionHosts(anyInt(), any());
+        verify(hostProvisioner).provisionHosts(any(), any());
 
         // Node-repo should now consist of 2 active hosts with 2 reserved nodes on each
         assertEquals(6, tester.nodeRepository().list().size());
@@ -121,15 +120,14 @@ public class DynamicDockerProvisionTest {
         return ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from(clusterId), Version.fromString("6.42"), false, Collections.emptySet());
     }
 
+    @SuppressWarnings("unchecked")
     private static void mockHostProvisioner(HostProvisioner hostProvisioner, Flavor hostFlavor) {
-        final int[] numProvisioned = { 0 };
         doAnswer(invocation -> {
-            int numHosts = (int) invocation.getArguments()[0];
+            List<Integer> provisionIndexes = (List<Integer>) invocation.getArguments()[0];
             Flavor nodeFlavor = (Flavor) invocation.getArguments()[1];
-            return IntStream.range(0, numHosts)
-                    .map(i -> ++numProvisioned[0])
-                    .mapToObj(i -> new ProvisionedHost("id-" + i, "host-" + i, hostFlavor, "host-" + i + "-1", nodeFlavor))
+            return provisionIndexes.stream()
+                    .map(i -> new ProvisionedHost("id-" + i, "host-" + i, hostFlavor, "host-" + i + "-1", nodeFlavor))
                     .collect(Collectors.toList());
-        }).when(hostProvisioner).provisionHosts(anyInt(), any());
+        }).when(hostProvisioner).provisionHosts(any(), any());
     }
 }