Check explicitly for content cluster and add test

2 files changed, 52 insertions, 7 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 965b221c61f..d570806d08c 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
@@ -52,7 +52,6 @@ import com.yahoo.vespa.model.container.search.searchchain.SearchChains;
 import com.yahoo.vespa.model.container.xml.document.DocumentFactoryBuilder;
 
 import com.yahoo.vespa.model.content.StorageGroup;
-import com.yahoo.vespa.model.content.cluster.ContentCluster;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 
@@ -399,7 +398,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         if (nodesElement.hasAttribute("count"))
             return createNodesFromNodeCount(cluster, nodesElement);
         else if (nodesElement.hasAttribute("of"))
-            return createNodesFromContentClusterReference(cluster, nodesElement, context);
+            return createNodesFromContentServiceReference(cluster, nodesElement, context);
         else // the non-hosted option
             return createNodesFromNodeList(cluster, nodesElement);
     }
@@ -434,15 +433,18 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return createNodesFromHosts(hosts, cluster);
     }
     
-    private List<Container> createNodesFromContentClusterReference(ContainerCluster cluster, Element nodesElement, ConfigModelContext context) {
+    private List<Container> createNodesFromContentServiceReference(ContainerCluster cluster, Element nodesElement, ConfigModelContext context) {
         // Resolve references to content clusters at the XML level because content clusters must be built after container clusters
         String referenceId = nodesElement.getAttribute("of");
         Element services = servicesRootOf(nodesElement).orElseThrow(() -> clusterReferenceNotFoundException(cluster, referenceId));
-        Element referencedCluster = findChildById(services, referenceId).orElseThrow(() -> clusterReferenceNotFoundException(cluster, referenceId));
-        Element referencedNodesElement = XML.getChild(referencedCluster, "nodes");
+        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");
+                                               "but that service has no <nodes> element");
         Map<HostResource, ClusterMembership> hosts = 
                 StorageGroup.provisionHosts(NodesSpecification.from(new ModelElement(referencedNodesElement)), 
                                             referenceId, 
@@ -487,7 +489,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
     private IllegalArgumentException clusterReferenceNotFoundException(ContainerCluster cluster, String referenceId) {
         return new IllegalArgumentException(cluster + " references service '" + referenceId +
-                                            "' but this cluster is not defined");
+                                            "' but this service is not defined");
     }
 
     private Optional<Element> findChildById(Element parent, String id) {
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 09e2fcebd60..89fca9b4fe7 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
@@ -207,6 +207,49 @@ public class ModelProvisioningTest {
     }
 
     @Test
+    public void testNonExistingCombinedClusterReference() {
+        String xmlWithNodes =
+                "<?xml version='1.0' encoding='utf-8' ?>" +
+                "<services>" +
+                "  <container version='1.0' id='container1'>" +
+                "     <nodes of='container2'/>" +
+                "  </container>" +
+                "</services>";
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(2);
+        try {
+            tester.createModel(xmlWithNodes, true);
+            fail("Expected exception");
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("container cluster 'container1' references service 'container2' but this service is not defined", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testInvalidCombinedClusterReference() {
+        String xmlWithNodes =
+                "<?xml version='1.0' encoding='utf-8' ?>" +
+                "<services>" +
+                "  <container version='1.0' id='container1'>" +
+                "     <nodes of='container2'/><!-- invalid; only content clusters can be referenced -->" +
+                "  </container>" +
+                "  <container version='1.0' id='container2'>" +
+                "     <nodes count='2'/>" +
+                "  </container>" +
+                "</services>";
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(2);
+        try {
+            tester.createModel(xmlWithNodes, true);
+            fail("Expected exception");
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("container cluster 'container1' references service 'container2', but that is not a content service", e.getMessage());
+        }
+    }
+
+    @Test
     public void testNodeCountForContentGroupHierarchy() {
         String services = 
                 "<?xml version='1.0' encoding='utf-8' ?>\n" +