Merge pull request #515 from yahoo/bratseth/group-index-not-id

Model group ids as indexes

7 files changed, 16 insertions, 91 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 b4488cef385..69054cb7ae6 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
@@ -114,7 +114,7 @@ public class InMemoryProvisioner implements HostProvisioner {
         }
         else {
             for (int i = 0; i < groups; i++) {
-                allocation.addAll(allocateHostGroup(cluster.changeGroup(Optional.of(ClusterSpec.Group.from(String.valueOf(i)))),
+                allocation.addAll(allocateHostGroup(cluster.changeGroup(Optional.of(ClusterSpec.Group.from(i))),
                                                     flavor,
                                                     capacity / groups,
                                                     allocation.size()));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
index 1144ea775ab..c0baa50b3b0 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
@@ -71,7 +71,7 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
     }
 
     private Collection<HostResource> allocateHosts(HostSystem hostSystem, String clusterId, NodesSpecification nodesSpecification) {
-        return nodesSpecification.provision(hostSystem, ClusterSpec.Type.admin, ClusterSpec.Id.from(clusterId), Optional.empty(), deployLogger).keySet();
+        return nodesSpecification.provision(hostSystem, ClusterSpec.Type.admin, ClusterSpec.Id.from(clusterId), deployLogger).keySet();
     }
 
     /**
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
index 84a9b5f7e88..f22be501ff2 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java
@@ -93,10 +93,8 @@ public class NodesSpecification {
     /** Returns the number of host groups this specifies. Default is 1 */
     public int groups() { return groups; }
 
-    public Map<HostResource, ClusterMembership> provision(HostSystem hostSystem, ClusterSpec.Type clusterType, ClusterSpec.Id clusterId, Optional<ClusterSpec.Group> clusterGroup, DeployLogger logger) {
-        if (clusterGroup.isPresent() && groups > 1)
-            throw new IllegalArgumentException("Cannot both specify a group and request multiple groups");
-        ClusterSpec cluster = ClusterSpec.from(clusterType, clusterId, clusterGroup, dockerImage);
+    public Map<HostResource, ClusterMembership> provision(HostSystem hostSystem, ClusterSpec.Type clusterType, ClusterSpec.Id clusterId, DeployLogger logger) {
+        ClusterSpec cluster = ClusterSpec.request(clusterType, clusterId, dockerImage);
         return hostSystem.allocateHosts(cluster, Capacity.fromNodeCount(count, flavor), groups, logger);
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index b62f48d452a..8c4a451bb13 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -418,7 +418,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
     
     private HostResource allocateSingleNodeHost(ContainerCluster cluster, DeployLogger logger) {
         if (cluster.isHostedVespa()) {
-            ClusterSpec clusterSpec = ClusterSpec.from(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName()), Optional.empty());
+            ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName()), Optional.empty());
             return cluster.getHostSystem().allocateHosts(clusterSpec, Capacity.fromNodeCount(1), 1, logger).keySet().iterator().next();
         } else {
             return cluster.getHostSystem().getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC);
@@ -429,7 +429,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         NodesSpecification nodesSpecification = NodesSpecification.from(new ModelElement(nodesElement));
         Map<HostResource, ClusterMembership> hosts = nodesSpecification.provision(cluster.getRoot().getHostSystem(),
                                                                                   ClusterSpec.Type.container,
-                                                                                  ClusterSpec.Id.from(cluster.getName()), Optional.empty(), log);
+                                                                                  ClusterSpec.Id.from(cluster.getName()), 
+                                                                                  log);
         return createNodesFromHosts(hosts, cluster);
     }
     
@@ -452,7 +453,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
                 StorageGroup.provisionHosts(NodesSpecification.from(new ModelElement(referencedNodesElement)), 
                                             referenceId, 
                                             cluster.getRoot().getHostSystem(),
-                                            null,
                                             context.getDeployLogger());
         return createNodesFromHosts(hosts, cluster);
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
index 529abda25ef..501b2349035 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
@@ -158,11 +158,9 @@ public class StorageGroup {
     public static Map<HostResource, ClusterMembership> provisionHosts(NodesSpecification nodesSpecification, 
                                                                       String clusterIdString, 
                                                                       HostSystem hostSystem, 
-                                                                      String groupIndex, 
                                                                       DeployLogger logger) {
         ClusterSpec.Id clusterId = ClusterSpec.Id.from(clusterIdString);
-        Optional<ClusterSpec.Group> groupId = groupIndex == null ? Optional.empty() : Optional.of(ClusterSpec.Group.from(groupIndex));
-        return nodesSpecification.provision(hostSystem, ClusterSpec.Type.content, clusterId, groupId, logger);
+        return nodesSpecification.provision(hostSystem, ClusterSpec.Type.content, clusterId, logger);
     }
 
     public static class Builder {
@@ -248,9 +246,11 @@ public class StorageGroup {
              * @return the storage group build by this
              */
             public StorageGroup buildHosted(ContentCluster owner, Optional<GroupBuilder> parent) {
+                if (storageGroup.getIndex() != null)
+                    throw new IllegalArgumentException("Specifying individual groups is not supported on hosted applications");
                 Map<HostResource, ClusterMembership> hostMapping =
                         nodeRequirement.isPresent() ?
-                        provisionHosts(nodeRequirement.get(), owner.getStorageNodes().getClusterName(), owner.getRoot().getHostSystem(), storageGroup.getIndex(), deployLogger) :
+                        provisionHosts(nodeRequirement.get(), owner.getStorageNodes().getClusterName(), owner.getRoot().getHostSystem(), deployLogger) :
                         Collections.emptyMap();
 
                 Map<Optional<ClusterSpec.Group>, Map<HostResource, ClusterMembership>> hostGroups = collectAllocatedSubgroups(hostMapping);
@@ -268,7 +268,7 @@ public class StorageGroup {
 
                     // create subgroups as returned from allocation
                     for (Map.Entry<Optional<ClusterSpec.Group>, Map<HostResource, ClusterMembership>> hostGroup : hostGroups.entrySet()) {
-                        String groupIndex = hostGroup.getKey().get().value();
+                        String groupIndex = String.valueOf(hostGroup.getKey().get().index());
                         StorageGroup subgroup = new StorageGroup(owner, groupIndex, groupIndex, Optional.empty(), false, Optional.empty());
                         for (Map.Entry<HostResource, ClusterMembership> host : hostGroup.getValue().entrySet()) {
                             subgroup.nodes.add(createStorageNode(owner, host.getKey(), subgroup, host.getValue()));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 80f0791d5d8..fa417b34844 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -315,13 +315,13 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
         }
 
         private Collection<HostResource> getControllerHosts(NodesSpecification nodesSpecification, Admin admin, String clusterName) {
-            return nodesSpecification.provision(admin.getHostSystem(), ClusterSpec.Type.admin, ClusterSpec.Id.from(clusterName), Optional.empty(), deployLogger).keySet();
+            return nodesSpecification.provision(admin.getHostSystem(), ClusterSpec.Type.admin, ClusterSpec.Id.from(clusterName), deployLogger).keySet();
         }
 
         private List<HostResource> drawControllerHosts(int count, StorageGroup rootGroup, Collection<ContainerModel> containers) {
             List<HostResource> hosts = drawContentHostsRecursively(count, rootGroup);
-//            if (hosts.size() < count) // supply with containers
-//                hosts.addAll(drawContainerHosts(count - hosts.size(), containers, new HashSet<>(hosts)));
+            // if (hosts.size() < count) // supply with containers TODO: Currently disabled due to leading to topology change problems
+            //     hosts.addAll(drawContainerHosts(count - hosts.size(), containers, new HashSet<>(hosts)));
             if (hosts.size() % 2 == 0) // ZK clusters of even sizes are less available (even in the size=2 case)
                 hosts = hosts.subList(0, hosts.size()-1);
             return hosts;
@@ -334,6 +334,7 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
          * This will draw the same nodes each time it is 
          * invoked if cluster names and node indexes are unchanged.
          */
+        // DO NOT DELETE - see above
         private List<HostResource> drawContainerHosts(int count, Collection<ContainerModel> containerClusters, 
                                                       Set<HostResource> usedHosts) {
             if (containerClusters.isEmpty()) return Collections.emptyList();
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 38702d6920f..34f75d46a1c 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
@@ -278,80 +278,6 @@ public class ModelProvisioningTest {
     }
 
     @Test
-    public void testNodeCountForContentGroupHierarchy() {
-        String services = 
-                "<?xml version='1.0' encoding='utf-8' ?>\n" +
-                        "<services>\n" +
-                        "\n" +
-                        "  <admin version='3.0'>\n" +
-                        "    <nodes count='3'/>" + // Ignored
-                        "  </admin>\n" +
-                        "  <content version='1.0' id='bar'>" +
-                        "    <redundancy>2</redundancy>\n" +
-                        "    <documents>" +
-                        "      <document type='type1' mode='index'/>" +
-                        "    </documents>" +
-                        "    <group>" +
-                        "      <distribution partitions=\"1|*\"/>" +
-                        "      <group name='0' distribution-key='0'>" +
-                        "        <nodes count='2'/> " +
-                        "      </group>" +
-                        "      <group name='1' distribution-key='1'>" +
-                        "        <nodes count='2'/> " +
-                        "      </group>" +
-                        "    </group>" +
-                        "  </content>" +
-                        "  <content version='1.0' id='baz'>" +
-                        "    <redundancy>2</redundancy>\n" +
-                        "    <documents>" +
-                        "      <document type='type1' mode='index'/>" +
-                        "    </documents>" +
-                        "    <group>" +
-                        "      <distribution partitions=\"1|*\"/>" +
-                        "      <group name='0' distribution-key='10'>" +
-                        "        <nodes count='1'/> " +
-                        "      </group>" +
-                        "      <group name='1' distribution-key='11'>" +
-                        "        <nodes count='1'/> " +
-                        "      </group>" +
-                        "    </group>" +
-                        "  </content>" +
-                        "\n" +
-                        "</services>";
-        
-        int numberOfHosts = 6;
-        VespaModelTester tester = new VespaModelTester();
-        tester.addHosts(numberOfHosts);
-        VespaModel model = tester.createModel(services, true);
-        assertThat(model.getRoot().getHostSystem().getHosts().size(), is(numberOfHosts));
-        
-        ContentCluster cluster = model.getContentClusters().get("bar");
-        assertThat(cluster.getRootGroup().getNodes().size(), is(0));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getIndex(), is("0"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().size(), is(2));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().get(0).getDistributionKey(), is(0));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().get(0).getConfigId(), is("bar/storage/0"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().get(1).getDistributionKey(), is(1));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().get(1).getConfigId(), is("bar/storage/1"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getIndex(), is("1"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().size(), is(2));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().get(0).getDistributionKey(), is(2));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().get(0).getConfigId(), is("bar/storage/2"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().get(1).getDistributionKey(), is(3));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().get(1).getConfigId(), is("bar/storage/3"));
-
-        cluster = model.getContentClusters().get("baz");
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getIndex(), is("10"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().size(), is(1));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().get(0).getDistributionKey(), is(0));
-        assertThat(cluster.getRootGroup().getSubgroups().get(0).getNodes().get(0).getConfigId(), is("baz/storage/0"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getIndex(), is("11"));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().size(), is(1));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().get(0).getDistributionKey(), is(1));
-        assertThat(cluster.getRootGroup().getSubgroups().get(1).getNodes().get(0).getConfigId(), is("baz/storage/1"));
-    }
-
-    @Test
     public void testUsingNodesAndGroupCountAttributes() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>\n" +