diff options
author | jonmv <venstad@gmail.com> | 2023-01-18 09:16:19 +0100 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2023-01-18 09:16:19 +0100 |
commit | 221e259351b0f88547a0b96d693d580ba827a397 (patch) | |
tree | ae54bba123bb20efd35636ef90dba93426b3d434 | |
parent | 0a312d860239278841c03acef5720b5724844a40 (diff) |
Verify container cluster names in hsoted vespa
5 files changed, 101 insertions, 21 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java index 95d97bc9e87..756646beddb 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java @@ -80,7 +80,7 @@ public class ConfigModelRepo implements ConfigModelRepoAdder, Serializable, Iter ConfigModelRegistry configModelRegistry) throws IOException { Element userServicesElement = getServicesFromApp(deployState.getApplicationPackage()); readConfigModels(root, userServicesElement, deployState, vespaModel, configModelRegistry); - builder.postProc(deployState.getDeployLogger(), root, this); + builder.postProc(deployState, root, this); } private Element getServicesFromApp(ApplicationPackage applicationPackage) throws IOException { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java index 2cf32f1e8ff..421e3a2902c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java @@ -22,6 +22,6 @@ public abstract class VespaModelBuilder { * @param producerRoot the root producer. * @param configModelRepo a {@link com.yahoo.config.model.ConfigModelRepo instance} */ - public abstract void postProc(DeployLogger deployLogger, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); + public abstract void postProc(DeployState deployState, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java index cb3c43074fc..a5ff77dc1eb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.builder.xml.dom; +import ai.vespa.validation.Validation; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.ConfigModelRepo; @@ -22,8 +23,15 @@ import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.search.SearchCluster; import org.w3c.dom.Element; + +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Pattern; /** * Builds Vespa model components using the w3c dom api @@ -43,10 +51,12 @@ public class VespaDomBuilder extends VespaModelBuilder { public static final String VESPAMALLOC = "vespamalloc"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG = "vespamalloc-debug"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG_STACKTRACE = "vespamalloc-debug-stacktrace"; // Intended for vespa engineers - private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; public static final String CPU_SOCKET_AFFINITY_ATTRIB_NAME = "cpu-socket-affinity"; public static final String Allocated_MEMORY_ATTRIB_NAME = "allocated-memory"; + private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; + private static final Pattern clusterPattern = Pattern.compile("([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])"); + public static final Logger log = Logger.getLogger(VespaDomBuilder.class.getPackage().toString()); @@ -232,13 +242,14 @@ public class VespaDomBuilder extends VespaModelBuilder { * @param root root config producer * @param configModelRepo a {@link ConfigModelRepo} */ - public void postProc(DeployLogger deployLogger, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { + public void postProc(DeployState deployState, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { setContentSearchClusterIndexes(configModelRepo); createDocprocMBusServersAndClients(configModelRepo); + if (deployState.isHosted()) validateContainerClusterIds(configModelRepo); } private void createDocprocMBusServersAndClients(ConfigModelRepo pc) { - for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { + for (ContainerCluster<?> cluster: ContainerModel.containerClusters(pc)) { addServerAndClientsForChains(cluster.getDocproc()); } } @@ -248,6 +259,18 @@ public class VespaDomBuilder extends VespaModelBuilder { docproc.getChains().addServersAndClientsForChains(); } + private void validateContainerClusterIds(ConfigModelRepo pc) { + Map<String, String> normalizedClusterIds = new LinkedHashMap<>(); + for (ContainerCluster<?> cluster: ContainerModel.containerClusters(pc)) { + String name = cluster.getName(); + Validation.requireMatch(name, "container cluster name", clusterPattern); + String clashing = normalizedClusterIds.put(name.replaceAll("_", "-"), name); + if (clashing != null) throw new IllegalArgumentException("container clusters '" + clashing + "' and '" + name + + "' have clashing endpoint names, when '_' is replaced " + + "with '-' to form valid domain names"); + } + } + /** * For some reason, search clusters need to be enumerated. * @param configModelRepo a {@link ConfigModelRepo} 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 a5b396508ef..f0c39ecc920 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 @@ -23,8 +23,8 @@ import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.config.provision.ZoneEndpoint; -import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provision.ZoneEndpoint.AccessType; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provisioning.FlavorsConfig; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.QrConfig; @@ -181,6 +181,63 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } @Test + void container_cluster_with_invalid_name_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + <services version='1.0'> + <container id='C-1' version='1.0'> + <nodes count='1' /> + </container> + </services> + """; + + assertEquals("container cluster name must match '([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])', but got: 'C-1'", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + + @Test + void two_clusters_with_clashing_cluster_names_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + <services version='1.0'> + <container id='c-1' version='1.0'> + <nodes count='1' /> + </container> + <container id='c_1' version='1.0'> + <nodes count='1' /> + </container> + </services> + """; + + assertEquals("container clusters 'c-1' and 'c_1' have clashing endpoint names, when '_' is replaced with '-' to form valid domain names", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + + @Test void two_clusters_without_explicit_port_throws_exception() { Element cluster1Elem = DomBuilderTest.parse( "<container id='cluster1' version='1.0'>", diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java index 5143aa91f56..086df4d0c33 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java @@ -49,8 +49,8 @@ public class VirtualNodeProvisioningTest { private static final NodeResources resources1 = new NodeResources(4, 8, 100, 1); private static final NodeResources resources2 = new NodeResources(1, 4, 100, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.local); - private static final ClusterSpec contentClusterSpec = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion("6.42").build(); - private static final ClusterSpec containerClusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContainer")).vespaVersion("6.42").build(); + private static final ClusterSpec contentClusterSpec = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion("6.42").build(); + private static final ClusterSpec containerClusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-container")).vespaVersion("6.42").build(); private final ApplicationId applicationId = ProvisioningTester.applicationId("test"); @@ -242,7 +242,7 @@ public class VirtualNodeProvisioningTest { Version wantedVespaVersion = Version.fromString("6.39"); int nodeCount = 7; List<HostSpec> hosts = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), nodeCount, 1, resources2); tester.activate(application1, new HashSet<>(hosts)); @@ -253,7 +253,7 @@ public class VirtualNodeProvisioningTest { // Upgrade Vespa version on nodes Version upgradedWantedVespaVersion = Version.fromString("6.40"); List<HostSpec> upgradedHosts = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(upgradedWantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(upgradedWantedVespaVersion).build(), nodeCount, 1, resources2); tester.activate(application1, new HashSet<>(upgradedHosts)); NodeList upgradedNodes = tester.getNodes(application1, Node.State.active); @@ -275,7 +275,7 @@ public class VirtualNodeProvisioningTest { int nodeCount = 7; try { List<HostSpec> nodes = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), nodeCount, 1, resources2); fail("Expected the allocation to fail due to parent hosts not being active yet"); } catch (NodeAllocationException expected) { } @@ -285,7 +285,7 @@ public class VirtualNodeProvisioningTest { // Try allocating tenants again List<HostSpec> nodes = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), nodeCount, 1, resources2); tester.activate(application1, new HashSet<>(nodes)); @@ -309,14 +309,14 @@ public class VirtualNodeProvisioningTest { Version wantedVespaVersion = Version.fromString("6.39"); List<HostSpec> nodes = tester.prepare(application2_1, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 6, 1, resources); assertHostSpecParentReservation(nodes, Optional.empty(), tester); // We do not get nodes on hosts reserved to tenant1 tester.activate(application2_1, nodes); try { tester.prepare(application2_2, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 5, 1, resources); fail("Expected exception"); } @@ -325,7 +325,7 @@ public class VirtualNodeProvisioningTest { } nodes = tester.prepare(application1_1, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 10, 1, resources); assertHostSpecParentReservation(nodes, Optional.of(tenant1), tester); tester.activate(application1_1, nodes); @@ -346,14 +346,14 @@ public class VirtualNodeProvisioningTest { try { // No capacity for 'container' nodes tester.prepare(applicationId, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 6, 1, resources); fail("Expected to fail node allocation"); } catch (NodeAllocationException ignored) { } // Same cluster, but content type is now 'content' List<HostSpec> nodes = tester.prepare(applicationId, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 6, 1, resources); tester.activate(applicationId, nodes); @@ -445,7 +445,7 @@ public class VirtualNodeProvisioningTest { assertEquals("No room for 3 nodes as 2 of 4 hosts are exclusive", "Could not satisfy request for 3 nodes with " + "[vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64] " + - "in tenant2.app2 container cluster 'myContainer' 6.39: " + + "in tenant2.app2 container cluster 'my-container' 6.39: " + "Node allocation failure on group 0: " + "Not enough suitable nodes available due to host exclusivity constraints", e.getMessage()); @@ -465,14 +465,14 @@ public class VirtualNodeProvisioningTest { tester.makeReadyChildren(1, resources2, "host2"); tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion("6.42").build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion("6.42").build(), 2, 1, resources2.with(NodeResources.StorageType.remote)); } catch (NodeAllocationException e) { assertEquals("Could not satisfy request for 2 nodes with " + "[vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: remote, architecture: x86_64] " + - "in tenant.app1 content cluster 'myContent'" + + "in tenant.app1 content cluster 'my-content'" + " 6.42: Node allocation failure on group 0", e.getMessage()); } @@ -672,7 +672,7 @@ public class VirtualNodeProvisioningTest { private void prepareAndActivate(ApplicationId application, int nodeCount, boolean exclusive, NodeResources resources, ProvisioningTester tester) { Set<HostSpec> hosts = new HashSet<>(tester.prepare(application, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContainer")).vespaVersion("6.39").exclusive(exclusive).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-container")).vespaVersion("6.39").exclusive(exclusive).build(), Capacity.from(new ClusterResources(nodeCount, 1, resources), false, true))); tester.activate(application, hosts); } |