diff options
author | Harald Musum <musum@yahooinc.com> | 2022-06-03 13:04:34 +0200 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2022-06-08 11:45:27 +0200 |
commit | 6f96b439fb93764e718d7dd6df91e12e8ea27b0c (patch) | |
tree | 98bac48a7db38dc38fd00de888191001045c80ef /config-model/src | |
parent | 179a932b7dd4b2b2370a1881523be0b3f44f74a5 (diff) |
Remove support for jdisc tag in services.xml
Diffstat (limited to 'config-model/src')
9 files changed, 124 insertions, 166 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java index 5a6a8e86e6a..acad7532404 100644 --- a/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java +++ b/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java @@ -27,7 +27,7 @@ public class HostsXmlProvisioner implements HostProvisioner { @Override public HostSpec allocateHost(String alias) { - // Some special rules to allow no admin elements as well as jdisc element without nodes. + // Some special rules to allow no admin elements as well as container element without nodes. if (alias.equals(IMPLICIT_ADMIN_HOSTALIAS)) { if (hosts.asCollection().size() > 1) { throw new IllegalArgumentException("More than 1 host specified (" + hosts.asCollection().size() + ") and <admin> not specified"); 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 67b2e4b81b5..472e15add98 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 @@ -391,12 +391,8 @@ public class NodesSpecification { /** Returns the ID of the parent container element of nodesElement, if any */ private static Optional<String> containerIdOf(ModelElement nodesElement) { var element = nodesElement.getXml(); - for (var containerTag : List.of("container", "jdisc")) { - var container = findParentByTag(containerTag, element); - if (container.isEmpty()) continue; - return container.map(el -> el.getAttribute("id")); - } - return Optional.empty(); + var container = findParentByTag("container", element); + return container.map(el -> el.getAttribute("id")); } /** Returns the ID of the container element referencing nodesElement, if any */ diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/HttpFilter.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/HttpFilter.java index 71446438b06..e77099a0598 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/HttpFilter.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/HttpFilter.java @@ -10,7 +10,7 @@ import com.yahoo.osgi.provider.model.ComponentModel; /** * This is only for the legacy certificate filter setup, outside http. * - * TODO: Remove when 'filter' directly under 'jdisc' can be removed from services.xml + * TODO: Remove when 'filter' directly under 'container' can be removed from services.xml * * @author Tony Vaagenes */ 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 0b8598f05f7..552ada756b6 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 @@ -88,7 +88,6 @@ import com.yahoo.vespa.model.container.xml.embedder.EmbedderConfig; import com.yahoo.vespa.model.content.StorageGroup; import org.w3c.dom.Element; import org.w3c.dom.Node; - import java.net.URI; import java.security.cert.X509Certificate; import java.util.ArrayList; @@ -120,7 +119,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private static final String HOSTED_VESPA_STATUS_FILE_SETTING = "VESPA_LB_STATUS_FILE"; private static final String CONTAINER_TAG = "container"; - private static final String DEPRECATED_CONTAINER_TAG = "jdisc"; private static final String ENVIRONMENT_VARIABLES_ELEMENT = "environment-variables"; // The node count to enforce in a cluster running ZooKeeper @@ -136,8 +134,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private final boolean httpServerEnabled; protected DeployLogger log; - public static final List<ConfigModelId> configModelIds = - ImmutableList.of(ConfigModelId.fromName(CONTAINER_TAG), ConfigModelId.fromName(DEPRECATED_CONTAINER_TAG)); + public static final List<ConfigModelId> configModelIds = ImmutableList.of(ConfigModelId.fromName(CONTAINER_TAG)); private static final String xmlRendererId = RendererRegistry.xmlRendererId.getName(); private static final String jsonRendererId = RendererRegistry.jsonRendererId.getName(); @@ -162,7 +159,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { app = modelContext.getApplicationPackage(); checkVersion(spec); - checkTagName(spec, log); ApplicationContainerCluster cluster = createContainerCluster(spec, modelContext); addClusterContent(cluster, spec, modelContext); @@ -637,13 +633,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { throw new IllegalArgumentException("Expected container version to be 1.0, but got " + version); } - private void checkTagName(Element spec, DeployLogger logger) { - if (spec.getTagName().equals(DEPRECATED_CONTAINER_TAG)) { - logger.logApplicationPackage(WARNING, "'" + DEPRECATED_CONTAINER_TAG + - "' is deprecated as tag name. Use '" + CONTAINER_TAG + "' instead."); - } - } - private void addNodes(ApplicationContainerCluster cluster, Element spec, ConfigModelContext context) { if (standaloneBuilder) addStandaloneNode(cluster, context.getDeployState()); @@ -1046,7 +1035,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } public static boolean isContainerTag(Element element) { - return CONTAINER_TAG.equals(element.getTagName()) || DEPRECATED_CONTAINER_TAG.equals(element.getTagName()); + return CONTAINER_TAG.equals(element.getTagName()); } /** diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc index 207324f8d18..1ab3c9893bf 100644 --- a/config-model/src/main/resources/schema/containercluster.rnc +++ b/config-model/src/main/resources/schema/containercluster.rnc @@ -1,5 +1,5 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -ContainerCluster = element container | jdisc { +ContainerCluster = element container { attribute version { "1.0" } & attribute id { xsd:NCName }? & Include* & 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 ffc38ba932d..5e56efc4460 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 @@ -249,108 +249,101 @@ public class ModelProvisioningTest { @Test public void testCombinedCluster() { - var containerElements = Set.of("jdisc", "container"); - for (var containerElement : containerElements) { - String xmlWithNodes = - "<?xml version='1.0' encoding='utf-8' ?>" + - "<services>" + - " <" + containerElement + " version='1.0' id='container1'>" + - " <search/>" + - " <nodes of='content1'/>" + - " </" + containerElement + ">" + - " <content version='1.0' id='content1'>" + - " <redundancy>2</redundancy>" + - " <documents>" + - " <document type='type1' mode='index'/>" + - " </documents>" + - " <nodes count='2'>" + - " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + - " </nodes>" + - " </content>" + - "</services>"; - VespaModelTester tester = new VespaModelTester(); - tester.addHosts(5); - VespaModel model = tester.createModel(xmlWithNodes, true); - assertEquals("Nodes in content1", 2, model.getContentClusters().get("content1").getRootGroup().getNodes().size()); - assertEquals("Nodes in container1", 2, model.getContainerClusters().get("container1").getContainers().size()); - assertEquals("Heap size is lowered with combined clusters", - 18, physicalMemoryPercentage(model.getContainerClusters().get("container1"))); - assertEquals("Memory for proton is lowered to account for the jvm heap", - (long)((3 - reservedMemoryGb) * (Math.pow(1024, 3)) * (1 - 0.18)), protonMemorySize(model.getContentClusters().get("content1"))); - assertProvisioned(0, ClusterSpec.Id.from("container1"), ClusterSpec.Type.container, model); - assertProvisioned(2, ClusterSpec.Id.from("content1"), ClusterSpec.Id.from("container1"), ClusterSpec.Type.combined, model); - } + String xmlWithNodes = + "<?xml version='1.0' encoding='utf-8' ?>" + + "<services>" + + " <container version='1.0' id='container1'>" + + " <search/>" + + " <nodes of='content1'/>" + + " </container>" + + " <content version='1.0' id='content1'>" + + " <redundancy>2</redundancy>" + + " <documents>" + + " <document type='type1' mode='index'/>" + + " </documents>" + + " <nodes count='2'>" + + " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + + " </nodes>" + + " </content>" + + "</services>"; + VespaModelTester tester = new VespaModelTester(); + tester.addHosts(5); + VespaModel model = tester.createModel(xmlWithNodes, true); + assertEquals("Nodes in content1", 2, model.getContentClusters().get("content1").getRootGroup().getNodes().size()); + assertEquals("Nodes in container1", 2, model.getContainerClusters().get("container1").getContainers().size()); + assertEquals("Heap size is lowered with combined clusters", + 18, physicalMemoryPercentage(model.getContainerClusters().get("container1"))); + assertEquals("Memory for proton is lowered to account for the jvm heap", + (long) ((3 - reservedMemoryGb) * (Math.pow(1024, 3)) * (1 - 0.18)), protonMemorySize(model.getContentClusters() + .get("content1"))); + assertProvisioned(0, ClusterSpec.Id.from("container1"), ClusterSpec.Type.container, model); + assertProvisioned(2, ClusterSpec.Id.from("content1"), ClusterSpec.Id.from("container1"), ClusterSpec.Type.combined, model); } @Test public void testCombinedClusterWithJvmHeapSizeOverride() { - var containerElements = Set.of("jdisc", "container"); - for (var containerElement : containerElements) { - String xmlWithNodes = - "<?xml version='1.0' encoding='utf-8' ?>" + - "<services>" + - " <" + containerElement + " version='1.0' id='container1'>" + - " <search/>" + - " <nodes of='content1'>" + - " <jvm allocated-memory=\"30%\"/>" + - " </nodes>" + - " </" + containerElement + ">" + - " <content version='1.0' id='content1'>" + - " <redundancy>2</redundancy>" + - " <documents>" + - " <document type='type1' mode='index'/>" + - " </documents>" + - " <nodes count='2'>" + - " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + - " </nodes>" + - " </content>" + - "</services>"; - VespaModelTester tester = new VespaModelTester(); - tester.addHosts(5); - VespaModel model = tester.createModel(xmlWithNodes, true); - assertEquals("Nodes in content1", 2, model.getContentClusters().get("content1").getRootGroup().getNodes().size()); - assertEquals("Nodes in container1", 2, model.getContainerClusters().get("container1").getContainers().size()); - assertEquals("Heap size is lowered with combined clusters", - 30, physicalMemoryPercentage(model.getContainerClusters().get("container1"))); - assertEquals("Memory for proton is lowered to account for the jvm heap", - (long)((3 - reservedMemoryGb) * (Math.pow(1024, 3)) * (1 - 0.30)), protonMemorySize(model.getContentClusters().get("content1"))); - assertProvisioned(0, ClusterSpec.Id.from("container1"), ClusterSpec.Type.container, model); - assertProvisioned(2, ClusterSpec.Id.from("content1"), ClusterSpec.Id.from("container1"), ClusterSpec.Type.combined, model); - } + String xmlWithNodes = + "<?xml version='1.0' encoding='utf-8' ?>" + + "<services>" + + " <container version='1.0' id='container1'>" + + " <search/>" + + " <nodes of='content1'>" + + " <jvm allocated-memory=\"30%\"/>" + + " </nodes>" + + " </container>" + + " <content version='1.0' id='content1'>" + + " <redundancy>2</redundancy>" + + " <documents>" + + " <document type='type1' mode='index'/>" + + " </documents>" + + " <nodes count='2'>" + + " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + + " </nodes>" + + " </content>" + + "</services>"; + VespaModelTester tester = new VespaModelTester(); + tester.addHosts(5); + VespaModel model = tester.createModel(xmlWithNodes, true); + assertEquals("Nodes in content1", 2, model.getContentClusters().get("content1").getRootGroup().getNodes().size()); + assertEquals("Nodes in container1", 2, model.getContainerClusters().get("container1").getContainers().size()); + assertEquals("Heap size is lowered with combined clusters", + 30, physicalMemoryPercentage(model.getContainerClusters().get("container1"))); + assertEquals("Memory for proton is lowered to account for the jvm heap", + (long) ((3 - reservedMemoryGb) * (Math.pow(1024, 3)) * (1 - 0.30)), protonMemorySize(model.getContentClusters() + .get("content1"))); + assertProvisioned(0, ClusterSpec.Id.from("container1"), ClusterSpec.Type.container, model); + assertProvisioned(2, ClusterSpec.Id.from("content1"), ClusterSpec.Id.from("container1"), ClusterSpec.Type.combined, model); } /** For comparison with the above */ @Test public void testNonCombinedCluster() { - var containerElements = Set.of("jdisc", "container"); - for (var containerElement : containerElements) { - String xmlWithNodes = - "<?xml version='1.0' encoding='utf-8' ?>" + - "<services>" + - " <" + containerElement + " version='1.0' id='container1'>" + - " <search/>" + - " <nodes count='2'/>" + - " </" + containerElement + ">" + - " <content version='1.0' id='content1'>" + - " <redundancy>2</redundancy>" + - " <documents>" + - " <document type='type1' mode='index'/>" + - " </documents>" + - " <nodes count='2'>" + - " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + - " </nodes>" + - " </content>" + - "</services>"; - VespaModelTester tester = new VespaModelTester(); - tester.addHosts(7); - VespaModel model = tester.createModel(xmlWithNodes, true); - assertEquals("Nodes in content1", 2, model.getContentClusters().get("content1").getRootGroup().getNodes().size()); - assertEquals("Nodes in container1", 2, model.getContainerClusters().get("container1").getContainers().size()); - assertEquals("Heap size is normal", - 70, physicalMemoryPercentage(model.getContainerClusters().get("container1"))); - assertEquals("Memory for proton is normal", - (long)((3 - reservedMemoryGb) * (Math.pow(1024, 3))), protonMemorySize(model.getContentClusters().get("content1"))); - } + String xmlWithNodes = + "<?xml version='1.0' encoding='utf-8' ?>" + + "<services>" + + " <container version='1.0' id='container1'>" + + " <search/>" + + " <nodes count='2'/>" + + " </container>" + + " <content version='1.0' id='content1'>" + + " <redundancy>2</redundancy>" + + " <documents>" + + " <document type='type1' mode='index'/>" + + " </documents>" + + " <nodes count='2'>" + + " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + + " </nodes>" + + " </content>" + + "</services>"; + VespaModelTester tester = new VespaModelTester(); + tester.addHosts(7); + VespaModel model = tester.createModel(xmlWithNodes, true); + assertEquals("Nodes in content1", 2, model.getContentClusters().get("content1").getRootGroup().getNodes().size()); + assertEquals("Nodes in container1", 2, model.getContainerClusters().get("container1").getContainers().size()); + assertEquals("Heap size is normal", + 70, physicalMemoryPercentage(model.getContainerClusters().get("container1"))); + assertEquals("Memory for proton is normal", + (long) ((3 - reservedMemoryGb) * (Math.pow(1024, 3))), protonMemorySize(model.getContentClusters().get("content1"))); } @Test @@ -463,35 +456,31 @@ public class ModelProvisioningTest { @Test public void testCombinedClusterWithZooKeeperFails() { - var containerElements = Set.of("jdisc", "container"); - for (var containerElement : containerElements) { - String xmlWithNodes = - "<?xml version='1.0' encoding='utf-8' ?>" + - "<services>" + - " <" + containerElement + " version='1.0' id='container1'>" + - " <search/>" + - " <nodes of='content1'/>" + - " <zookeeper />" + - " </" + containerElement + ">" + - " <content version='1.0' id='content1'>" + - " <redundancy>2</redundancy>" + - " <documents>" + - " <document type='type1' mode='index'/>" + - " </documents>" + - " <nodes count='2'>" + - " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + - " </nodes>" + - " </content>" + - "</services>"; - VespaModelTester tester = new VespaModelTester(); - tester.addHosts(2); - try { - tester.createModel(xmlWithNodes, true); - fail("ZooKeeper should not be allowed on combined clusters"); - } - catch (IllegalArgumentException e) { - assertEquals("A combined cluster cannot run ZooKeeper", e.getMessage()); - } + String xmlWithNodes = + "<?xml version='1.0' encoding='utf-8' ?>" + + "<services>" + + " <container version='1.0' id='container1'>" + + " <search/>" + + " <nodes of='content1'/>" + + " <zookeeper />" + + " </container>" + + " <content version='1.0' id='content1'>" + + " <redundancy>2</redundancy>" + + " <documents>" + + " <document type='type1' mode='index'/>" + + " </documents>" + + " <nodes count='2'>" + + " <resources vcpu='1' memory='3Gb' disk='9Gb'/>" + + " </nodes>" + + " </content>" + + "</services>"; + VespaModelTester tester = new VespaModelTester(); + tester.addHosts(2); + try { + tester.createModel(xmlWithNodes, true); + fail("ZooKeeper should not be allowed on combined clusters"); + } catch (IllegalArgumentException e) { + assertEquals("A combined cluster cannot run ZooKeeper", e.getMessage()); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java b/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java index 4d8dd3dacfd..ea075dc1129 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java @@ -34,9 +34,9 @@ public class HostResourceTest { } @Test - public void host_witrh_membership() { - HostResource host = hostResourceWithMemberships(ClusterMembership.from(clusterSpec(container, "jdisc"), 0)); - assertClusterMembership(host, container, "jdisc"); + public void host_with_membership() { + HostResource host = hostResourceWithMemberships(ClusterMembership.from(clusterSpec(container, "container"), 0)); + assertClusterMembership(host, container, "container"); } private void assertClusterMembership(HostResource host, ClusterSpec.Type type, String id) { 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 e76d89a6854..d91dba0572f 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 @@ -115,22 +115,6 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } @Test - public void deprecated_jdisc_tag_is_allowed() { - Element clusterElem = DomBuilderTest.parse( - "<jdisc version='1.0'>", - nodesXml, - "</jdisc>" ); - TestLogger logger = new TestLogger(); - createModel(root, logger, clusterElem); - AbstractService container = (AbstractService)root.getProducer("jdisc/container.0"); - assertNotNull(container); - - assertFalse(logger.msgs.isEmpty()); - assertEquals(Level.WARNING, logger.msgs.get(0).getFirst()); - assertEquals("'jdisc' is deprecated as tag name. Use 'container' instead.", logger.msgs.get(0).getSecond()); - } - - @Test public void default_port_is_4080() { Element clusterElem = DomBuilderTest.parse( "<container version='1.0'>", diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/ModelAmendingTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/test/ModelAmendingTestCase.java index 3de8cfe540f..2ae1399e9d1 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/ModelAmendingTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/ModelAmendingTestCase.java @@ -82,7 +82,7 @@ public class ModelAmendingTestCase { assertFalse(host + " is amended", host.getHost().getChildrenByTypeRecursive(AmendedService.class).isEmpty()); } - // Check that jdisc clusters are amended + // Check that container clusters are amended assertEquals(2, model.getContainerClusters().size()); assertNotNull(model.getContainerClusters().get("test1").getComponentsMap().get(new ComponentId("com.yahoo.MyAmendedComponent"))); assertNotNull(model.getContainerClusters().get("test2").getComponentsMap().get(new ComponentId("com.yahoo.MyAmendedComponent"))); @@ -129,7 +129,7 @@ public class ModelAmendingTestCase { assertFalse(host + " is amended", host.getHost().getChildrenByTypeRecursive(AmendedService.class).isEmpty()); } - // Check that jdisc clusters are amended + // Check that container clusters are amended assertEquals(2, model.getContainerClusters().size()); assertNotNull(model.getContainerClusters().get("test1").getComponentsMap().get(new ComponentId("com.yahoo.MyAmendedComponent"))); assertNotNull(model.getContainerClusters().get("test2").getComponentsMap().get(new ComponentId("com.yahoo.MyAmendedComponent"))); @@ -211,7 +211,7 @@ public class ModelAmendingTestCase { @Override public void doBuild(ContainerModelAmender model, Element spec, ConfigModelContext modelContext) { - if (built) return; // the same instance will be called once per jdisc cluster + if (built) return; // the same instance will be called once per container cluster for (ContainerModel containerModel : model.containerModels) amend(containerModel.getCluster()); built = true; |