Revert "Missing node specification allocates one node per cluster"

2 files changed, 59 insertions, 16 deletions

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 14e667aba00..3166938cacd 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
@@ -535,7 +535,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
     private void addNodesFromXml(ApplicationContainerCluster cluster, Element containerElement, ConfigModelContext context) {
         Element nodesElement = XML.getChild(containerElement, "nodes");
-        if (nodesElement == null) {
+        if (nodesElement == null) { // default single node on localhost
             ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa());
             HostResource host = allocateSingleNodeHost(cluster, log, containerElement, context);
             node.setHostResource(host);
@@ -621,18 +621,24 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
     private HostResource allocateSingleNodeHost(ApplicationContainerCluster cluster, DeployLogger logger, Element containerElement, ConfigModelContext context) {
         DeployState deployState = context.getDeployState();
         HostSystem hostSystem = cluster.hostSystem();
-        if (deployState.isHosted()) { // request 1 node
-            ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container,
-                                                          ClusterSpec.Id.from(cluster.getName()),
-                                                          deployState.getWantedNodeVespaVersion(),
-                                                          false,
-                                                          Optional.empty(),
-                                                          deployState.getWantedDockerImageRepo());
-            Capacity capacity = Capacity.fromCount(1,
-                                                   Optional.empty(),
-                                                   false,
-                                                   !deployState.getProperties().isBootstrap());
-            return hostSystem.allocateHosts(clusterSpec, capacity, 1, logger).keySet().iterator().next();
+        if (deployState.isHosted()) {
+            Optional<HostResource> singleContentHost = getHostResourceFromContentClusters(cluster, containerElement, context);
+            if (singleContentHost.isPresent()) { // there is a content cluster; put the container on its first node 
+                return singleContentHost.get();
+            }
+            else { // request 1 node
+                ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container,
+                                                              ClusterSpec.Id.from(cluster.getName()),
+                                                              deployState.getWantedNodeVespaVersion(),
+                                                              false,
+                                                              Optional.empty(),
+                                                              deployState.getWantedDockerImageRepo());
+                Capacity capacity = Capacity.fromCount(1,
+                                                       Optional.empty(),
+                                                       false,
+                                                       ! deployState.getProperties().isBootstrap());
+                return hostSystem.allocateHosts(clusterSpec, capacity, 1, logger).keySet().iterator().next();
+            }
         } else {
             return hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC);
         }
@@ -679,6 +685,43 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return createNodesFromHosts(context.getDeployLogger(), hosts, cluster);
     }
 
+    /**
+     * This is used in case we are on hosted Vespa and no nodes tag is supplied:
+     * If there are content clusters this will pick the first host in the first cluster as the container node.
+     * If there are no content clusters this will return empty (such that the node can be created by the container here).
+     */
+    private Optional<HostResource> getHostResourceFromContentClusters(ApplicationContainerCluster cluster, Element containersElement, ConfigModelContext context) {
+        Optional<Element> services = servicesRootOf(containersElement);
+        if ( ! services.isPresent())
+            return Optional.empty();
+        List<Element> contentServices = XML.getChildren(services.get(), "content");
+        if ( contentServices.isEmpty() ) return Optional.empty();
+        Element contentNodesElementOrNull = XML.getChild(contentServices.get(0), "nodes");
+        
+        NodesSpecification nodesSpec;
+        if (contentNodesElementOrNull == null)
+            nodesSpec = NodesSpecification.nonDedicated(1, context);
+        else
+            nodesSpec = NodesSpecification.from(new ModelElement(contentNodesElementOrNull), context);
+
+        Map<HostResource, ClusterMembership> hosts =
+                StorageGroup.provisionHosts(nodesSpec,
+                                            contentServices.get(0).getAttribute("id"),
+                                            cluster.getRoot().hostSystem(),
+                                            context.getDeployLogger());
+        return Optional.of(hosts.keySet().iterator().next());
+    }
+
+    /** Returns the services element above the given Element, or empty if there is no services element */
+    private Optional<Element> servicesRootOf(Element element) {
+        Node parent = element.getParentNode();
+        if (parent == null) return Optional.empty();
+        if ( ! (parent instanceof Element)) return Optional.empty();
+        Element parentElement = (Element)parent;
+        if (parentElement.getTagName().equals("services")) return Optional.of(parentElement);
+        return servicesRootOf(parentElement);
+    }
+    
     private List<ApplicationContainer> createNodesFromHosts(DeployLogger deployLogger, Map<HostResource, ClusterMembership> hosts, ApplicationContainerCluster cluster) {
         List<ApplicationContainer> nodes = new ArrayList<>();
         for (Map.Entry<HostResource, ClusterMembership> entry : hosts.entrySet()) {
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 f303ea9a42d..d215fdbb7a0 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
@@ -1374,7 +1374,7 @@ public class ModelProvisioningTest {
     }
 
     @Test
-    public void testNoNodeTagMeans1NodePerCluster() {
+    public void testNoNodeTagMeans1Node() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>\n" +
                 "<services>" +
@@ -1389,9 +1389,9 @@ public class ModelProvisioningTest {
                 "  </content>" +
                 "</services>";
         VespaModelTester tester = new VespaModelTester();
-        tester.addHosts(2);
+        tester.addHosts(1);
         VespaModel model = tester.createModel(services, true);
-        assertEquals(2, model.getRoot().hostSystem().getHosts().size());
+        assertEquals(1, model.getRoot().hostSystem().getHosts().size());
         assertEquals(1, model.getAdmin().getSlobroks().size());
         assertEquals(1, model.getContainerClusters().get("foo").getContainers().size());
         assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes());