From 6f6c2746261858c6950e9685c2a1693fae96848c Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 16 Mar 2020 14:29:13 +0100 Subject: Missing node specification allocates one node per cluster --- .../model/container/xml/ContainerModelBuilder.java | 67 +++++----------------- .../model/provision/ModelProvisioningTest.java | 6 +- 2 files changed, 16 insertions(+), 57 deletions(-) (limited to 'config-model') 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 165404ce9d4..30ba843a19f 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 @@ -570,7 +570,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { private void addNodesFromXml(ApplicationContainerCluster cluster, Element containerElement, ConfigModelContext context) { Element nodesElement = XML.getChild(containerElement, "nodes"); - if (nodesElement == null) { // default single node on localhost + if (nodesElement == null) { ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa()); HostResource host = allocateSingleNodeHost(cluster, log, containerElement, context); node.setHostResource(host); @@ -656,22 +656,18 @@ public class ContainerModelBuilder extends ConfigModelBuilder { private HostResource allocateSingleNodeHost(ApplicationContainerCluster cluster, DeployLogger logger, Element containerElement, ConfigModelContext context) { DeployState deployState = context.getDeployState(); HostSystem hostSystem = cluster.hostSystem(); - if (deployState.isHosted()) { - Optional 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())) - .vespaVersion(deployState.getWantedNodeVespaVersion()) - .dockerImageRepo(deployState.getWantedDockerImageRepo()) - .build(); - Capacity capacity = Capacity.fromCount(1, - Optional.empty(), - false, - ! deployState.getProperties().isBootstrap()); - return hostSystem.allocateHosts(clusterSpec, capacity, 1, logger).keySet().iterator().next(); - } + 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(); } else { return hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC); } @@ -716,43 +712,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder { 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 getHostResourceFromContentClusters(ApplicationContainerCluster cluster, Element containersElement, ConfigModelContext context) { - Optional services = servicesRootOf(containersElement); - if ( ! services.isPresent()) - return Optional.empty(); - List 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 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 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 createNodesFromHosts(DeployLogger deployLogger, Map hosts, ApplicationContainerCluster cluster) { List nodes = new ArrayList<>(); for (Map.Entry 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 d215fdbb7a0..f303ea9a42d 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 testNoNodeTagMeans1Node() { + public void testNoNodeTagMeans1NodePerCluster() { String services = "\n" + "" + @@ -1389,9 +1389,9 @@ public class ModelProvisioningTest { " " + ""; VespaModelTester tester = new VespaModelTester(); - tester.addHosts(1); + tester.addHosts(2); VespaModel model = tester.createModel(services, true); - assertEquals(1, model.getRoot().hostSystem().getHosts().size()); + assertEquals(2, 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()); -- cgit v1.2.3 From 908f657408a00b80c8907b1b09512fb0837e4bca Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Wed, 18 Mar 2020 13:35:21 +0100 Subject: Satisfy capacity requirement when allocating without nodes tag --- .../model/container/xml/ContainerModelBuilder.java | 27 +++++++++++----------- .../model/provision/ModelProvisioningTest.java | 24 +++++++++---------- .../container/xml/ContainerModelBuilderTest.java | 24 ++++++++++++------- 3 files changed, 42 insertions(+), 33 deletions(-) (limited to 'config-model') 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 30ba843a19f..b6ea5b182b3 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 @@ -571,11 +571,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { private void addNodesFromXml(ApplicationContainerCluster cluster, Element containerElement, ConfigModelContext context) { Element nodesElement = XML.getChild(containerElement, "nodes"); if (nodesElement == null) { - ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa()); - HostResource host = allocateSingleNodeHost(cluster, log, containerElement, context); - node.setHostResource(host); - node.initService(context.getDeployLogger()); - cluster.addContainers(Collections.singleton(node)); + cluster.addContainers(allocateWithoutNodesTag(cluster, log, context)); } else { List nodes = createNodes(cluster, nodesElement, context); @@ -652,25 +648,30 @@ public class ContainerModelBuilder extends ConfigModelBuilder { } } - /** Creates a single host when there is no nodes tag */ - private HostResource allocateSingleNodeHost(ApplicationContainerCluster cluster, DeployLogger logger, Element containerElement, ConfigModelContext context) { + /** Allocate a container cluster without a nodes tag */ + private List allocateWithoutNodesTag(ApplicationContainerCluster cluster, DeployLogger logger, ConfigModelContext context) { DeployState deployState = context.getDeployState(); HostSystem hostSystem = cluster.hostSystem(); - if (deployState.isHosted()) { // request 1 node + if (deployState.isHosted()) { // request just enough nodes to satisfy environment capacity requirement ClusterSpec clusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(cluster.getName()), deployState.getWantedNodeVespaVersion(), false, Optional.empty(), deployState.getWantedDockerImageRepo()); - Capacity capacity = Capacity.fromCount(1, + int nodeCount = deployState.zone().environment().isProduction() ? 2 : 1; + Capacity capacity = Capacity.fromCount(nodeCount, Optional.empty(), false, !deployState.getProperties().isBootstrap()); - return hostSystem.allocateHosts(clusterSpec, capacity, 1, logger).keySet().iterator().next(); - } else { - return hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC); - } + var hosts = hostSystem.allocateHosts(clusterSpec, capacity, 1, logger); + return createNodesFromHosts(logger, hosts, cluster); + } + ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa()); + HostResource host = hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC); + node.setHostResource(host); + node.initService(context.getDeployLogger()); + return List.of(node); } private List createNodesFromNodeCount(ApplicationContainerCluster cluster, Element nodesElement, ConfigModelContext context) { 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..60a34128368 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 @@ -1308,9 +1308,9 @@ public class ModelProvisioningTest { " " + ""; VespaModelTester tester = new VespaModelTester(); - tester.addHosts(1); + tester.addHosts(2); VespaModel model = tester.createModel(services, true); - assertEquals(1, model.getHosts().size()); + assertEquals(2, model.getHosts().size()); assertEquals(1, model.getContainerClusters().size()); } @@ -1374,7 +1374,7 @@ public class ModelProvisioningTest { } @Test - public void testNoNodeTagMeans1NodePerCluster() { + public void testNoNodeTagMeansTwoNodesInContainerCluster() { String services = "\n" + "" + @@ -1389,16 +1389,16 @@ public class ModelProvisioningTest { " " + ""; VespaModelTester tester = new VespaModelTester(); - tester.addHosts(2); + tester.addHosts(3); VespaModel model = tester.createModel(services, true); - assertEquals(2, model.getRoot().hostSystem().getHosts().size()); - assertEquals(1, model.getAdmin().getSlobroks().size()); - assertEquals(1, model.getContainerClusters().get("foo").getContainers().size()); + assertEquals(3, model.getRoot().hostSystem().getHosts().size()); + assertEquals(2, model.getAdmin().getSlobroks().size()); + assertEquals(2, model.getContainerClusters().get("foo").getContainers().size()); assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes()); } @Test - public void testNoNodeTagMeans1NodeNoContent() { + public void testNoNodeTagMeansTwoContainersNoContent() { String services = "\n" + "" + @@ -1408,11 +1408,11 @@ public class ModelProvisioningTest { " " + ""; VespaModelTester tester = new VespaModelTester(); - tester.addHosts(1); + tester.addHosts(3); VespaModel model = tester.createModel(services, true); - assertEquals(1, model.getRoot().hostSystem().getHosts().size()); - assertEquals(1, model.getAdmin().getSlobroks().size()); - assertEquals(1, model.getContainerClusters().get("foo").getContainers().size()); + assertEquals(2, model.getRoot().hostSystem().getHosts().size()); + assertEquals(2, model.getAdmin().getSlobroks().size()); + assertEquals(2, model.getContainerClusters().get("foo").getContainers().size()); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 73db6e35428..6d5baa51404 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -619,14 +619,22 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { public void singlenode_servicespec_is_used_with_hosted_vespa() throws IOException, SAXException { String servicesXml = ""; ApplicationPackage applicationPackage = new MockApplicationPackage.Builder().withServices(servicesXml).build(); - VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() - .modelHostProvisioner(new InMemoryProvisioner(true, "host1.yahoo.com", "host2.yahoo.com")) - .applicationPackage(applicationPackage) - .properties(new TestProperties() - .setMultitenant(true) - .setHostedVespa(true)) - .build()); - assertEquals(1, model.hostSystem().getHosts().size()); + + var tests = Map.of(Environment.test, 1, + Environment.prod, 2); + for (var test : tests.entrySet()) { + VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(true, "host1.yahoo.com", "host2.yahoo.com")) + .applicationPackage(applicationPackage) + .zone(new Zone(test.getKey(), RegionName.defaultName())) + .properties(new TestProperties() + .setMultitenant(true) + .setHostedVespa(true)) + .build()); + assertEquals("Allocates " + test.getValue() + " node(s) in " + test.getKey(), + (int) test.getValue(), + model.hostSystem().getHosts().size()); + } } @Test(expected = IllegalArgumentException.class) -- cgit v1.2.3