From d9671728c339a75e0d747e61a32dcbb9e6877ef2 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 20 Mar 2020 10:46:46 +0100 Subject: Allocate dedicated nodes when no nodes are specified Guarded by feature flag. --- config-model-api/abi-spec.json | 3 +- .../com/yahoo/config/model/api/ModelContext.java | 1 + .../yahoo/config/model/deploy/TestProperties.java | 7 +++ .../model/container/xml/ContainerModelBuilder.java | 70 ++++++++++++++-------- .../model/provision/ModelProvisioningTest.java | 25 ++++++++ .../yahoo/vespa/model/test/VespaModelTester.java | 8 ++- .../config/server/deploy/ModelContextImpl.java | 9 +++ .../src/main/java/com/yahoo/vespa/flags/Flags.java | 6 ++ 8 files changed, 102 insertions(+), 27 deletions(-) diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index ada3119f5bb..0c061dd8222 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -884,7 +884,8 @@ "public boolean useNewAthenzFilter()", "public boolean usePhraseSegmenting()", "public java.lang.String proxyProtocol()", - "public java.util.Optional athenzDomain()" + "public java.util.Optional athenzDomain()", + "public boolean useDedicatedNodesWhenUnspecified()" ], "fields": [] }, diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 7fcde1b5e6b..b9ada59cb0b 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -67,6 +67,7 @@ public interface ModelContext { default boolean usePhraseSegmenting() { return false; } default String proxyProtocol() { return "https-only"; } default Optional athenzDomain() { return Optional.empty(); } + default boolean useDedicatedNodesWhenUnspecified() { return false; } } } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index e49ceffabc1..802fdcc1dda 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -44,6 +44,7 @@ public class TestProperties implements ModelContext.Properties { private Optional endpointCertificateSecrets = Optional.empty(); private boolean useNewAthenzFilter = false; private boolean usePhraseSegmenting = false; + private boolean useDedicatedNodesWhenUnspecified = false; private AthenzDomain athenzDomain; @Override public boolean multitenant() { return multitenant; } @@ -66,6 +67,7 @@ public class TestProperties implements ModelContext.Properties { @Override public boolean useBucketSpaceMetric() { return true; } @Override public boolean useNewAthenzFilter() { return useNewAthenzFilter; } @Override public boolean usePhraseSegmenting() { return usePhraseSegmenting; } + @Override public boolean useDedicatedNodesWhenUnspecified() { return useDedicatedNodesWhenUnspecified; } @Override public Optional athenzDomain() { return Optional.ofNullable(athenzDomain); } public TestProperties setDefaultTermwiseLimit(double limit) { @@ -123,6 +125,11 @@ public class TestProperties implements ModelContext.Properties { return this; } + public TestProperties setUseDedicatedNodesWhenUnspecified(boolean useDedicatedNodesWhenUnspecified) { + this.useDedicatedNodesWhenUnspecified = useDedicatedNodesWhenUnspecified; + return this; + } + public TestProperties setAthenzDomain(AthenzDomain domain) { this.athenzDomain = domain; return this; 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..73f4804ad18 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,12 +570,8 @@ 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 - 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)); + if (nodesElement == null) { + cluster.addContainers(allocateWithoutNodesTag(cluster, containerElement, context)); } else { List nodes = createNodes(cluster, nodesElement, context); @@ -652,29 +648,53 @@ 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, 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(); + // TODO(mpolden): The old way of allocating. Remove when 7.198 is the oldest model in production + if (!context.properties().useDedicatedNodesWhenUnspecified()) { + Optional singleContentHost = getHostResourceFromContentClusters(cluster, containerElement, context); + if (singleContentHost.isPresent()) { // there is a content cluster; put the container on its first node + return singleHostContainerCluster(cluster, singleContentHost.get(), context); + } + 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()); + HostResource host = hostSystem.allocateHosts(clusterSpec, capacity, 1, log).keySet().iterator().next(); + return singleHostContainerCluster(cluster, host, context); + } } - 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(); - } - } else { - return hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC); - } + // 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()); + int nodeCount = deployState.zone().environment().isProduction() ? 2 : 1; + Capacity capacity = Capacity.fromCount(nodeCount, + Optional.empty(), + false, + !deployState.getProperties().isBootstrap()); + var hosts = hostSystem.allocateHosts(clusterSpec, capacity, 1, log); + return createNodesFromHosts(log, hosts, cluster); + } + return singleHostContainerCluster(cluster, hostSystem.getHost(Container.SINGLENODE_CONTAINER_SERVICESPEC), context); + } + + private List singleHostContainerCluster(ApplicationContainerCluster cluster, HostResource host, ConfigModelContext context) { + ApplicationContainer node = new ApplicationContainer(cluster, "container.0", 0, cluster.isHostedVespa()); + 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 d215fdbb7a0..1670ac23ba4 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 @@ -1397,6 +1397,31 @@ public class ModelProvisioningTest { assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes()); } + @Test + public void testNoNodeTagMeansTwoNodesInContainerClusterWithFeatureFlag() { + String services = + "\n" + + "" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + ""; + VespaModelTester tester = new VespaModelTester(); + tester.setUseDedicatedNodesWhenUnspecified(true); + tester.addHosts(3); + VespaModel model = tester.createModel(services, true); + 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() { String services = diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java index b6180ab78b9..fd837c6dea3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java @@ -48,6 +48,7 @@ public class VespaModelTester { private Map> hostsByResources = new HashMap<>(); private ApplicationId applicationId = ApplicationId.defaultId(); private boolean useDedicatedNodeForLogserver = false; + private boolean useDedicatedNodesWhenUnspecified = false; public VespaModelTester() { this(new NullConfigModelRegistry()); @@ -97,6 +98,10 @@ public class VespaModelTester { this.useDedicatedNodeForLogserver = useDedicatedNodeForLogserver; } + public void setUseDedicatedNodesWhenUnspecified(boolean useDedicatedNodesWhenUnspecified) { + this.useDedicatedNodesWhenUnspecified = useDedicatedNodesWhenUnspecified; + } + /** Creates a model which uses 0 as start index and fails on out of capacity */ public VespaModel createModel(String services, String ... retiredHostNames) { return createModel(Zone.defaultZone(), services, true, retiredHostNames); @@ -137,7 +142,8 @@ public class VespaModelTester { .setMultitenant(true) .setHostedVespa(hosted) .setApplicationId(applicationId) - .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver); + .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver) + .setUseDedicatedNodesWhenUnspecified(useDedicatedNodesWhenUnspecified); DeployState deployState = new DeployState.Builder() .applicationPackage(appPkg) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 930bdaadcea..a292ea65d9d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -146,6 +146,7 @@ public class ModelContextImpl implements ModelContext { private final boolean usePhraseSegmenting; private final String proxyProtocol; private final Optional athenzDomain; + private final boolean useDedicatedNodesWhenUnspecified; public Properties(ApplicationId applicationId, boolean multitenantFromConfig, @@ -186,6 +187,8 @@ public class ModelContextImpl implements ModelContext { this.proxyProtocol = Flags.PROXY_PROTOCOL.bindTo(flagSource) .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); this.athenzDomain = athenzDomain; + this.useDedicatedNodesWhenUnspecified = Flags.DEDICATED_NODES_WHEN_UNSPECIFIED.bindTo(flagSource) + .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); } @Override @@ -251,6 +254,12 @@ public class ModelContextImpl implements ModelContext { @Override public Optional athenzDomain() { return athenzDomain; } + + @Override + public boolean useDedicatedNodesWhenUnspecified() { + return useDedicatedNodesWhenUnspecified; + } + } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 14ac9ecd678..2cc8ee1c867 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -273,6 +273,12 @@ public class Flags { "Takes effect immediately", APPLICATION_ID); + public static final UnboundBooleanFlag DEDICATED_NODES_WHEN_UNSPECIFIED = defineFeatureFlag( + "dedicated-nodes-when-unspecified", false, + "Whether config-server should allocate dedicated container nodes when is not specified in services.xml", + "Takes effect on redeploy", + APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { -- cgit v1.2.3 From 2d1bf0d4fb7f8c78c5eb796dd44be7ee2c08c581 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 20 Mar 2020 13:26:16 +0100 Subject: Use builder --- .../vespa/model/container/xml/ContainerModelBuilder.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 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 73f4804ad18..1b2df3d47e3 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 @@ -653,7 +653,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder { DeployState deployState = context.getDeployState(); HostSystem hostSystem = cluster.hostSystem(); if (deployState.isHosted()) { - // TODO(mpolden): The old way of allocating. Remove when 7.198 is the oldest model in production + // TODO(mpolden): The old way of allocating. Remove when 7.198 is the oldest model in production and the + // feature flag is set to true in all zones. if (!context.properties().useDedicatedNodesWhenUnspecified()) { Optional singleContentHost = getHostResourceFromContentClusters(cluster, containerElement, context); if (singleContentHost.isPresent()) { // there is a content cluster; put the container on its first node @@ -674,11 +675,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder { } // 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()); + ClusterSpec.Id.from(cluster.getName())) + .vespaVersion(deployState.getWantedNodeVespaVersion()) + .dockerImageRepo(deployState.getWantedDockerImageRepo()) + .build(); int nodeCount = deployState.zone().environment().isProduction() ? 2 : 1; Capacity capacity = Capacity.fromCount(nodeCount, Optional.empty(), -- cgit v1.2.3