From 482ad2d6f959149531a8dd9ecef5b540f14b430d Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 28 May 2020 15:37:12 +0200 Subject: Avoid creating empty instance specs --- .../config/application/api/xml/DeploymentSpecXmlReader.java | 3 +++ .../com/yahoo/config/application/api/DeploymentSpecTest.java | 4 ++-- .../application/api/DeploymentSpecWithoutInstanceTest.java | 12 ++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java index 68f27a21ce4..6519a6f50af 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java @@ -133,6 +133,9 @@ public class DeploymentSpecXmlReader { Element instanceTag, MutableOptional globalServiceId, Element parentTag) { + if (isEmptySpec(instanceTag)) + return List.of(); + if (validate) validateTagOrder(instanceTag); diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index 32d903786dc..efaf90e1eed 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -652,7 +652,7 @@ public class DeploymentSpecTest { public void deploymentSpecWithIncreasinglyStrictUpgradePoliciesInParallel() { StringReader r = new StringReader( "" + - " " + + " " + " " + " " + " " + @@ -678,7 +678,7 @@ public class DeploymentSpecTest { " " + " " + " " + - " " + + " " + "" ); DeploymentSpec.fromXml(r); diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java index 4a7ef7b43f7..79a425f5e26 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java @@ -325,6 +325,18 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals("", DeploymentSpec.empty.xmlForm()); } + @Test + public void testOnlyAthenzServiceDefined() { + StringReader r = new StringReader( + "" + + "" + ); + DeploymentSpec spec = DeploymentSpec.fromXml(r); + + assertEquals("domain", spec.athenzDomain().get().value()); + assertEquals(List.of(), spec.instances()); + } + @Test public void productionSpecWithParallelDeployments() { StringReader r = new StringReader( -- cgit v1.2.3 From 57d19fab47444292cb4d9a87d5f8a3c4b4da5dee Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 28 May 2020 21:05:55 +0200 Subject: Fix check for whether instance is empty --- .../application/api/xml/DeploymentSpecXmlReader.java | 10 +++++++++- .../config/application/api/DeploymentSpecTest.java | 18 ++++++++++++++++++ .../api/DeploymentSpecWithoutInstanceTest.java | 1 + 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java index 6519a6f50af..48a675fa182 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java @@ -45,6 +45,7 @@ import java.util.stream.Stream; */ public class DeploymentSpecXmlReader { + private static final String deploymentTag = "deployment"; private static final String instanceTag = "instance"; private static final String majorVersionTag = "major-version"; private static final String testTag = "test"; @@ -93,6 +94,9 @@ public class DeploymentSpecXmlReader { /** Reads a deployment spec from XML */ public DeploymentSpec read(String xmlForm) { Element root = XML.getDocument(xmlForm).getDocumentElement(); + if ( ! root.getTagName().equals(deploymentTag)) + throw new IllegalArgumentException("The root tag must be "); + if (isEmptySpec(root)) return DeploymentSpec.empty; @@ -133,7 +137,11 @@ public class DeploymentSpecXmlReader { Element instanceTag, MutableOptional globalServiceId, Element parentTag) { - if (isEmptySpec(instanceTag)) + if (instanceNameString.isBlank()) + throw new IllegalArgumentException(" attribute 'id' must be specified, and not be blank"); + + // If this is an absolutely empty instance, or the implicit "default" instance but without content, ignore it + if (XML.getChildren(instanceTag).isEmpty() && (instanceTag.getAttributes().getLength() == 0 || instanceTag == parentTag)) return List.of(); if (validate) diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index efaf90e1eed..a793630c8b9 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -3,6 +3,7 @@ package com.yahoo.config.application.api; import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import org.junit.Test; @@ -421,6 +422,23 @@ public class DeploymentSpecTest { } } + @Test + public void testOnlyAthenzServiceDefinedInInstance() { + StringReader r = new StringReader( + "" + + " " + + "" + ); + DeploymentSpec spec = DeploymentSpec.fromXml(r); + + assertEquals("domain", spec.athenzDomain().get().value()); + assertEquals(1, spec.instances().size()); + + DeploymentInstanceSpec instance = spec.instances().get(0); + assertEquals("default", instance.name().value()); + assertEquals("service", instance.athenzService(Environment.prod, RegionName.defaultName()).get().value()); + } + @Test public void productionSpecWithParallelDeployments() { StringReader r = new StringReader( diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java index 79a425f5e26..89972773e1c 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java @@ -3,6 +3,7 @@ package com.yahoo.config.application.api; import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import org.junit.Test; -- cgit v1.2.3 From da3bafc2b3609153a2c3c12c29c3f7ed0615691a Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 28 May 2020 22:28:08 +0200 Subject: Use real resource forecast in initial deployment --- .../hosted/provision/autoscale/AllocatableClusterResources.java | 6 ++++-- .../hosted/provision/provisioning/NodeRepositoryProvisioner.java | 4 +++- .../hosted/provision/provisioning/DynamicDockerProvisionTest.java | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java index 84f0b962d34..59dbe4bb8d4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java @@ -32,10 +32,12 @@ public class AllocatableClusterResources { private final double fulfilment; /** Fake allocatable resources from requested capacity */ - public AllocatableClusterResources(ClusterResources requested, ClusterSpec.Type clusterType) { + public AllocatableClusterResources(ClusterResources requested, + ClusterSpec.Type clusterType, + NodeRepository nodeRepository) { this.nodes = requested.nodes(); this.groups = requested.groups(); - this.realResources = requested.nodeResources(); // we don't know + this.realResources = nodeRepository.resourcesCalculator().requestToReal(requested.nodeResources()); this.advertisedResources = requested.nodeResources(); this.clusterType = clusterType; this.fulfilment = 1; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 3ddfb646cf3..59fca955a68 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -159,7 +159,9 @@ public class NodeRepositoryProvisioner implements Provisioner { .not().removable() .asList(); AllocatableClusterResources currentResources = - nodes.isEmpty() ? new AllocatableClusterResources(requested.minResources(), clusterSpec.type()) // new deployment: Use min + nodes.isEmpty() ? new AllocatableClusterResources(requested.minResources(), + clusterSpec.type(), + nodeRepository) // new deployment: Use min : new AllocatableClusterResources(nodes, nodeRepository); return within(Limits.of(requested), clusterSpec.isExclusive(), currentResources); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java index ea0d78e3015..25387a2746c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerProvisionTest.java @@ -192,14 +192,14 @@ public class DynamicDockerProvisionTest { tester.activate(app1, cluster1, Capacity.from(resources(2, 1, 2, 20, 40), resources(4, 1, 2, 20, 40))); tester.assertNodes("Allocation specifies memory in the advertised amount", - 3, 1, 2, 20, 40, + 2, 1, 2, 20, 40, app1, cluster1); // Redeploy the same tester.activate(app1, cluster1, Capacity.from(resources(2, 1, 2, 20, 40), resources(4, 1, 2, 20, 40))); tester.assertNodes("Allocation specifies memory in the advertised amount", - 3, 1, 2, 20, 40, + 2, 1, 2, 20, 40, app1, cluster1); } -- cgit v1.2.3