Merge pull request #11644 from vespa-engine/mpolden/lb-combined-cluster

Provision load balancer for combined cluster

4 files changed, 107 insertions, 36 deletions

diff --git a/config-model/.gitignore b/config-model/.gitignore
index 6adbe26187e..b0f358e8113 100644
--- a/config-model/.gitignore
+++ b/config-model/.gitignore
@@ -2,3 +2,5 @@
 /tmp
 /temp
 /target
+/src/test/integration/*/copy/
+/src/test/integration/*/models.generated/
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 54850dedbba..f221b1974d7 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
@@ -8,8 +8,11 @@ import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.NodeResources;
+import com.yahoo.text.XML;
 import com.yahoo.vespa.model.HostResource;
 import com.yahoo.vespa.model.HostSystem;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
 
 import java.util.Map;
 import java.util.Optional;
@@ -41,6 +44,11 @@ public class NodesSpecification {
     
     private final boolean exclusive;
 
+    /**
+     * Whether this requires running container and content processes co-located on the same node.
+     */
+    private final boolean combined;
+
     /** The resources each node should have, or empty to use the default */
     private final Optional<NodeResources> resources;
 
@@ -48,7 +56,7 @@ public class NodesSpecification {
     private final Optional<String> dockerImage;
 
     private NodesSpecification(boolean dedicated, int count, int groups, Version version,
-                               boolean required, boolean canFail, boolean exclusive,
+                               boolean required, boolean canFail, boolean exclusive, boolean combined,
                                Optional<NodeResources> resources, Optional<String> dockerImage) {
         this.dedicated = dedicated;
         this.count = count;
@@ -59,9 +67,10 @@ public class NodesSpecification {
         this.exclusive = exclusive;
         this.resources = resources;
         this.dockerImage = dockerImage;
+        this.combined = combined;
     }
 
-    private NodesSpecification(boolean dedicated, boolean canFail, Version version, ModelElement nodesElement) {
+    private NodesSpecification(boolean dedicated, boolean canFail, boolean combined, Version version, ModelElement nodesElement) {
         this(dedicated,
              nodesElement.integerAttribute("count", 1),
              nodesElement.integerAttribute("groups", 1),
@@ -69,16 +78,23 @@ public class NodesSpecification {
              nodesElement.booleanAttribute("required", false),
              canFail,
              nodesElement.booleanAttribute("exclusive", false),
+             combined,
              getResources(nodesElement),
              Optional.ofNullable(nodesElement.stringAttribute("docker-image")));
     }
 
+    private static NodesSpecification create(boolean dedicated, boolean canFail, Version version, ModelElement nodesElement) {
+        var resolvedElement = resolveElement(nodesElement);
+        boolean combined = resolvedElement != nodesElement || isReferencedByOtherElement(nodesElement);
+        return new NodesSpecification(dedicated, canFail, combined, version, resolvedElement);
+    }
+
     /** Returns a requirement for dedicated nodes taken from the given <code>nodes</code> element */
     public static NodesSpecification from(ModelElement nodesElement, ConfigModelContext context) {
-        return new NodesSpecification(true,
-                                      ! context.getDeployState().getProperties().isBootstrap(),
-                                      context.getDeployState().getWantedNodeVespaVersion(),
-                                      nodesElement);
+        return create(true,
+                      ! context.getDeployState().getProperties().isBootstrap(),
+                      context.getDeployState().getWantedNodeVespaVersion(),
+                      nodesElement);
     }
 
     /**
@@ -103,10 +119,9 @@ public class NodesSpecification {
         if (parentElement == null) return Optional.empty();
         ModelElement nodesElement = parentElement.child("nodes");
         if (nodesElement == null) return Optional.empty();
-        return Optional.of(new NodesSpecification(nodesElement.booleanAttribute("dedicated", false),
-                                                  ! context.getDeployState().getProperties().isBootstrap(),
-                                                  context.getDeployState().getWantedNodeVespaVersion(),
-                                                  nodesElement));
+        return Optional.of(create(nodesElement.booleanAttribute("dedicated", false),
+                                  ! context.getDeployState().getProperties().isBootstrap(),
+                                  context.getDeployState().getWantedNodeVespaVersion(), nodesElement));
     }
 
     /** Returns a requirement from <code>count</code> nondedicated nodes in one group */
@@ -118,6 +133,7 @@ public class NodesSpecification {
                                       false,
                                       ! context.getDeployState().getProperties().isBootstrap(),
                                       false,
+                                      false,
                                       Optional.empty(),
                                       Optional.empty());
     }
@@ -131,6 +147,7 @@ public class NodesSpecification {
                                       false,
                                       ! context.getDeployState().getProperties().isBootstrap(),
                                       false,
+                                      false,
                                       Optional.empty(),
                                       Optional.empty());
     }
@@ -158,6 +175,9 @@ public class NodesSpecification {
                                                           ClusterSpec.Type clusterType,
                                                           ClusterSpec.Id clusterId,
                                                           DeployLogger logger) {
+        if (combined) {
+            clusterType = ClusterSpec.Type.combined;
+        }
         ClusterSpec cluster = ClusterSpec.request(clusterType, clusterId, version, exclusive);
         return hostSystem.allocateHosts(cluster, Capacity.fromCount(count, resources, required, canFail), groups, logger);
     }
@@ -239,6 +259,66 @@ public class NodesSpecification {
         }
     }
 
+    /**
+     * Resolve any reference in nodesElement and return the referred element.
+     *
+     * If nodesElement does not refer to a different element, this method behaves as the identity function.
+     */
+    private static ModelElement resolveElement(ModelElement nodesElement) {
+        var element = nodesElement.getXml();
+        var referenceId = element.getAttribute("of");
+        if (referenceId.isEmpty()) return nodesElement;
+
+        var services = findParentByTag("services", element).orElseThrow(() -> clusterReferenceNotFoundException(referenceId));
+        var referencedService = findChildById(services, referenceId).orElseThrow(() -> clusterReferenceNotFoundException(referenceId));
+        if ( ! referencedService.getTagName().equals("content"))
+            throw new IllegalArgumentException("service '" + referenceId + "' is not a content service");
+        var referencedNodesElement = XML.getChild(referencedService, "nodes");
+        if (referencedNodesElement == null)
+            throw new IllegalArgumentException("expected reference to service '" + referenceId + "' to supply nodes, " +
+                                               "but that service has no <nodes> element");
+
+        return new ModelElement(referencedNodesElement);
+    }
+
+    /** Returns whether the given nodesElement is referenced by any other nodes element */
+    private static boolean isReferencedByOtherElement(ModelElement nodesElement) {
+        var element = nodesElement.getXml();
+        var services = findParentByTag("services", element);
+        if (services.isEmpty()) return false;
+
+        var content = findParentByTag("content", element);
+        if (content.isEmpty()) return false;
+        var clusterId = content.get().getAttribute("id");
+        for (var rootChild : XML.getChildren(services.get())) {
+            if (!"container".equals(rootChild.getTagName())) continue; // Only container can reference content
+            var nodes = XML.getChild(rootChild, "nodes");
+            if (nodes == null) continue;
+            if (!clusterId.equals(nodes.getAttribute("of"))) continue;
+            return true;
+        }
+        return false;
+    }
+
+    private static Optional<Element> findChildById(Element parent, String id) {
+        for (Element child : XML.getChildren(parent))
+            if (id.equals(child.getAttribute("id"))) return Optional.of(child);
+        return Optional.empty();
+    }
+
+    private static Optional<Element> findParentByTag(String tag, 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(tag)) return Optional.of(parentElement);
+        return findParentByTag(tag, parentElement);
+    }
+
+    private static IllegalArgumentException clusterReferenceNotFoundException(String referenceId) {
+        return new IllegalArgumentException("referenced service '" + referenceId + "' is not defined");
+    }
+
     @Override
     public String toString() {
         return "specification of " + count + (dedicated ? " dedicated " : " ") + "nodes" +
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 188a9b4765c..16ecf5f761c 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
@@ -651,22 +651,17 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
     }
     
     private List<ApplicationContainer> createNodesFromContentServiceReference(ApplicationContainerCluster cluster, Element nodesElement, ConfigModelContext context) {
-        // Resolve references to content clusters at the XML level because content clusters must be built after container clusters
+        NodesSpecification nodeSpecification;
+        try {
+            nodeSpecification = NodesSpecification.from(new ModelElement(nodesElement), context);
+        } catch (IllegalArgumentException e) {
+            throw new IllegalArgumentException(cluster + " contains an invalid reference", e);
+        }
         String referenceId = nodesElement.getAttribute("of");
-        Element services = servicesRootOf(nodesElement).orElseThrow(() -> clusterReferenceNotFoundException(cluster, referenceId));
-        Element referencedService = findChildById(services, referenceId).orElseThrow(() -> clusterReferenceNotFoundException(cluster, referenceId));
-        if ( ! referencedService.getTagName().equals("content"))
-            throw new IllegalArgumentException(cluster + " references service '" + referenceId + "', " +
-                                               "but that is not a content service");
-        Element referencedNodesElement = XML.getChild(referencedService, "nodes");
-        if (referencedNodesElement == null)
-            throw new IllegalArgumentException(cluster + " references service '" + referenceId + "' to supply nodes, " + 
-                                               "but that service has no <nodes> element");
-        
         cluster.setHostClusterId(referenceId);
 
         Map<HostResource, ClusterMembership> hosts = 
-                StorageGroup.provisionHosts(NodesSpecification.from(new ModelElement(referencedNodesElement), context),
+                StorageGroup.provisionHosts(nodeSpecification,
                                             referenceId, 
                                             cluster.getRoot().getHostSystem(),
                                             context.getDeployLogger());
@@ -732,17 +727,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return nodes;
     }
 
-    private IllegalArgumentException clusterReferenceNotFoundException(ApplicationContainerCluster cluster, String referenceId) {
-        return new IllegalArgumentException(cluster + " references service '" + referenceId +
-                                            "' but this service is not defined");
-    }
-
-    private Optional<Element> findChildById(Element parent, String id) {
-        for (Element child : XML.getChildren(parent))
-            if (id.equals(child.getAttribute("id"))) return Optional.of(child);
-        return Optional.empty();
-    }
-
     private static boolean useCpuSocketAffinity(Element nodesElement) {
         if (nodesElement.hasAttribute(VespaDomBuilder.CPU_SOCKET_AFFINITY_ATTRIB_NAME))
             return Boolean.parseBoolean(nodesElement.getAttribute(VespaDomBuilder.CPU_SOCKET_AFFINITY_ATTRIB_NAME));
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 93c3c9ea2ea..6011b138a61 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
@@ -6,7 +6,7 @@ import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.api.container.ContainerServiceType;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.deploy.TestProperties;
-import com.yahoo.config.provision.ClusterMembership;
+import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.RegionName;
@@ -31,6 +31,7 @@ import com.yahoo.vespa.model.search.SearchNode;
 import com.yahoo.vespa.model.test.VespaModelTester;
 import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils;
 import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg;
+import com.yahoo.yolean.Exceptions;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -249,6 +250,10 @@ public class ModelProvisioningTest {
         assertEquals("Nodes in container1", 2, model.getContainerClusters().get("container1").getContainers().size());
         assertEquals("Heap size is lowered with combined clusters", 
                      17, physicalMemoryPercentage(model.getContainerClusters().get("container1")));
+        assertEquals(2, model.getHostSystem().getHosts().stream()
+                             .filter(hostResource -> hostResource.spec().membership().get().cluster().type() == ClusterSpec.Type.combined)
+                             .count());
+
     }
 
     @Test
@@ -330,7 +335,7 @@ public class ModelProvisioningTest {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("container cluster 'container1' references service 'container2' but this service is not defined", e.getMessage());
+            assertEquals("container cluster 'container1' contains an invalid reference: referenced service 'container2' is not defined", Exceptions.toMessageString(e));
         }
     }
 
@@ -353,7 +358,7 @@ public class ModelProvisioningTest {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("container cluster 'container1' references service 'container2', but that is not a content service", e.getMessage());
+            assertEquals("container cluster 'container1' contains an invalid reference: service 'container2' is not a content service", Exceptions.toMessageString(e));
         }
     }