diff options
32 files changed, 876 insertions, 312 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java index ac36e8e6c4d..fc170db5897 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.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.config.application.api; +import ai.vespa.validation.Validation; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; @@ -31,6 +32,7 @@ import static ai.vespa.validation.Validation.requireAtLeast; import static ai.vespa.validation.Validation.requireInRange; import static com.yahoo.config.application.api.DeploymentSpec.RevisionChange.whenClear; import static com.yahoo.config.application.api.DeploymentSpec.RevisionTarget.next; +import static com.yahoo.config.application.api.DeploymentSpec.illegal; import static com.yahoo.config.provision.Environment.prod; /** @@ -58,6 +60,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { private final Optional<String> globalServiceId; private final Optional<AthenzService> athenzService; private final Optional<CloudAccount> cloudAccount; + private final Optional<Duration> hostTTL; private final Notifications notifications; private final List<Endpoint> endpoints; private final Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpoints; @@ -75,6 +78,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { Optional<String> globalServiceId, Optional<AthenzService> athenzService, Optional<CloudAccount> cloudAccount, + Optional<Duration> hostTTL, Notifications notifications, List<Endpoint> endpoints, Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpoints, @@ -98,6 +102,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { this.globalServiceId = Objects.requireNonNull(globalServiceId); this.athenzService = Objects.requireNonNull(athenzService); this.cloudAccount = Objects.requireNonNull(cloudAccount); + this.hostTTL = Objects.requireNonNull(hostTTL); this.notifications = Objects.requireNonNull(notifications); this.endpoints = List.copyOf(Objects.requireNonNull(endpoints)); Map<ClusterSpec.Id, Map<ZoneId, ZoneEndpoint>> zoneEndpointsCopy = new HashMap<>(); @@ -108,6 +113,10 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { validateEndpoints(globalServiceId, this.endpoints); validateChangeBlockers(changeBlockers, now); validateBcp(bcp); + hostTTL.ifPresent(ttl -> { + if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); + if (ttl.isNegative()) illegal("Host TTL cannot be negative"); + }); } public InstanceName name() { return name; } @@ -269,6 +278,15 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { .or(() -> cloudAccount); } + /** Returns the host TTL to use for given environment and region, if any */ + public Optional<Duration> hostTTL(Environment environment, Optional<RegionName> region) { + return zones().stream() + .filter(zone -> zone.concerns(environment, region)) + .findFirst() + .flatMap(DeploymentSpec.DeclaredZone::hostTTL) + .or(() -> hostTTL); + } + /** Returns the notification configuration of these instances */ public Notifications notifications() { return notifications; } diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index 1f44e599e11..ec79f9e554a 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.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.config.application.api; +import ai.vespa.validation.Validation; import com.yahoo.collections.Comparables; import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; @@ -45,6 +46,7 @@ public class DeploymentSpec { Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), List.of(), "<deployment version='1.0'/>", List.of()); @@ -56,6 +58,7 @@ public class DeploymentSpec { private final Optional<AthenzDomain> athenzDomain; private final Optional<AthenzService> athenzService; private final Optional<CloudAccount> cloudAccount; + private final Optional<Duration> hostTTL; private final List<Endpoint> endpoints; private final List<DeprecatedElement> deprecatedElements; @@ -66,6 +69,7 @@ public class DeploymentSpec { Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, Optional<CloudAccount> cloudAccount, + Optional<Duration> hostTTL, List<Endpoint> endpoints, String xmlForm, List<DeprecatedElement> deprecatedElements) { @@ -74,6 +78,7 @@ public class DeploymentSpec { this.athenzDomain = Objects.requireNonNull(athenzDomain); this.athenzService = Objects.requireNonNull(athenzService); this.cloudAccount = Objects.requireNonNull(cloudAccount); + this.hostTTL = Objects.requireNonNull(hostTTL); this.xmlForm = Objects.requireNonNull(xmlForm); this.endpoints = List.copyOf(Objects.requireNonNull(endpoints)); this.deprecatedElements = List.copyOf(Objects.requireNonNull(deprecatedElements)); @@ -81,6 +86,10 @@ public class DeploymentSpec { validateUpgradePoliciesOfIncreasingConservativeness(steps); validateAthenz(); validateApplicationEndpoints(); + hostTTL.ifPresent(ttl -> { + if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); + if (ttl.isNegative()) illegal("Host TTL cannot be negative"); + }); } public boolean isEmpty() { return this == empty; } @@ -184,6 +193,14 @@ public class DeploymentSpec { public Optional<CloudAccount> cloudAccount() { return cloudAccount; } /** + * Additional host time-to-live for this application. Requires a custom cloud account to be set. + * This also applies only to zones with dynamic provisioning, and is then the time hosts are + * allowed remain empty, before being deprovisioned. This is useful for applications which frequently + * deploy to, e.g., test and staging zones, and want to avoid the delay of having to provision hosts. + */ + public Optional<Duration> hostTTL() { return hostTTL; } + + /** * Returns the most specific zone endpoint, where specificity is given, in decreasing order: * 1. The given instance has declared a zone endpoint for the cluster, for the given region. * 2. The given instance has declared a universal zone endpoint for the cluster. @@ -262,7 +279,7 @@ public class DeploymentSpec { } - private static void illegal(String message) { + static void illegal(String message) { throw new IllegalArgumentException(message); } @@ -370,6 +387,8 @@ public class DeploymentSpec { return true; } + public Optional<Duration> hostTTL() { return Optional.empty(); } + } /** A deployment step which is to wait for some time before progressing to the next step */ @@ -403,14 +422,15 @@ public class DeploymentSpec { private final Optional<AthenzService> athenzService; private final Optional<String> testerFlavor; private final Optional<CloudAccount> cloudAccount; + private final Optional<Duration> hostTTL; public DeclaredZone(Environment environment) { - this(environment, Optional.empty(), false, Optional.empty(), Optional.empty(), Optional.empty()); + this(environment, Optional.empty(), false, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); } public DeclaredZone(Environment environment, Optional<RegionName> region, boolean active, Optional<AthenzService> athenzService, Optional<String> testerFlavor, - Optional<CloudAccount> cloudAccount) { + Optional<CloudAccount> cloudAccount, Optional<Duration> hostTTL) { if (environment != Environment.prod && region.isPresent()) illegal("Non-prod environments cannot specify a region"); if (environment == Environment.prod && region.isEmpty()) @@ -421,6 +441,11 @@ public class DeploymentSpec { this.athenzService = Objects.requireNonNull(athenzService); this.testerFlavor = Objects.requireNonNull(testerFlavor); this.cloudAccount = Objects.requireNonNull(cloudAccount); + this.hostTTL = Objects.requireNonNull(hostTTL); + hostTTL.ifPresent(ttl -> { + if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); + if (ttl.isNegative()) illegal("Host TTL cannot be negative"); + }); } public Environment environment() { return environment; } @@ -472,15 +497,26 @@ public class DeploymentSpec { return environment + (region.map(regionName -> "." + regionName).orElse("")); } + @Override + public Optional<Duration> hostTTL() { + return hostTTL; + } + } /** A declared production test */ public static class DeclaredTest extends Step { private final RegionName region; + private final Optional<Duration> hostTTL; - public DeclaredTest(RegionName region) { + public DeclaredTest(RegionName region, Optional<CloudAccount> cloudAccount, Optional<Duration> hostTTL) { this.region = Objects.requireNonNull(region); + this.hostTTL = Objects.requireNonNull(hostTTL); + hostTTL.ifPresent(ttl -> { + if (cloudAccount.isEmpty()) illegal("Host TTL can only be specified with custom cloud accounts"); + if (ttl.isNegative()) illegal("Host TTL cannot be negative"); + }); } @Override @@ -497,6 +533,11 @@ public class DeploymentSpec { } @Override + public Optional<Duration> hostTTL() { + return hostTTL; + } + + @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; 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 89373d8bca0..f265ad3bb71 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 @@ -94,6 +94,7 @@ public class DeploymentSpecXmlReader { private static final String majorVersionAttribute = "major-version"; private static final String globalServiceIdAttribute = "global-service-id"; private static final String cloudAccountAttribute = "cloud-account"; + private static final String hostTTLAttribute = "empty-host-ttl"; private final boolean validate; private final Clock clock; @@ -165,6 +166,7 @@ public class DeploymentSpecXmlReader { stringAttribute(athenzDomainAttribute, root).map(AthenzDomain::from), stringAttribute(athenzServiceAttribute, root).map(AthenzService::from), stringAttribute(cloudAccountAttribute, root).map(CloudAccount::from), + stringAttribute(hostTTLAttribute, root).map(s -> toDuration(s, "empty host TTL")), applicationEndpoints, xmlForm, deprecatedElements); @@ -204,6 +206,7 @@ public class DeploymentSpecXmlReader { List<DeploymentSpec.ChangeBlocker> changeBlockers = readChangeBlockers(instanceElement, parentTag); Optional<AthenzService> athenzService = mostSpecificAttribute(instanceElement, athenzServiceAttribute).map(AthenzService::from); Optional<CloudAccount> cloudAccount = mostSpecificAttribute(instanceElement, cloudAccountAttribute).map(CloudAccount::from); + Optional<Duration> hostTTL = mostSpecificAttribute(instanceElement, hostTTLAttribute).map(s -> toDuration(s, "empty host TTL")); Notifications notifications = readNotifications(instanceElement, parentTag); // Values where there is no default @@ -233,6 +236,7 @@ public class DeploymentSpecXmlReader { Optional.ofNullable(prodAttributes.get(globalServiceIdAttribute)), athenzService, cloudAccount, + hostTTL, notifications, endpoints, zoneEndpoints, @@ -258,6 +262,7 @@ public class DeploymentSpecXmlReader { } // Consume the given tag as 0-N steps. 0 if it is not a step, >1 if it contains multiple nested steps that should be flattened + @SuppressWarnings("fallthrough") private List<Step> readNonInstanceSteps(Element stepTag, Map<String, String> prodAttributes, Element parentTag, Bcp defaultBcp) { Optional<AthenzService> athenzService = mostSpecificAttribute(stepTag, athenzServiceAttribute).map(AthenzService::from); Optional<String> testerFlavor = mostSpecificAttribute(stepTag, testerFlavorAttribute); @@ -272,12 +277,10 @@ public class DeploymentSpecXmlReader { case testTag: if (Stream.iterate(stepTag, Objects::nonNull, Node::getParentNode) .anyMatch(node -> prodTag.equals(node.getNodeName()))) { - // A production test - return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim()))); + return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim()), readCloudAccount(stepTag), readHostTTL(stepTag))); // A production test } - return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag))); - case devTag, perfTag, stagingTag: - return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag))); + case devTag, perfTag, stagingTag: // Intentional fallthrough from test tag. + return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag), readHostTTL(stepTag))); case prodTag: // regions, delay and parallel may be nested within, but we can flatten them return XML.getChildren(stepTag).stream() .flatMap(child -> readNonInstanceSteps(child, prodAttributes, stepTag, defaultBcp).stream()) @@ -682,13 +685,17 @@ public class DeploymentSpecXmlReader { Optional<String> testerFlavor, Element regionTag) { return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())), readActive(regionTag), athenzService, testerFlavor, - readCloudAccount(regionTag)); + readCloudAccount(regionTag), readHostTTL(regionTag)); } private Optional<CloudAccount> readCloudAccount(Element tag) { return mostSpecificAttribute(tag, cloudAccountAttribute).map(CloudAccount::from); } + private Optional<Duration> readHostTTL(Element tag) { + return mostSpecificAttribute(tag, hostTTLAttribute).map(s -> toDuration(s, "empty host TTL")); + } + private Optional<String> readGlobalServiceId(Element environmentTag) { String globalServiceId = environmentTag.getAttribute(globalServiceIdAttribute); if (globalServiceId.isEmpty()) return Optional.empty(); @@ -804,8 +811,8 @@ public class DeploymentSpecXmlReader { } /** - * Returns a string consisting of a number followed by "m" or "h" to a duration given in that unit, - * or zero duration if null of blank. + * Returns a string consisting of a number followed by "m", "h" or "d" to a duration given in that unit, + * or zero duration if null or blank. */ private static Duration toDuration(String durationSpec, String sourceDescription) { try { 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 89b7318739e..721c327e5d4 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 @@ -32,6 +32,11 @@ import java.util.stream.Collectors; import static com.yahoo.config.application.api.Notifications.Role.author; import static com.yahoo.config.application.api.Notifications.When.failing; import static com.yahoo.config.application.api.Notifications.When.failingCommit; +import static com.yahoo.config.provision.Environment.dev; +import static com.yahoo.config.provision.Environment.perf; +import static com.yahoo.config.provision.Environment.prod; +import static com.yahoo.config.provision.Environment.staging; +import static com.yahoo.config.provision.Environment.test; import static com.yahoo.config.provision.zone.ZoneId.defaultId; import static com.yahoo.config.provision.zone.ZoneId.from; import static org.junit.Assert.assertEquals; @@ -60,11 +65,11 @@ public class DeploymentSpecTest { assertEquals(specXml, spec.xmlForm()); assertEquals(1, spec.requireInstance("default").steps().size()); assertFalse(spec.majorVersion().isPresent()); - assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.test)); - assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.empty())); - assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region - assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); - assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty())); + assertTrue(spec.requireInstance("default").steps().get(0).concerns(test)); + assertTrue(spec.requireInstance("default").concerns(test, Optional.empty())); + assertTrue(spec.requireInstance("default").concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region + assertFalse(spec.requireInstance("default").concerns(staging, Optional.empty())); + assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @@ -97,10 +102,10 @@ public class DeploymentSpecTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals(1, spec.steps().size()); assertEquals(1, spec.requireInstance("default").steps().size()); - assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.staging)); - assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty())); - assertTrue(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); - assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty())); + assertTrue(spec.requireInstance("default").steps().get(0).concerns(staging)); + assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); + assertTrue(spec.requireInstance("default").concerns(staging, Optional.empty())); + assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @@ -121,17 +126,17 @@ public class DeploymentSpecTest { assertEquals(1, spec.steps().size()); assertEquals(2, spec.requireInstance("default").steps().size()); - assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.requireInstance("default").steps().get(0).concerns(prod, Optional.of(RegionName.from("us-east1")))); assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(0)).active()); - assertTrue(spec.requireInstance("default").steps().get(1).concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(spec.requireInstance("default").steps().get(1).concerns(prod, Optional.of(RegionName.from("us-west1")))); assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(1)).active()); - assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty())); - assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); - assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("no-such-region")))); + assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); + assertFalse(spec.requireInstance("default").concerns(staging, Optional.empty())); + assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-west1")))); + assertFalse(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("no-such-region")))); assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy()); @@ -293,7 +298,7 @@ public class DeploymentSpecTest { assertEquals(1, instance2.steps().size()); assertEquals(1, instance2.zones().size()); - assertTrue(instance2.steps().get(0).concerns(Environment.prod, Optional.of(RegionName.from("us-central1")))); + assertTrue(instance2.steps().get(0).concerns(prod, Optional.of(RegionName.from("us-central1")))); } @Test @@ -322,25 +327,25 @@ public class DeploymentSpecTest { assertEquals(5, instance.steps().size()); assertEquals(4, instance.zones().size()); - assertTrue(instance.steps().get(0).concerns(Environment.test)); + assertTrue(instance.steps().get(0).concerns(test)); - assertTrue(instance.steps().get(1).concerns(Environment.staging)); + assertTrue(instance.steps().get(1).concerns(staging)); - assertTrue(instance.steps().get(2).concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(instance.steps().get(2).concerns(prod, Optional.of(RegionName.from("us-east1")))); assertFalse(((DeploymentSpec.DeclaredZone)instance.steps().get(2)).active()); assertTrue(instance.steps().get(3) instanceof DeploymentSpec.Delay); assertEquals(3 * 60 * 60 + 30 * 60, instance.steps().get(3).delay().getSeconds()); - assertTrue(instance.steps().get(4).concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(instance.steps().get(4).concerns(prod, Optional.of(RegionName.from("us-west1")))); assertTrue(((DeploymentSpec.DeclaredZone)instance.steps().get(4)).active()); - assertTrue(instance.concerns(Environment.test, Optional.empty())); - assertTrue(instance.concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region - assertTrue(instance.concerns(Environment.staging, Optional.empty())); - assertTrue(instance.concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(instance.concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(instance.concerns(Environment.prod, Optional.of(RegionName.from("no-such-region")))); + assertTrue(instance.concerns(test, Optional.empty())); + assertTrue(instance.concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region + assertTrue(instance.concerns(staging, Optional.empty())); + assertTrue(instance.concerns(prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(instance.concerns(prod, Optional.of(RegionName.from("us-west1")))); + assertFalse(instance.concerns(prod, Optional.of(RegionName.from("no-such-region")))); assertFalse(instance.globalServiceId().isPresent()); } @@ -563,7 +568,7 @@ public class DeploymentSpecTest { DeploymentInstanceSpec instance = spec.instances().get(0); assertEquals("default", instance.name().value()); - assertEquals("service", instance.athenzService(Environment.prod, RegionName.defaultName()).get().value()); + assertEquals("service", instance.athenzService(prod, RegionName.defaultName()).get().value()); } @Test @@ -695,9 +700,9 @@ public class DeploymentSpecTest { List<DeploymentSpec.Step> innerParallelSteps = secondSerialSteps.get(2).steps(); assertEquals(3, innerParallelSteps.size()); assertEquals("prod.ap-northeast-1", innerParallelSteps.get(0).toString()); - assertEquals("no-service", spec.requireInstance("instance").athenzService(Environment.prod, RegionName.from("ap-northeast-1")).get().value()); + assertEquals("no-service", spec.requireInstance("instance").athenzService(prod, RegionName.from("ap-northeast-1")).get().value()); assertEquals("prod.ap-southeast-2", innerParallelSteps.get(1).toString()); - assertEquals("in-service", spec.requireInstance("instance").athenzService(Environment.prod, RegionName.from("ap-southeast-2")).get().value()); + assertEquals("in-service", spec.requireInstance("instance").athenzService(prod, RegionName.from("ap-southeast-2")).get().value()); assertEquals("tests for prod.aws-us-east-1a", innerParallelSteps.get(2).toString()); } @@ -956,7 +961,7 @@ public class DeploymentSpecTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); assertEquals("service", spec.athenzService().get().value()); - assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod, + assertEquals("service", spec.requireInstance("instance1").athenzService(prod, RegionName.from("us-west-1")).get().value()); } @@ -979,11 +984,11 @@ public class DeploymentSpecTest { assertEquals("domain", spec.athenzDomain().get().value()); assertEquals("service", spec.athenzService().get().value()); - assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod, + assertEquals("prod-service", spec.requireInstance("instance1").athenzService(prod, RegionName.from("us-central-1")).get().value()); - assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod, + assertEquals("prod-service", spec.requireInstance("instance1").athenzService(prod, RegionName.from("us-west-1")).get().value()); - assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod, + assertEquals("prod-service", spec.requireInstance("instance1").athenzService(prod, RegionName.from("us-east-3")).get().value()); } @@ -1014,11 +1019,11 @@ public class DeploymentSpecTest { """; DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); - assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod, + assertEquals("service", spec.requireInstance("instance1").athenzService(prod, RegionName.from("us-west-1")).get().value()); - assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod, + assertEquals("service", spec.requireInstance("instance1").athenzService(prod, RegionName.from("us-east-3")).get().value()); - assertEquals("service", spec.requireInstance("instance2").athenzService(Environment.prod, + assertEquals("service", spec.requireInstance("instance2").athenzService(prod, RegionName.from("us-east-3")).get().value()); } @@ -1036,7 +1041,7 @@ public class DeploymentSpecTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); assertEquals(Optional.empty(), spec.athenzService()); - assertEquals("service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value()); + assertEquals("service", spec.requireInstance("default").athenzService(prod, RegionName.from("us-west-1")).get().value()); } @Test @@ -1054,13 +1059,13 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("service", - spec.requireInstance("default").athenzService(Environment.test, + spec.requireInstance("default").athenzService(test, RegionName.from("us-east-1")).get().value()); assertEquals("staging-service", - spec.requireInstance("default").athenzService(Environment.staging, + spec.requireInstance("default").athenzService(staging, RegionName.from("us-north-1")).get().value()); assertEquals("prod-service", - spec.requireInstance("default").athenzService(Environment.prod, + spec.requireInstance("default").athenzService(prod, RegionName.from("us-west-1")).get().value()); } @@ -1273,8 +1278,8 @@ public class DeploymentSpecTest { assertEquals(List.of(RegionName.from("us-east")), spec.requireInstance("default").endpoints().get(0).regions()); - var zone = from(Environment.prod, RegionName.from("us-east")); - var testZone = from(Environment.test, RegionName.from("us-east")); + var zone = from(prod, RegionName.from("us-east")); + var testZone = from(test, RegionName.from("us-east")); assertEquals(ZoneEndpoint.defaultEndpoint, spec.zoneEndpoint(InstanceName.from("custom"), zone, ClusterSpec.Id.from("bax"))); assertEquals(ZoneEndpoint.defaultEndpoint, @@ -1778,23 +1783,152 @@ public class DeploymentSpecTest { """; DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals(Optional.of(CloudAccount.from("100000000000")), spec.cloudAccount()); - assertCloudAccount("800000000000", spec.requireInstance("alpha"), Environment.prod, "us-east-1"); - assertCloudAccount("200000000000", spec.requireInstance("beta"), Environment.prod, "us-west-1"); - assertCloudAccount("600000000000", spec.requireInstance("beta"), Environment.staging, ""); - assertCloudAccount("700000000000", spec.requireInstance("beta"), Environment.perf, ""); - assertCloudAccount("200000000000", spec.requireInstance("beta"), Environment.dev, ""); - assertCloudAccount("300000000000", spec.requireInstance("main"), Environment.prod, "us-east-1"); - assertCloudAccount("100000000000", spec.requireInstance("main"), Environment.prod, "eu-west-1"); - assertCloudAccount("400000000000", spec.requireInstance("main"), Environment.dev, ""); - assertCloudAccount("500000000000", spec.requireInstance("main"), Environment.test, ""); - assertCloudAccount("100000000000", spec.requireInstance("main"), Environment.staging, ""); - assertCloudAccount("default", spec.requireInstance("beta"), Environment.prod, "us-west-2"); + assertCloudAccount("800000000000", spec.requireInstance("alpha"), prod, "us-east-1"); + assertCloudAccount("200000000000", spec.requireInstance("beta"), prod, "us-west-1"); + assertCloudAccount("600000000000", spec.requireInstance("beta"), staging, ""); + assertCloudAccount("700000000000", spec.requireInstance("beta"), perf, ""); + assertCloudAccount("200000000000", spec.requireInstance("beta"), dev, ""); + assertCloudAccount("300000000000", spec.requireInstance("main"), prod, "us-east-1"); + assertCloudAccount("100000000000", spec.requireInstance("main"), prod, "eu-west-1"); + assertCloudAccount("400000000000", spec.requireInstance("main"), dev, ""); + assertCloudAccount("500000000000", spec.requireInstance("main"), test, ""); + assertCloudAccount("100000000000", spec.requireInstance("main"), staging, ""); + assertCloudAccount("default", spec.requireInstance("beta"), prod, "us-west-2"); + } + + @Test + public void hostTTL() { + String r = + """ + <deployment version='1.0' cloud-account='100000000000' empty-host-ttl='1h'> + <instance id='alpha'> + <staging /> + <prod empty-host-ttl='1m'> + <region>us-east</region> + <region empty-host-ttl='2m'>us-west</region> + <test>us-east</test> + <test empty-host-ttl='3m'>us-west</test> + </prod> + </instance> + <instance id='beta'> + <staging empty-host-ttl='3d'/> + <perf empty-host-ttl='4h'/> + <prod> + <region>us-east</region> + <region empty-host-ttl='0d'>us-west</region> + </prod> + </instance> + <instance id='gamma' empty-host-ttl='6h'> + <dev empty-host-ttl='7d'/> + <prod> + <region>us-east</region> + </prod> + </instance> + </deployment> + """; + DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals(Optional.of(CloudAccount.from("100000000000")), spec.cloudAccount()); + + assertHostTTL(Duration.ofHours(1), spec, "alpha", test, null); + assertHostTTL(Duration.ofHours(1), spec, "alpha", staging, null); + assertHostTTL(Duration.ofHours(1), spec, "alpha", dev, null); + assertHostTTL(Duration.ofHours(1), spec, "alpha", perf, null); + assertHostTTL(Duration.ofMinutes(1), spec, "alpha", prod, "us-east"); + assertHostTTL(Duration.ofMinutes(2), spec, "alpha", prod, "us-west"); + assertEquals(Optional.of(Duration.ofMinutes(1)), spec.requireInstance("alpha").steps().stream() + .filter(step -> step.concerns(prod, Optional.of(RegionName.from("us-east"))) && step.isTest()) + .findFirst().orElseThrow() + .hostTTL()); + assertEquals(Optional.of(Duration.ofMinutes(3)), spec.requireInstance("alpha").steps().stream() + .filter(step -> step.concerns(prod, Optional.of(RegionName.from("us-west"))) && step.isTest()) + .findFirst().orElseThrow() + .hostTTL()); + + assertHostTTL(Duration.ofHours(1), spec, "beta", test, null); + assertHostTTL(Duration.ofDays(3), spec, "beta", staging, null); + assertHostTTL(Duration.ofHours(1), spec, "beta", dev, null); + assertHostTTL(Duration.ofHours(4), spec, "beta", perf, null); + assertHostTTL(Duration.ofHours(1), spec, "beta", prod, "us-east"); + assertHostTTL(Duration.ZERO, spec, "beta", prod, "us-west"); + + assertHostTTL(Duration.ofHours(6), spec, "gamma", test, null); + assertHostTTL(Duration.ofHours(6), spec, "gamma", staging, null); + assertHostTTL(Duration.ofDays(7), spec, "gamma", dev, null); + assertHostTTL(Duration.ofHours(6), spec, "gamma", perf, null); + assertHostTTL(Duration.ofHours(6), spec, "gamma", prod, "us-east"); + assertHostTTL(Duration.ofHours(6), spec, "gamma", prod, "us-west"); + + assertHostTTL(Duration.ofHours(1), spec, "nope", test, null); + assertHostTTL(Duration.ofHours(1), spec, "nope", staging, null); + assertHostTTL(Duration.ofHours(1), spec, "nope", dev, null); + assertHostTTL(Duration.ofHours(1), spec, "nope", perf, null); + assertHostTTL(Duration.ofHours(1), spec, "nope", prod, "us-east"); + assertHostTTL(Duration.ofHours(1), spec, "nope", prod, "us-west"); + + assertEquals("Host TTL can only be specified with custom cloud accounts", + assertThrows(IllegalArgumentException.class, + () -> DeploymentSpec.fromXml(""" + <deployment version='1.0' empty-host-ttl='0m'> + <instance id='main'> + <prod> + <region>us-east-1</region> + </prod> + </instance> + </deployment> + """)) + .getMessage()); + + assertEquals("Host TTL can only be specified with custom cloud accounts", + assertThrows(IllegalArgumentException.class, + () -> DeploymentSpec.fromXml(""" + <deployment version='1.0'> + <instance id='main' empty-host-ttl='0m'> + <prod> + <region>us-east-1</region> + </prod> + </instance> + </deployment> + """)) + .getMessage()); + + assertEquals("Host TTL can only be specified with custom cloud accounts", + assertThrows(IllegalArgumentException.class, + () -> DeploymentSpec.fromXml(""" + <deployment version='1.0'> + <instance id='main'> + <prod empty-host-ttl='0m'> + <region>us-east-1</region> + </prod> + </instance> + </deployment> + """)) + .getMessage()); + + assertEquals("Host TTL can only be specified with custom cloud accounts", + assertThrows(IllegalArgumentException.class, + () -> DeploymentSpec.fromXml(""" + <deployment version='1.0'> + <instance id='main'> + <prod> + <region empty-host-ttl='0m'>us-east-1</region> + </prod> + </instance> + </deployment> + """)) + .getMessage()); + } private void assertCloudAccount(String expected, DeploymentInstanceSpec instance, Environment environment, String region) { assertEquals(Optional.of(expected).map(CloudAccount::from), instance.cloudAccount(environment, Optional.of(region).filter(s -> !s.isEmpty()).map(RegionName::from))); } + private void assertHostTTL(Duration expected, DeploymentSpec spec, String instance, Environment environment, String region) { + assertEquals(Optional.of(expected), spec.instance(InstanceName.from(instance)) + .flatMap(iSpec -> iSpec.hostTTL(environment, Optional.ofNullable(region).map(RegionName::from))) + .or(spec::hostTTL)); + } + private static void assertInvalid(String deploymentSpec, String errorMessagePart) { assertInvalid(deploymentSpec, errorMessagePart, new ManualClock()); } 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 38410cc5b37..ed8074ddc61 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 @@ -25,6 +25,9 @@ import java.util.stream.Collectors; import static com.yahoo.config.application.api.Notifications.Role.author; import static com.yahoo.config.application.api.Notifications.When.failing; import static com.yahoo.config.application.api.Notifications.When.failingCommit; +import static com.yahoo.config.provision.Environment.dev; +import static com.yahoo.config.provision.Environment.prod; +import static com.yahoo.config.provision.Environment.test; import static com.yahoo.config.provision.zone.ZoneId.defaultId; import static com.yahoo.config.provision.zone.ZoneId.from; import static org.junit.Assert.assertEquals; @@ -49,11 +52,11 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals(specXml, spec.xmlForm()); assertEquals(1, spec.steps().size()); assertFalse(spec.majorVersion().isPresent()); - assertTrue(spec.steps().get(0).concerns(Environment.test)); - assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.empty())); - assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region + assertTrue(spec.steps().get(0).concerns(test)); + assertTrue(spec.requireInstance("default").concerns(test, Optional.empty())); + assertTrue(spec.requireInstance("default").concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); - assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty())); + assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @@ -83,9 +86,9 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals(1, spec.steps().size()); assertEquals(1, spec.requireInstance("default").steps().size()); assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.staging)); - assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty())); + assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); assertTrue(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); - assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty())); + assertFalse(spec.requireInstance("default").concerns(prod, Optional.empty())); assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @@ -104,17 +107,17 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals(1, spec.steps().size()); assertEquals(2, spec.requireInstance("default").steps().size()); - assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.requireInstance("default").steps().get(0).concerns(prod, Optional.of(RegionName.from("us-east1")))); assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(0)).active()); - assertTrue(spec.requireInstance("default").steps().get(1).concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(spec.requireInstance("default").steps().get(1).concerns(prod, Optional.of(RegionName.from("us-west1")))); assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(1)).active()); - assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty())); + assertFalse(spec.requireInstance("default").concerns(test, Optional.empty())); assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); - assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("no-such-region")))); + assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-west1")))); + assertFalse(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("no-such-region")))); assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy()); @@ -139,25 +142,25 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals(5, spec.requireInstance("default").steps().size()); assertEquals(4, spec.requireInstance("default").zones().size()); - assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.test)); + assertTrue(spec.requireInstance("default").steps().get(0).concerns(test)); assertTrue(spec.requireInstance("default").steps().get(1).concerns(Environment.staging)); - assertTrue(spec.requireInstance("default").steps().get(2).concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.requireInstance("default").steps().get(2).concerns(prod, Optional.of(RegionName.from("us-east1")))); assertFalse(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(2)).active()); assertTrue(spec.requireInstance("default").steps().get(3) instanceof DeploymentSpec.Delay); assertEquals(3 * 60 * 60 + 30 * 60, spec.requireInstance("default").steps().get(3).delay().getSeconds()); - assertTrue(spec.requireInstance("default").steps().get(4).concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); + assertTrue(spec.requireInstance("default").steps().get(4).concerns(prod, Optional.of(RegionName.from("us-west1")))); assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(4)).active()); - assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.empty())); - assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region + assertTrue(spec.requireInstance("default").concerns(test, Optional.empty())); + assertTrue(spec.requireInstance("default").concerns(test, Optional.of(RegionName.from("region1")))); // test steps specify no region assertTrue(spec.requireInstance("default").concerns(Environment.staging, Optional.empty())); - assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-east1")))); - assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-west1")))); - assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("no-such-region")))); + assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-east1")))); + assertTrue(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("us-west1")))); + assertFalse(spec.requireInstance("default").concerns(prod, Optional.of(RegionName.from("no-such-region")))); assertFalse(spec.requireInstance("default").globalServiceId().isPresent()); } @@ -436,9 +439,9 @@ public class DeploymentSpecWithoutInstanceTest { List<DeploymentSpec.Step> innerParallelSteps = secondSerialSteps.get(2).steps(); assertEquals(3, innerParallelSteps.size()); assertEquals("prod.ap-northeast-1", innerParallelSteps.get(0).toString()); - assertEquals("no-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("ap-northeast-1")).get().value()); + assertEquals("no-service", spec.requireInstance("default").athenzService(prod, RegionName.from("ap-northeast-1")).get().value()); assertEquals("prod.ap-southeast-2", innerParallelSteps.get(1).toString()); - assertEquals("service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("ap-southeast-2")).get().value()); + assertEquals("service", spec.requireInstance("default").athenzService(prod, RegionName.from("ap-southeast-2")).get().value()); assertEquals("tests for prod.aws-us-east-1a", innerParallelSteps.get(2).toString()); } @@ -534,7 +537,7 @@ public class DeploymentSpecWithoutInstanceTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals(spec.athenzDomain().get().value(), "domain"); - assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); + assertEquals(spec.requireInstance("default").athenzService(prod, RegionName.from("us-west-1")).get().value(), "service"); } @Test @@ -553,11 +556,11 @@ public class DeploymentSpecWithoutInstanceTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); assertEquals("service", spec.athenzService().get().value()); - assertEquals("prod-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-central-1")) + assertEquals("prod-service", spec.requireInstance("default").athenzService(prod, RegionName.from("us-central-1")) .get().value()); - assertEquals("prod-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")) + assertEquals("prod-service", spec.requireInstance("default").athenzService(prod, RegionName.from("us-west-1")) .get().value()); - assertEquals("prod-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-east-3")) + assertEquals("prod-service", spec.requireInstance("default").athenzService(prod, RegionName.from("us-east-3")) .get().value()); } @@ -575,9 +578,9 @@ public class DeploymentSpecWithoutInstanceTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("service", spec.athenzService().get().value()); assertEquals(spec.athenzDomain().get().value(), "domain"); - assertEquals(spec.requireInstance("default").athenzService(Environment.test, RegionName.from("us-east-1")).get().value(), "service"); + assertEquals(spec.requireInstance("default").athenzService(test, RegionName.from("us-east-1")).get().value(), "service"); assertEquals(spec.requireInstance("default").athenzService(Environment.staging, RegionName.from("us-north-1")).get().value(), "staging-service"); - assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service"); + assertEquals(spec.requireInstance("default").athenzService(prod, RegionName.from("us-west-1")).get().value(), "prod-service"); } @Test(expected = IllegalArgumentException.class) @@ -694,7 +697,7 @@ public class DeploymentSpecWithoutInstanceTest { assertEquals(List.of(RegionName.from("us-east")), spec.requireInstance("default").endpoints().get(0).regions()); - var zone = from(Environment.prod, RegionName.from("us-east")); + var zone = from(prod, RegionName.from("us-east")); assertEquals(ZoneEndpoint.defaultEndpoint, spec.zoneEndpoint(InstanceName.from("custom"), zone, ClusterSpec.Id.from("bax"))); assertEquals(ZoneEndpoint.defaultEndpoint, @@ -742,8 +745,8 @@ public class DeploymentSpecWithoutInstanceTest { DeploymentSpec spec = DeploymentSpec.fromXml(r); DeploymentInstanceSpec instance = spec.requireInstance("default"); assertEquals(Optional.of(CloudAccount.from("012345678912")), spec.cloudAccount()); - assertEquals(Optional.of(CloudAccount.from("219876543210")), instance.cloudAccount(Environment.prod, Optional.of(RegionName.from("us-east-1")))); - assertEquals(Optional.of(CloudAccount.from("012345678912")), instance.cloudAccount(Environment.prod, Optional.of(RegionName.from("us-west-1")))); + assertEquals(Optional.of(CloudAccount.from("219876543210")), instance.cloudAccount(prod, Optional.of(RegionName.from("us-east-1")))); + assertEquals(Optional.of(CloudAccount.from("012345678912")), instance.cloudAccount(prod, Optional.of(RegionName.from("us-west-1")))); assertEquals(Optional.of(CloudAccount.from("012345678912")), instance.cloudAccount(Environment.staging, Optional.empty())); r = new StringReader( @@ -756,8 +759,41 @@ public class DeploymentSpecWithoutInstanceTest { ); spec = DeploymentSpec.fromXml(r); assertEquals(Optional.empty(), spec.cloudAccount()); - assertEquals(Optional.of(CloudAccount.from("219876543210")), spec.requireInstance("default").cloudAccount(Environment.prod, Optional.of(RegionName.from("us-east-1")))); - assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(Environment.prod, Optional.of(RegionName.from("us-west-1")))); + assertEquals(Optional.of(CloudAccount.from("219876543210")), spec.requireInstance("default").cloudAccount(prod, Optional.of(RegionName.from("us-east-1")))); + assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(prod, Optional.of(RegionName.from("us-west-1")))); + } + + @Test + public void productionSpecWithHostTTL() { + String r = """ + <deployment version='1.0' cloud-account='012345678912' empty-host-ttl='1d'> + <prod> + <region empty-host-ttl='1m'>us-east-1</region> + <region>us-west-1</region> + </prod> + </deployment> + """; + DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals(Optional.of(Duration.ofDays(1)), spec.hostTTL()); + DeploymentInstanceSpec instance = spec.requireInstance("default"); + assertEquals(Optional.of(Duration.ofMinutes(1)), instance.hostTTL(prod, Optional.of(RegionName.from("us-east-1")))); + assertEquals(Optional.of(Duration.ofDays(1)), instance.hostTTL(prod, Optional.of(RegionName.from("us-west-1")))); + assertEquals(Optional.of(Duration.ofDays(1)), instance.hostTTL(test, Optional.empty())); + + r = """ + <deployment version='1.0' cloud-account='012345678912'> + <prod empty-host-ttl='1d'> + <region empty-host-ttl='1m'>us-east-1</region> + <region>us-west-1</region> + </prod> + </deployment> + """; + spec = DeploymentSpec.fromXml(r); + assertEquals(Optional.empty(), spec.hostTTL()); + instance = spec.requireInstance("default"); + assertEquals(Optional.of(Duration.ofMinutes(1)), instance.hostTTL(prod, Optional.of(RegionName.from("us-east-1")))); + assertEquals(Optional.of(Duration.ofDays(1)), instance.hostTTL(prod, Optional.of(RegionName.from("us-west-1")))); + assertEquals(Optional.empty(), instance.hostTTL(test, Optional.empty())); } private static Set<String> endpointRegions(String endpointId, DeploymentSpec spec) { diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java index 13d87b852e4..a9ee884e977 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelContext.java @@ -3,15 +3,18 @@ package com.yahoo.config.model; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AnyConfigProducer; import com.yahoo.config.model.producer.TreeConfigProducer; import com.yahoo.config.provision.ClusterInfo; +import com.yahoo.config.provision.ClusterInfo.Builder; import com.yahoo.vespa.model.VespaModel; import java.time.Duration; import java.util.Comparator; +import java.util.Optional; import java.util.stream.Stream; /** @@ -72,14 +75,20 @@ public final class ConfigModelContext { /** Returns a cluster info builder pre-populated with info known in this context. */ public ClusterInfo.Builder clusterInfo() { - var instance = getApplicationPackage().getDeploymentSpec().instance(properties().applicationId().instance()); - if ( ! instance.isPresent()) return new ClusterInfo.Builder(); + DeploymentSpec spec = getApplicationPackage().getDeploymentSpec(); + var instance = spec.instance(properties().applicationId().instance()); + ClusterInfo.Builder builder = new ClusterInfo.Builder(); + spec.hostTTL().ifPresent(builder::hostTTL); + if (instance.isEmpty()) return builder; + instance.get() + .hostTTL(deployState.zone().environment(), Optional.of(deployState.zone().region())) + .ifPresent(builder::hostTTL); var maxDeadline = instance.get().bcp().groups().stream() .filter(group -> group.memberRegions().contains(properties().zone().region())) .map(group -> group.deadline()) .min(Comparator.comparing(deadline -> deadline)) .orElse(Duration.ofMinutes(0)); - return new ClusterInfo.Builder().bcpDeadline(maxDeadline); + return builder.bcpDeadline(maxDeadline); } /** diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java index f3c214da6ec..d75634d8c55 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.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.config.provision; +import java.time.Duration; import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; @@ -35,6 +36,8 @@ public final class Capacity { if (max.smallerThan(min)) throw new IllegalArgumentException("The max capacity must be larger than the min capacity, but got min " + min + " and max " + max); + if (cloudAccount.isEmpty() && ! clusterInfo.hostTTL().isZero()) + throw new IllegalArgumentException("Cannot set hostTTL without a custom cloud account"); this.min = min; this.max = max; this.groupSize = groupSize; @@ -105,22 +108,11 @@ public final class Capacity { } public static Capacity from(ClusterResources resources, boolean required, boolean canFail) { - return from(resources, required, canFail, NodeType.tenant); + return from(resources, required, canFail, Duration.ZERO); } - // TODO: Remove after March 2023 - public static Capacity from(ClusterResources min, ClusterResources max, IntRange groupSize, boolean required, boolean canFail, Optional<CloudAccount> cloudAccount) { - return new Capacity(min, max, groupSize, required, canFail, NodeType.tenant, cloudAccount, ClusterInfo.empty()); - } - - // TODO: Remove after March 2023 - public static Capacity from(ClusterResources min, ClusterResources max, boolean required, boolean canFail) { - return new Capacity(min, max, IntRange.empty(), required, canFail, NodeType.tenant, Optional.empty(), ClusterInfo.empty()); - } - - // TODO: Remove after March 2023 - public static Capacity from(ClusterResources min, ClusterResources max, boolean required, boolean canFail, Optional<CloudAccount> cloudAccount) { - return new Capacity(min, max, IntRange.empty(), required, canFail, NodeType.tenant, cloudAccount, ClusterInfo.empty()); + public static Capacity from(ClusterResources resources, boolean required, boolean canFail, Duration hostTTL) { + return from(resources, required, canFail, NodeType.tenant, hostTTL); } public static Capacity from(ClusterResources min, ClusterResources max, IntRange groupSize, boolean required, boolean canFail, @@ -130,11 +122,11 @@ public final class Capacity { /** Creates this from a node type */ public static Capacity fromRequiredNodeType(NodeType type) { - return from(new ClusterResources(0, 1, NodeResources.unspecified()), true, false, type); + return from(new ClusterResources(0, 1, NodeResources.unspecified()), true, false, type, Duration.ZERO); } - private static Capacity from(ClusterResources resources, boolean required, boolean canFail, NodeType type) { - return new Capacity(resources, resources, IntRange.empty(), required, canFail, type, Optional.empty(), ClusterInfo.empty()); + private static Capacity from(ClusterResources resources, boolean required, boolean canFail, NodeType type, Duration hostTTL) { + return new Capacity(resources, resources, IntRange.empty(), required, canFail, type, Optional.empty(), new ClusterInfo.Builder().hostTTL(hostTTL).build()); } } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterInfo.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterInfo.java index fe8acb0c3c0..d9076557ac7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterInfo.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterInfo.java @@ -3,6 +3,8 @@ package com.yahoo.config.provision; import java.time.Duration; import java.util.Objects; +import static ai.vespa.validation.Validation.requireAtLeast; + /** * Auxiliary information about a cluster, provided by the config model to the node repo during a * capacity request. @@ -14,13 +16,18 @@ public class ClusterInfo { private static final ClusterInfo empty = new ClusterInfo.Builder().build(); private final Duration bcpDeadline; + private final Duration hostTTL; private ClusterInfo(Builder builder) { this.bcpDeadline = builder.bcpDeadline; + this.hostTTL = builder.hostTTL; + requireAtLeast(hostTTL, "host TTL", Duration.ZERO); } public Duration bcpDeadline() { return bcpDeadline; } + public Duration hostTTL() { return hostTTL; } + public static ClusterInfo empty() { return empty; } public boolean isEmpty() { return this.equals(empty); } @@ -30,28 +37,35 @@ public class ClusterInfo { if (o == this) return true; if ( ! (o instanceof ClusterInfo other)) return false; if ( ! other.bcpDeadline.equals(this.bcpDeadline)) return false; + if ( ! other.hostTTL.equals(this.hostTTL)) return false; return true; } @Override public int hashCode() { - return Objects.hash(bcpDeadline); + return Objects.hash(bcpDeadline, hostTTL); } @Override public String toString() { - return "cluster info: [bcp deadline: " + bcpDeadline + "]"; + return "cluster info: [bcp deadline: " + bcpDeadline + ", host TTL: " + hostTTL + "]"; } public static class Builder { - private Duration bcpDeadline = Duration.ofMinutes(0); + private Duration bcpDeadline = Duration.ZERO; + private Duration hostTTL = Duration.ZERO; public Builder bcpDeadline(Duration duration) { this.bcpDeadline = Objects.requireNonNull(duration); return this; } + public Builder hostTTL(Duration duration) { + this.hostTTL = Objects.requireNonNull(duration); + return this; + } + public ClusterInfo build() { return new ClusterInfo(this); } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/CapacityTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/CapacityTest.java index a7614bbc016..b3d2e0afa7d 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/CapacityTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/CapacityTest.java @@ -3,9 +3,11 @@ package com.yahoo.config.provision; import org.junit.jupiter.api.Test; +import java.time.Duration; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; /** @@ -35,20 +37,23 @@ public class CapacityTest { new ClusterResources(4, 2, new NodeResources(1, 2, 3, 4))); assertValidationFailure(new ClusterResources(4, 2, new NodeResources(1, 2, 3, 5)), new ClusterResources(4, 2, new NodeResources(1, 2, 3, 4))); - // It's enough than one dimension is smaller also when the others are larger + // It's enough that one dimension is smaller also when the others are larger assertValidationFailure(new ClusterResources(4, 2, new NodeResources(1, 2, 3, 4)), new ClusterResources(8, 4, new NodeResources(2, 1, 6, 8))); + + assertEquals("Cannot set hostTTL without a custom cloud account", + assertThrows(IllegalArgumentException.class, + () -> Capacity.from(new ClusterResources(4, 2, new NodeResources(1, 2, 3, 4)), + new ClusterResources(4, 2, new NodeResources(1, 2, 3, 4)), + IntRange.empty(), false, true, Optional.empty(), new ClusterInfo.Builder().hostTTL(Duration.ofSeconds(1)).build())) + .getMessage()); } private void assertValidationFailure(ClusterResources min, ClusterResources max) { - try { - Capacity.from(min, max, IntRange.empty(), false, true, Optional.empty(), ClusterInfo.empty()); - fail("Expected exception with min " + min + " and max " + max); - } - catch (IllegalArgumentException e) { - assertEquals("The max capacity must be larger than the min capacity, but got min " + min + " and max " + max, - e.getMessage()); - } + assertEquals("The max capacity must be larger than the min capacity, but got min " + min + " and max " + max, + assertThrows(IllegalArgumentException.class, + () -> Capacity.from(min, max, IntRange.empty(), false, true, Optional.empty(), ClusterInfo.empty())) + .getMessage()); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java index afb854b2aaa..7d4cd6b8747 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackage.java @@ -5,7 +5,9 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.DeploymentSpec.Step; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; +import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; @@ -128,10 +130,7 @@ public class TestPackage { testerApp))); entries.put(deploymentFile, - __ -> new ByteArrayInputStream(deploymentXml(id.tester(), - spec.athenzDomain(), - spec.requireInstance(id.application().instance()) - .athenzService(id.type().zone().environment(), id.type().zone().region())))); + __ -> new ByteArrayInputStream(deploymentXml(id.tester(), id.application().instance(), id.type().zone(), spec))); if (certificate != null) { entries.put("artifacts/key", __ -> new ByteArrayInputStream(KeyUtils.toPem(keyPair.getPrivate()).getBytes(UTF_8))); @@ -297,13 +296,26 @@ public class TestPackage { } /** Returns a dummy deployment xml which sets up the service identity for the tester, if present. */ - static byte[] deploymentXml(TesterId id, Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService) { + static byte[] deploymentXml(TesterId id, InstanceName instance, ZoneId zone, DeploymentSpec original) { + Optional<AthenzDomain> athenzDomain = original.athenzDomain(); + Optional<AthenzService> athenzService = original.requireInstance(instance) + .athenzService(zone.environment(), zone.region()); + Optional<CloudAccount> cloudAccount = original.requireInstance(instance) + .cloudAccount(zone.environment(), Optional.of(zone.region())); + Optional<Duration> hostTTL = zone.environment().isProduction() + ? original.requireInstance(instance) + .steps().stream().filter(step -> step.isTest() && step.concerns(zone.environment(), Optional.of(zone.region()))) + .findFirst().flatMap(Step::hostTTL) + : original.requireInstance(instance).hostTTL(zone.environment(), Optional.of(zone.region())); String deploymentSpec = "<?xml version='1.0' encoding='UTF-8'?>\n" + - "<deployment version=\"1.0\" " + - athenzDomain.map(domain -> "athenz-domain=\"" + domain.value() + "\" ").orElse("") + - athenzService.map(service -> "athenz-service=\"" + service.value() + "\" ").orElse("") + ">" + - " <instance id=\"" + id.id().instance().value() + "\" />" + + "<deployment version='1.0'" + + athenzDomain.map(domain -> " athenz-domain='" + domain.value() + "'").orElse("") + + athenzService.map(service -> " athenz-service='" + service.value() + "'").orElse("") + + cloudAccount.map(account -> " cloud-account='" + account.value() + "'").orElse("") + + hostTTL.map(ttl -> " empty-host-ttl='" + ttl.getSeconds() / 60 + "m'").orElse("") + + ">" + + " <instance id='" + id.id().instance().value() + "' />" + "</deployment>"; return deploymentSpec.getBytes(UTF_8); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java index 6da8db1c259..2bfd081914b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java @@ -2,10 +2,12 @@ package com.yahoo.vespa.hosted.controller.application.pkg; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.application.pkg.TestPackage.TestSummary; import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.config.ControllerConfig.Steprunner.Testerapp; @@ -19,6 +21,9 @@ import java.nio.file.Paths; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.BiConsumer; +import java.util.function.Function; +import java.util.function.UnaryOperator; import java.util.jar.JarOutputStream; import java.util.zip.ZipEntry; @@ -147,28 +152,61 @@ public class TestPackageTest { } @Test + void generates_correct_deployment_spec() { + DeploymentSpec spec = DeploymentSpec.fromXml(""" + <deployment version='1.0' athenz-domain='domain' athenz-service='service' cloud-account='123123123123' empty-host-ttl='1h'> + <test empty-host-ttl='1d' /> + <staging cloud-account='321321321321'/> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + <region>us-west-1</region> + <test empty-host-ttl='0m'>us-west-1</test> + <region empty-host-ttl='1d'>us-central-1</region> + <test>us-central-1</test> + </prod> + </deployment> + """); + verifyAttributes("123123123123", 1440, ZoneId.from("test", "us-east-1"), spec); + verifyAttributes("321321321321", 60, ZoneId.from("staging", "us-east-2"), spec); + verifyAttributes("123123123123", 60, ZoneId.from("prod", "us-east-3"), spec); + verifyAttributes("123123123123", 0, ZoneId.from("prod", "us-west-1"), spec); + verifyAttributes("123123123123", 60, ZoneId.from("prod", "us-central-1"), spec); + } + + private void verifyAttributes(String expectedAccount, int expectedTTL, ZoneId zone, DeploymentSpec spec) { + assertEquals("<?xml version='1.0' encoding='UTF-8'?>\n" + + "<deployment version='1.0' athenz-domain='domain' athenz-service='service' " + + "cloud-account='" + expectedAccount + "' empty-host-ttl='" + expectedTTL + "m'> " + + "<instance id='default-t' /></deployment>", + new String(TestPackage.deploymentXml(TesterId.of(ApplicationId.defaultId()), InstanceName.defaultName(), zone, spec))); + } + + @Test void generates_correct_tester_flavor() { - DeploymentSpec spec = DeploymentSpec.fromXml("<deployment version='1.0' athenz-domain='domain' athenz-service='service'>\n" + - " <instance id='first'>\n" + - " <test tester-flavor=\"d-6-16-100\" />\n" + - " <prod>\n" + - " <region active=\"true\">us-west-1</region>\n" + - " <test>us-west-1</test>\n" + - " </prod>\n" + - " </instance>\n" + - " <instance id='second'>\n" + - " <test />\n" + - " <staging />\n" + - " <prod tester-flavor=\"d-6-16-100\">\n" + - " <parallel>\n" + - " <region active=\"true\">us-east-3</region>\n" + - " <region active=\"true\">us-central-1</region>\n" + - " </parallel>\n" + - " <region active=\"true\">us-west-1</region>\n" + - " <test>us-west-1</test>\n" + - " </prod>\n" + - " </instance>\n" + - "</deployment>\n"); + DeploymentSpec spec = DeploymentSpec.fromXml(""" + <deployment version='1.0' athenz-domain='domain' athenz-service='service'> + <instance id='first'> + <test tester-flavor="d-6-16-100" /> + <prod> + <region active="true">us-west-1</region> + <test>us-west-1</test> + </prod> + </instance> + <instance id='second'> + <test /> + <staging /> + <prod tester-flavor="d-6-16-100"> + <parallel> + <region active="true">us-east-3</region> + <region active="true">us-central-1</region> + </parallel> + <region active="true">us-west-1</region> + <test>us-west-1</test> + </prod> + </instance> + </deployment> + """); NodeResources firstResources = TestPackage.testerResourcesFor(ZoneId.from("prod", "us-west-1"), spec.requireInstance("first")); assertEquals(TestPackage.DEFAULT_TESTER_RESOURCES, firstResources); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index 3b518728607..20bd0be18b6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -23,6 +23,7 @@ import com.yahoo.vespa.hosted.provision.node.Reports; import com.yahoo.vespa.hosted.provision.node.Status; import com.yahoo.vespa.hosted.provision.node.TrustStoreItem; +import java.time.Duration; import java.time.Instant; import java.util.Arrays; import java.util.EnumSet; @@ -54,6 +55,8 @@ public final class Node implements Nodelike { private final Optional<String> modelName; private final Optional<TenantName> reservedTo; private final Optional<ApplicationId> exclusiveToApplicationId; + private final Optional<Duration> hostTTL; + private final Optional<Instant> hostEmptyAt; private final Optional<ClusterSpec.Type> exclusiveToClusterType; private final Optional<String> switchHostname; private final List<TrustStoreItem> trustStoreItems; @@ -87,11 +90,11 @@ public final class Node implements Nodelike { /** DO NOT USE: public for serialization purposes. See {@code create} helper methods. */ public Node(String id, IP.Config ipConfig, String hostname, Optional<String> parentHostname, - Flavor flavor, Status status, State state, Optional<Allocation> allocation, History history, NodeType type, - Reports reports, Optional<String> modelName, Optional<TenantName> reservedTo, - Optional<ApplicationId> exclusiveToApplicationId, Optional<ClusterSpec.Type> exclusiveToClusterType, - Optional<String> switchHostname, List<TrustStoreItem> trustStoreItems, - CloudAccount cloudAccount, Optional<WireguardKey> wireguardPubKey) { + Flavor flavor, Status status, State state, Optional<Allocation> allocation, History history, + NodeType type, Reports reports, Optional<String> modelName, Optional<TenantName> reservedTo, + Optional<ApplicationId> exclusiveToApplicationId, Optional<Duration> hostTTL, Optional<Instant> hostEmptyAt, + Optional<ClusterSpec.Type> exclusiveToClusterType, Optional<String> switchHostname, + List<TrustStoreItem> trustStoreItems, CloudAccount cloudAccount, Optional<WireguardKey> wireguardPubKey) { this.id = Objects.requireNonNull(id, "A node must have an ID"); this.hostname = requireNonEmptyString(hostname, "A node must have a hostname"); this.ipConfig = Objects.requireNonNull(ipConfig, "A node must a have an IP config"); @@ -106,6 +109,8 @@ public final class Node implements Nodelike { this.modelName = Objects.requireNonNull(modelName, "A null modelName is not permitted"); this.reservedTo = Objects.requireNonNull(reservedTo, "reservedTo cannot be null"); this.exclusiveToApplicationId = Objects.requireNonNull(exclusiveToApplicationId, "exclusiveToApplicationId cannot be null"); + this.hostTTL = Objects.requireNonNull(hostTTL, "hostTTL cannot be null"); + this.hostEmptyAt = Objects.requireNonNull(hostEmptyAt, "hostEmptyAt cannot be null"); this.exclusiveToClusterType = Objects.requireNonNull(exclusiveToClusterType, "exclusiveToClusterType cannot be null"); this.switchHostname = requireNonEmptyString(switchHostname, "switchHostname cannot be null"); this.trustStoreItems = Objects.requireNonNull(trustStoreItems).stream().distinct().toList(); @@ -133,6 +138,9 @@ public final class Node implements Nodelike { if (type != NodeType.host && exclusiveToApplicationId.isPresent()) throw new IllegalArgumentException("Only tenant hosts can be exclusive to an application"); + if (type != NodeType.host && hostTTL.isPresent()) + throw new IllegalArgumentException("Only tenant hosts can have a TTL"); + if (type != NodeType.host && exclusiveToClusterType.isPresent()) throw new IllegalArgumentException("Only tenant hosts can be exclusive to a cluster type"); } @@ -212,6 +220,19 @@ public final class Node implements Nodelike { public Optional<ApplicationId> exclusiveToApplicationId() { return exclusiveToApplicationId; } /** + * Returns the additional time to live of tenant host, in a dynamically provisioned zone, after all its child + * nodes are removed, before being deprovisioned, if any. + * This is set during provisioning and applies for the entire lifetime of the host. + */ + public Optional<Duration> hostTTL() { return hostTTL; } + + /** + * Returns the time at which a tenant host became empty, i.e., no longer had any child nodes allocated. + * This is used with {@link #hostTTL} to determine when to deprovision a tenant host in a dynamically provisioned zone. + */ + public Optional<Instant> hostEmptyAt() { return hostEmptyAt; } + + /** * Returns the cluster type this host is exclusive to, if any. Only tenant hosts can be exclusive to a cluster type. * If this is set, resources on this host cannot be allocated to any other cluster type. This is set during * provisioning and applies for the entire lifetime of the host @@ -330,15 +351,15 @@ public final class Node implements Nodelike { /** Returns a node with the status assigned to the given value */ public Node with(Status status) { return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, type, - reports, modelName, reservedTo, exclusiveToApplicationId, exclusiveToClusterType, switchHostname, - trustStoreItems, cloudAccount, wireguardPubKey); + reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a node with the type assigned to the given value */ public Node with(NodeType type) { return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, type, - reports, modelName, reservedTo, exclusiveToApplicationId, exclusiveToClusterType, switchHostname, - trustStoreItems, cloudAccount, wireguardPubKey); + reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a node with the flavor assigned to the given value */ @@ -346,37 +367,36 @@ public final class Node implements Nodelike { if (flavor.equals(this.flavor)) return this; History updateHistory = history.with(new History.Event(History.Event.Type.resized, agent, instant)); return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, updateHistory, type, - reports, modelName, reservedTo, exclusiveToApplicationId, exclusiveToClusterType, switchHostname, - trustStoreItems, cloudAccount, wireguardPubKey); + reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this with the reboot generation set to generation */ public Node withReboot(Generation generation) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status.withReboot(generation), state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status.withReboot(generation), state, allocation, + history, type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this with given id set */ public Node withId(String id) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, + history, type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this with model name set to given value */ public Node withModelName(String modelName) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, Optional.of(modelName), reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, Optional.of(modelName), reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this with model name cleared */ public Node withoutModelName() { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, Optional.empty(), reservedTo, - exclusiveToApplicationId, exclusiveToClusterType, switchHostname, trustStoreItems, - cloudAccount, wireguardPubKey); + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, Optional.empty(), reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this with a history record saying it was detected to be down at this instant */ @@ -416,66 +436,75 @@ public final class Node implements Nodelike { * Do not use this to allocate a node. */ public Node with(Allocation allocation) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - Optional.of(allocation), history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, Optional.of(allocation), history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this node with IP config set to the given value. */ public Node with(IP.Config ipConfig) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this node with the parent hostname assigned to the given value. */ public Node withParentHostname(String parentHostname) { - return new Node(id, ipConfig, hostname, Optional.of(parentHostname), flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, Optional.of(parentHostname), flavor, status, state, allocation, + history, type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } public Node withReservedTo(TenantName tenant) { if (type != NodeType.host) throw new IllegalArgumentException("Only host nodes can be reserved, " + hostname + " has type " + type); - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, Optional.of(tenant), exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, Optional.of(tenant), exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this node which is not reserved to a tenant */ public Node withoutReservedTo() { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, Optional.empty(), - exclusiveToApplicationId, exclusiveToClusterType, switchHostname, trustStoreItems, - cloudAccount, wireguardPubKey); + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, Optional.empty(), exclusiveToApplicationId, hostTTL, hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } public Node withExclusiveToApplicationId(ApplicationId exclusiveTo) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, Optional.ofNullable(exclusiveTo), + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, Optional.ofNullable(exclusiveTo), hostTTL, hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); + } + + public Node withHostTTL(Duration hostTTL) { + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, Optional.ofNullable(hostTTL), hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); + } + + public Node withHostEmptyAt(Instant hostEmptyAt) { + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, Optional.ofNullable(hostEmptyAt), exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } public Node withExclusiveToClusterType(ClusterSpec.Type exclusiveTo) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, Optional.ofNullable(exclusiveTo), switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } public Node withWireguardPubkey(WireguardKey wireguardPubkey) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, - exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, - Optional.ofNullable(wireguardPubkey)); + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, + exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, Optional.ofNullable(wireguardPubkey)); } /** Returns a copy of this node with switch hostname set to given value */ public Node withSwitchHostname(String switchHostname) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, - exclusiveToClusterType, Optional.ofNullable(switchHostname), trustStoreItems, cloudAccount, - wireguardPubKey); + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, + exclusiveToClusterType, Optional.ofNullable(switchHostname), trustStoreItems, cloudAccount, wireguardPubKey); } /** Returns a copy of this node with switch hostname unset */ @@ -526,20 +555,20 @@ public final class Node implements Nodelike { /** Returns a copy of this node with the given history. */ public Node with(History history) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } public Node with(Reports reports) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } public Node with(List<TrustStoreItem> trustStoreItems) { - return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, - allocation, history, type, reports, modelName, reservedTo, exclusiveToApplicationId, + return new Node(id, ipConfig, hostname, parentHostname, flavor, status, state, allocation, history, + type, reports, modelName, reservedTo, exclusiveToApplicationId, hostTTL, hostEmptyAt, exclusiveToClusterType, switchHostname, trustStoreItems, cloudAccount, wireguardPubKey); } @@ -673,6 +702,8 @@ public final class Node implements Nodelike { private String modelName; private TenantName reservedTo; private ApplicationId exclusiveToApplicationId; + private Duration hostTTL; + private Instant hostEmptyAt; private ClusterSpec.Type exclusiveToClusterType; private String switchHostname; private Allocation allocation; @@ -712,6 +743,16 @@ public final class Node implements Nodelike { return this; } + public Builder hostTTL(Duration hostTTL) { + this.hostTTL = hostTTL; + return this; + } + + public Builder hostEmptyAt(Instant hostEmptyAt) { + this.hostEmptyAt = hostEmptyAt; + return this; + } + public Builder exclusiveToClusterType(ClusterSpec.Type exclusiveTo) { this.exclusiveToClusterType = exclusiveTo; return this; @@ -772,9 +813,9 @@ public final class Node implements Nodelike { flavor, Optional.ofNullable(status).orElseGet(Status::initial), state, Optional.ofNullable(allocation), Optional.ofNullable(history).orElseGet(History::empty), type, Optional.ofNullable(reports).orElseGet(Reports::new), Optional.ofNullable(modelName), Optional.ofNullable(reservedTo), Optional.ofNullable(exclusiveToApplicationId), - Optional.ofNullable(exclusiveToClusterType), Optional.ofNullable(switchHostname), - Optional.ofNullable(trustStoreItems).orElseGet(List::of), - cloudAccount, Optional.ofNullable(wireguardPubKey)); + Optional.ofNullable(hostTTL), Optional.ofNullable(hostEmptyAt), Optional.ofNullable(exclusiveToClusterType), + Optional.ofNullable(switchHostname), Optional.ofNullable(trustStoreItems).orElseGet(List::of), cloudAccount, + Optional.ofNullable(wireguardPubKey)); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java index a4bc3a1aea5..1533780e694 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java @@ -34,15 +34,19 @@ import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; -import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; + +import static java.util.Comparator.comparing; +import static java.util.Comparator.naturalOrder; +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.toSet; /** * @author freva @@ -67,10 +71,9 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { @Override protected double maintain() { - NodeList nodes = nodeRepository().nodes().list(); - List<Node> excessHosts; + List<Node> provisionedSnapshot; try { - excessHosts = provision(nodes); + provisionedSnapshot = provision(nodeRepository().nodes().list()); } catch (NodeAllocationException | IllegalStateException e) { log.log(Level.WARNING, "Failed to allocate preprovisioned capacity and/or find excess hosts: " + e.getMessage()); return 0; // avoid removing excess hosts @@ -79,33 +82,68 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { return 0; // avoid removing excess hosts } - return markForRemoval(excessHosts); + return markForRemoval(provisionedSnapshot); } - private double markForRemoval(List<Node> excessHosts) { - if (excessHosts.isEmpty()) return 1; + private double markForRemoval(List<Node> provisionedSnapshot) { + // Group nodes by parent; no parent means it's a host. + Map<Optional<String>, List<Node>> nodesByParent = provisionedSnapshot.stream().collect(groupingBy(Node::parentHostname)); + + // Find all hosts that we once thought were empty (first clouse), or whose children are now all removable (second clause). + List<Node> emptyHosts = nodesByParent.get(Optional.<String>empty()).stream() + .filter(host -> host.hostEmptyAt().isPresent() + || nodesByParent.getOrDefault(Optional.of(host.hostname()), List.of()) + .stream().allMatch(HostCapacityMaintainer::canDeprovision)) + .toList(); + + if (emptyHosts.isEmpty()) return 1; int attempts = 0, success = 0; - for (List<Node> typeExcessHosts : excessHosts.stream().collect(Collectors.groupingBy(Node::type)).values()) { + for (Set<Node> typeEmptyHosts : emptyHosts.stream().collect(groupingBy(Node::type, toSet())).values()) { attempts++; // All nodes in the list are hosts of the same type, so they use the same lock regardless of their allocation - Optional<NodeMutex> appMutex = nodeRepository().nodes().lockAndGet(typeExcessHosts.get(0), Duration.ofSeconds(10)); + Optional<NodeMutex> appMutex = nodeRepository().nodes().lockAndGet(typeEmptyHosts.iterator().next(), Duration.ofSeconds(10)); if (appMutex.isEmpty()) continue; try (Mutex lock = appMutex.get(); Mutex unallocatedLock = nodeRepository().nodes().lockUnallocated()) { // Re-read all nodes under lock and compute the candidates for removal. The actual nodes we want - // to mark for removal is the intersection with typeExcessHosts - List<Node> toMarkForRemoval = candidatesForRemoval(nodeRepository().nodes().list().asList()).stream() - .filter(typeExcessHosts::contains) - .toList(); + // to mark for removal is the intersection with typeEmptyHosts, which excludes the preprovisioned hosts. + Map<Optional<String>, List<Node>> currentNodesByParent = nodeRepository().nodes().list().stream().collect(groupingBy(Node::parentHostname)); + List<Node> candidateHosts = new ArrayList<>(currentNodesByParent.get(Optional.<String>empty())); + candidateHosts.retainAll(typeEmptyHosts); - for (Node host : toMarkForRemoval) { + for (Node host : candidateHosts) { attempts++; - // Retire the host to parked if possible, otherwise move it straight to parked - if (EnumSet.of(Node.State.reserved, Node.State.active, Node.State.inactive).contains(host.state())) { - Node retiredHost = host.withWantToRetire(true, true, Agent.HostCapacityMaintainer, nodeRepository().clock().instant()); - nodeRepository().nodes().write(retiredHost, lock); - } else nodeRepository().nodes().park(host.hostname(), true, Agent.HostCapacityMaintainer, "Parked for removal"); + + // Any hosts that are no longer empty should be marked as such, and excluded from removal. + if (currentNodesByParent.getOrDefault(Optional.of(host.hostname()), List.of()) + .stream().anyMatch(n -> ! canDeprovision(n))) { + if (host.hostEmptyAt().isPresent()) { + nodeRepository().nodes().write(host.withHostEmptyAt(null), lock); + } + } + // If the host is still empty, we can mark it as empty now, or mark it for removal if it has already expired. + else { + Instant now = clock().instant(); + Node emptyHost = host.hostEmptyAt().isPresent() ? host : host.withHostEmptyAt(now); + boolean expired = ! now.isBefore(emptyHost.hostEmptyAt().get().plus(host.hostTTL().orElse(Duration.ZERO))); + + if (expired && canRemoveHost(emptyHost)) { + // Retire the host to parked if possible, otherwise move it straight to parked. + if (EnumSet.of(Node.State.reserved, Node.State.active, Node.State.inactive).contains(host.state())) { + emptyHost = emptyHost.withWantToRetire(true, true, Agent.HostCapacityMaintainer, now); + nodeRepository().nodes().write(emptyHost, lock); + } + else { + if (emptyHost != host) nodeRepository().nodes().write(emptyHost, lock); + nodeRepository().nodes().park(host.hostname(), true, Agent.HostCapacityMaintainer, "Parked for removal"); + } + } + else { + if (emptyHost != host) nodeRepository().nodes().write(emptyHost, lock); + } + } + success++; } } catch (UncheckedTimeoutException e) { @@ -116,35 +154,13 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { return asSuccessFactorDeviation(attempts, attempts - success); } - /** - * Provision hosts to ensure there is room to allocate spare nodes. - * - * @param nodeList list of all nodes - * @return excess hosts that can safely be deprovisioned: An excess host 1. contains no nodes allocated - * to an application, and assuming the spare nodes have been allocated, and 2. is not parked - * without wantToDeprovision (which means an operator is looking at the node). - */ private List<Node> provision(NodeList nodeList) { - var nodes = new ArrayList<>(provisionUntilNoDeficit(nodeList)); - return candidatesForRemoval(nodes).stream() - .sorted(Comparator.comparing(node -> node.history().events().stream() - .map(History.Event::at).min(Comparator.naturalOrder()).orElse(Instant.MIN))) - .toList(); - } - - private static List<Node> candidatesForRemoval(List<Node> nodes) { - Map<String, Node> removableHostsByHostname = new HashMap<>(); - for (var node : nodes) { - if (canRemoveHost(node)) { - removableHostsByHostname.put(node.hostname(), node); - } - } - for (var node : nodes) { - if (node.parentHostname().isPresent() && !canDeprovision(node)) { - removableHostsByHostname.remove(node.parentHostname().get()); - } - } - return List.copyOf(removableHostsByHostname.values()); + return provisionUntilNoDeficit(nodeList).stream() + .sorted(comparing(node -> node.history().events().stream() + .map(History.Event::at) + .min(naturalOrder()) + .orElse(Instant.MIN))) + .toList(); } private static boolean canRemoveHost(Node host) { @@ -197,9 +213,10 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { List<Integer> provisionIndices = nodeRepository().database().readProvisionIndices(count); List<Node> hosts = new ArrayList<>(); hostProvisioner.provisionHosts(provisionIndices, NodeType.host, nodeResources, ApplicationId.defaultId(), osVersion, - HostSharing.shared, Optional.empty(), Optional.empty(), nodeRepository().zone().cloud().account(), + HostSharing.shared, Optional.empty(), Optional.empty(), + nodeRepository().zone().cloud().account(), provisionedHosts -> { - hosts.addAll(provisionedHosts.stream().map(ProvisionedHost::generateHost).toList()); + hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(Duration.ZERO)).toList()); nodeRepository().nodes().addNodes(hosts, Agent.HostCapacityMaintainer); }); return hosts; @@ -246,7 +263,8 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { // build() requires a version, even though it is not (should not be) used .vespaVersion(Vtag.currentVersion) .build(); - NodeSpec nodeSpec = NodeSpec.from(clusterCapacity.count(), nodeResources, false, true, nodeRepository().zone().cloud().account()); + NodeSpec nodeSpec = NodeSpec.from(clusterCapacity.count(), nodeResources, false, true, + nodeRepository().zone().cloud().account(), Duration.ZERO); int wantedGroups = 1; NodePrioritizer prioritizer = new NodePrioritizer(allNodes, applicationId, clusterSpec, nodeSpec, wantedGroups, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java index 1e378c80f90..f62e019e408 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializer.java @@ -58,6 +58,7 @@ public class ApplicationSerializer { private static final String suggestedKey = "suggested"; private static final String clusterInfoKey = "clusterInfo"; private static final String bcpDeadlineKey = "bcpDeadline"; + private static final String hostTTLKey = "hostTTL"; private static final String bcpGroupInfoKey = "bcpGroupInfo"; private static final String queryRateKey = "queryRateKey"; private static final String growthRateHeadroomKey = "growthRateHeadroomKey"; @@ -234,12 +235,14 @@ public class ApplicationSerializer { private static void toSlime(ClusterInfo clusterInfo, Cursor clusterInfoObject) { clusterInfoObject.setLong(bcpDeadlineKey, clusterInfo.bcpDeadline().toMinutes()); + if ( ! clusterInfo.hostTTL().isZero()) clusterInfoObject.setLong(hostTTLKey, clusterInfo.hostTTL().toMillis()); } private static ClusterInfo clusterInfoFromSlime(Inspector clusterInfoObject) { if ( ! clusterInfoObject.valid()) return ClusterInfo.empty(); ClusterInfo.Builder builder = new ClusterInfo.Builder(); builder.bcpDeadline(Duration.ofMinutes(clusterInfoObject.field(bcpDeadlineKey).asLong())); + builder.hostTTL(Duration.ofMillis(clusterInfoObject.field(hostTTLKey).asLong())); return builder.build(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDb.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDb.java index cec413cf4e3..fc008b7b9dc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDb.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDb.java @@ -206,8 +206,8 @@ public class CuratorDb { toState.isAllocated() ? node.allocation() : Optional.empty(), node.history().recordStateTransition(node.state(), toState, agent, clock.instant()), node.type(), node.reports(), node.modelName(), node.reservedTo(), - node.exclusiveToApplicationId(), node.exclusiveToClusterType(), node.switchHostname(), - node.trustedCertificates(), node.cloudAccount(), node.wireguardPubKey()); + node.exclusiveToApplicationId(), node.hostTTL(), node.hostEmptyAt(), node.exclusiveToClusterType(), + node.switchHostname(), node.trustedCertificates(), node.cloudAccount(), node.wireguardPubKey()); curatorTransaction.add(createOrSet(nodePath(newNode), nodeSerializer.toJson(newNode))); writtenNodes.add(newNode); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index 23ea14da4cc..b176d0ce765 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -96,6 +96,8 @@ public class NodeSerializer { private static final String modelNameKey = "modelName"; private static final String reservedToKey = "reservedTo"; private static final String exclusiveToApplicationIdKey = "exclusiveTo"; + private static final String hostTTLKey = "hostTTL"; + private static final String hostEmptyAtKey = "hostEmptyAt"; private static final String exclusiveToClusterTypeKey = "exclusiveToClusterType"; private static final String switchHostnameKey = "switchHostname"; private static final String trustedCertificatesKey = "trustedCertificates"; @@ -194,6 +196,8 @@ public class NodeSerializer { node.modelName().ifPresent(modelName -> object.setString(modelNameKey, modelName)); node.reservedTo().ifPresent(tenant -> object.setString(reservedToKey, tenant.value())); node.exclusiveToApplicationId().ifPresent(applicationId -> object.setString(exclusiveToApplicationIdKey, applicationId.serializedForm())); + node.hostTTL().ifPresent(hostTTL -> object.setLong(hostTTLKey, hostTTL.toMillis())); + node.hostEmptyAt().ifPresent(emptyAt -> object.setLong(hostEmptyAtKey, emptyAt.toEpochMilli())); node.exclusiveToClusterType().ifPresent(clusterType -> object.setString(exclusiveToClusterTypeKey, clusterType.name())); trustedCertificatesToSlime(node.trustedCertificates(), object.setArray(trustedCertificatesKey)); if (!node.cloudAccount().isUnspecified()) { @@ -292,6 +296,8 @@ public class NodeSerializer { SlimeUtils.optionalString(object.field(modelNameKey)), SlimeUtils.optionalString(object.field(reservedToKey)).map(TenantName::from), SlimeUtils.optionalString(object.field(exclusiveToApplicationIdKey)).map(ApplicationId::fromSerializedForm), + SlimeUtils.optionalDuration(object.field(hostTTLKey)), + SlimeUtils.optionalInstant(object.field(hostEmptyAtKey)), SlimeUtils.optionalString(object.field(exclusiveToClusterTypeKey)).map(ClusterSpec.Type::from), SlimeUtils.optionalString(object.field(switchHostnameKey)), trustedCertificatesFromSlime(object), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 06c1916dd4f..5ae5a4f09d9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -100,13 +100,13 @@ public class GroupPreparer { NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get(); List<Node> hosts = new ArrayList<>(); Consumer<List<ProvisionedHost>> provisionedHostsConsumer = provisionedHosts -> { - hosts.addAll(provisionedHosts.stream().map(ProvisionedHost::generateHost).toList()); + hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(requestedNodes.hostTTL())).toList()); nodeRepository.nodes().addNodes(hosts, Agent.application); // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit List<NodeCandidate> candidates = provisionedHosts.stream() .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), - host.generateHost())) + host.generateHost(requestedNodes.hostTTL()))) .toList(); allocation.offer(candidates); }; @@ -114,8 +114,8 @@ public class GroupPreparer { try { hostProvisioner.get().provisionHosts( allocation.provisionIndices(deficit.count()), hostType, deficit.resources(), application, - osVersion, sharing, Optional.of(cluster.type()), Optional.of(cluster.id()), requestedNodes.cloudAccount(), - provisionedHostsConsumer); + osVersion, sharing, Optional.of(cluster.type()), Optional.of(cluster.id()), + requestedNodes.cloudAccount(), provisionedHostsConsumer); } catch (NodeAllocationException e) { // Mark the nodes that were written to ZK in the consumer for deprovisioning. While these hosts do // not exist, we cannot remove them from ZK here because other nodes may already have been diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java index ce48c5adab8..7c17c22334f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java @@ -11,6 +11,7 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; +import java.time.Duration; import java.util.List; import java.util.Optional; import java.util.Set; @@ -53,7 +54,7 @@ public interface HostProvisioner { * to be shared by multiple cluster nodes * @param cloudAccount the cloud account to use * @param provisionedHostConsumer consumer of {@link ProvisionedHost}s describing the provisioned nodes, - * the {@link Node} returned from {@link ProvisionedHost#generateHost()} must be + * the {@link Node} returned from {@link ProvisionedHost#generateHost} must be * written to ZK immediately in case the config server goes down while waiting * for the provisioning to finish. * @throws NodeAllocationException if the cloud provider cannot satisfy the request diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java index 57d859df476..1d4ab5fc09c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java @@ -38,8 +38,6 @@ public class NodePrioritizer { private final NameResolver nameResolver; private final Nodes nodes; private final boolean dynamicProvisioning; - /** Whether node specification allows new nodes to be allocated. */ - private final boolean canAllocateNew; private final boolean canAllocateToSpareHosts; private final boolean topologyChange; private final int currentClusterSize; @@ -81,9 +79,6 @@ public class NodePrioritizer { // NodeCandidate::compareTo will ensure that they will not be used until there is no room elsewhere. // In non-dynamically provisioned zones, we only allow allocating to spare hosts to replace failed nodes. this.canAllocateToSpareHosts = dynamicProvisioning || isReplacement(nodesInCluster, clusterSpec.group()); - // Do not allocate new nodes for exclusive deployments in dynamically provisioned zones: provision new host instead. - this.canAllocateNew = requestedNodes instanceof NodeSpec.CountNodeSpec - && (!dynamicProvisioning || !requestedNodes.isExclusive()); } /** Collects all node candidates for this application and returns them in the most-to-least preferred order */ @@ -136,14 +131,14 @@ public class NodePrioritizer { /** Add a node on each host with enough capacity for the requested flavor */ private void addCandidatesOnExistingHosts() { - if ( !canAllocateNew) return; + if (requestedNodes.resources().isEmpty()) return; for (Node host : allNodes) { if ( ! nodes.canAllocateTenantNodeTo(host, dynamicProvisioning)) continue; if (nodes.suspended(host)) continue; // Hosts that are suspended may be down for some time, e.g. for OS upgrade if (host.reservedTo().isPresent() && !host.reservedTo().get().equals(application.tenant())) continue; if (host.reservedTo().isPresent() && application.instance().isTester()) continue; - if (host.exclusiveToApplicationId().isPresent()) continue; // Never allocate new nodes to exclusive hosts + if (host.exclusiveToApplicationId().isPresent() && ! fitsPerfectly(host)) continue; if ( ! host.exclusiveToClusterType().map(clusterSpec.type()::equals).orElse(true)) continue; if (spareHosts.contains(host) && !canAllocateToSpareHosts) continue; if ( ! capacity.hasCapacity(host, requestedNodes.resources().get())) continue; @@ -159,6 +154,10 @@ public class NodePrioritizer { } } + private boolean fitsPerfectly(Node host) { + return host.resources().compatibleWith(requestedNodes.resources().get()); + } + /** Add existing nodes allocated to the application */ private void addApplicationNodes() { EnumSet<Node.State> legalStates = EnumSet.of(Node.State.active, Node.State.inactive, Node.State.reserved); 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 c8d20d89dfa..ffd2805bcff 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 @@ -38,7 +38,6 @@ import java.util.List; import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Stream; /** * Implementation of the host provisioner API for hosted Vespa, using the node repository to allocate nodes. @@ -101,7 +100,8 @@ public class NodeRepositoryProvisioner implements Provisioner { groups = target.groups(); resources = getNodeResources(cluster, target.nodeResources(), application); nodeSpec = NodeSpec.from(target.nodes(), resources, cluster.isExclusive(), actual.canFail(), - requested.cloudAccount().orElse(nodeRepository.zone().cloud().account())); + requested.cloudAccount().orElse(nodeRepository.zone().cloud().account()), + requested.clusterInfo().hostTTL()); } else { groups = 1; // type request with multiple groups is not supported diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index 28d1e7c1c68..bfe6f4211cb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; +import java.time.Duration; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -67,6 +68,9 @@ public interface NodeSpec { /** Returns the cloud account to use when fulfilling this spec */ CloudAccount cloudAccount(); + /** Returns the host TTL to use for any hosts provisioned as a result of this fulfilling this spec. */ + default Duration hostTTL() { return Duration.ZERO; } + /** * Returns true if a node with given current resources and current spare host resources can be resized * in-place to resources in this spec. @@ -76,8 +80,9 @@ public interface NodeSpec { return false; } - static NodeSpec from(int nodeCount, NodeResources resources, boolean exclusive, boolean canFail, CloudAccount cloudAccount) { - return new CountNodeSpec(nodeCount, resources, exclusive, canFail, canFail, cloudAccount); + static NodeSpec from(int nodeCount, NodeResources resources, boolean exclusive, boolean canFail, + CloudAccount cloudAccount, Duration hostTTL) { + return new CountNodeSpec(nodeCount, resources, exclusive, canFail, canFail, cloudAccount, hostTTL); } static NodeSpec from(NodeType type, CloudAccount cloudAccount) { @@ -93,14 +98,17 @@ public interface NodeSpec { private final boolean canFail; private final boolean considerRetiring; private final CloudAccount cloudAccount; + private final Duration hostTTL; - private CountNodeSpec(int count, NodeResources resources, boolean exclusive, boolean canFail, boolean considerRetiring, CloudAccount cloudAccount) { + private CountNodeSpec(int count, NodeResources resources, boolean exclusive, boolean canFail, + boolean considerRetiring, CloudAccount cloudAccount, Duration hostTTL) { this.count = count; this.requestedNodeResources = Objects.requireNonNull(resources, "Resources must be specified"); this.exclusive = exclusive; this.canFail = canFail; this.considerRetiring = considerRetiring; this.cloudAccount = Objects.requireNonNull(cloudAccount); + this.hostTTL = Objects.requireNonNull(hostTTL); if (!canFail && considerRetiring) throw new IllegalArgumentException("Cannot consider retiring nodes if we cannot fail"); @@ -145,11 +153,11 @@ public interface NodeSpec { @Override public NodeSpec fraction(int divisor) { - return new CountNodeSpec(count/divisor, requestedNodeResources, exclusive, canFail, considerRetiring, cloudAccount); + return new CountNodeSpec(count/divisor, requestedNodeResources, exclusive, canFail, considerRetiring, cloudAccount, hostTTL); } public NodeSpec withoutRetiring() { - return new CountNodeSpec(count, requestedNodeResources, exclusive, canFail, false, cloudAccount); + return new CountNodeSpec(count, requestedNodeResources, exclusive, canFail, false, cloudAccount, hostTTL); } @Override @@ -187,6 +195,9 @@ public interface NodeSpec { } @Override + public Duration hostTTL() { return hostTTL; } + + @Override public String toString() { return "request for " + count + " nodes with " + requestedNodeResources; } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java index 98afc6e7482..c9fd1d08759 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java @@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.node.OsVersion; import com.yahoo.vespa.hosted.provision.node.Status; +import java.time.Duration; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -39,7 +40,9 @@ public class ProvisionedHost { public ProvisionedHost(String id, String hostHostname, Flavor hostFlavor, NodeType hostType, Optional<ApplicationId> exclusiveToApplicationId, Optional<ClusterSpec.Type> exclusiveToClusterType, - List<HostName> nodeHostnames, NodeResources nodeResources, Version osVersion, CloudAccount cloudAccount) { + List<HostName> nodeHostnames, NodeResources nodeResources, + Version osVersion, CloudAccount cloudAccount) { + if (!hostType.isHost()) throw new IllegalArgumentException(hostType + " is not a host"); this.id = Objects.requireNonNull(id, "Host id must be set"); this.hostHostname = Objects.requireNonNull(hostHostname, "Host hostname must be set"); this.hostFlavor = Objects.requireNonNull(hostFlavor, "Host flavor must be set"); @@ -50,7 +53,6 @@ public class ProvisionedHost { this.nodeResources = Objects.requireNonNull(nodeResources, "Node resources must be set"); this.osVersion = Objects.requireNonNull(osVersion, "OS version must be set"); this.cloudAccount = Objects.requireNonNull(cloudAccount, "Cloud account must be set"); - if (!hostType.isHost()) throw new IllegalArgumentException(hostType + " is not a host"); } private static List<HostName> validateNodeAddresses(List<HostName> nodeHostnames) { @@ -62,13 +64,13 @@ public class ProvisionedHost { } /** Generate {@link Node} instance representing the provisioned physical host */ - public Node generateHost() { - Node.Builder builder = Node.create(id, IP.Config.of(Set.of(), Set.of(), nodeHostnames), hostHostname, hostFlavor, - hostType) + public Node generateHost(Duration hostTTL) { + Node.Builder builder = Node.create(id, IP.Config.of(Set.of(), Set.of(), nodeHostnames), hostHostname, hostFlavor, hostType) .status(Status.initial().withOsVersion(OsVersion.EMPTY.withCurrent(Optional.of(osVersion)))) .cloudAccount(cloudAccount); exclusiveToApplicationId.ifPresent(builder::exclusiveToApplicationId); exclusiveToClusterType.ifPresent(builder::exclusiveToClusterType); + if ( ! hostTTL.isZero()) builder.hostTTL(hostTTL); return builder.build(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java index 4dc48459ec9..5d1194f247e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java @@ -256,6 +256,10 @@ public class NodePatcher { case "exclusiveTo": case "exclusiveToApplicationId": return node.withExclusiveToApplicationId(SlimeUtils.optionalString(value).map(ApplicationId::fromSerializedForm).orElse(null)); + case "hostTTL": + return node.withHostTTL(SlimeUtils.optionalDuration(value).orElse(null)); + case "hostEmptyAt": + return node.withHostEmptyAt(SlimeUtils.optionalInstant(value).orElse(null)); case "exclusiveToClusterType": return node.withExclusiveToClusterType(SlimeUtils.optionalString(value).map(ClusterSpec.Type::valueOf).orElse(null)); case "switchHostname": diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java index 0e33e3461e7..bc6f9c9cca1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java @@ -133,6 +133,8 @@ class NodesResponse extends SlimeJsonResponse { object.setString("flavor", node.flavor().name()); node.reservedTo().ifPresent(reservedTo -> object.setString("reservedTo", reservedTo.value())); node.exclusiveToApplicationId().ifPresent(applicationId -> object.setString("exclusiveTo", applicationId.serializedForm())); + node.hostTTL().ifPresent(ttl -> object.setLong("hostTTL", ttl.toMillis())); + node.hostEmptyAt().ifPresent(emptyAt -> object.setLong("hostEmptyAt", emptyAt.toEpochMilli())); node.exclusiveToClusterType().ifPresent(clusterType -> object.setString("exclusiveToClusterType", clusterType.name())); if (node.flavor().isConfigured()) object.setDouble("cpuCores", node.flavor().resources().vcpu()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java index 24ea9361823..5e3cdaec216 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java @@ -20,6 +20,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner; import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator; import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost; +import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java index 7f5bb79b20c..d143657310d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java @@ -21,11 +21,14 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; +import com.yahoo.docproc.jdisc.metric.NullMetric; import com.yahoo.net.HostName; +import com.yahoo.test.ManualClock; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.flags.custom.ClusterCapacity; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.Node.State; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -37,6 +40,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; import com.yahoo.vespa.hosted.provision.provisioning.InfraDeployerImpl; import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost; import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester; +import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; import com.yahoo.vespa.hosted.provision.testutils.MockDuperModel; import com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; @@ -50,6 +54,7 @@ import org.junit.Test; import java.time.Duration; import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -60,6 +65,7 @@ import java.util.stream.Stream; import static com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner.Behaviour; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -497,15 +503,101 @@ public class HostCapacityMaintainerTest { } @Test + public void deprovision_node_when_no_allocation_and_past_TTL() { + var tester = new DynamicProvisioningTester(); + ManualClock clock = (ManualClock) tester.nodeRepository.clock(); + tester.hostProvisioner.with(Behaviour.failProvisioning); + tester.provisioningTester.makeReadyHosts(2, new NodeResources(1, 1, 1, 1)).activateTenantHosts(); + List<Node> hosts = tester.nodeRepository.nodes().list(Node.State.active).asList(); + Node host1 = hosts.get(0); + Node host2 = hosts.get(1); + tester.nodeRepository.nodes().write(host1.withHostTTL(Duration.ofDays(1)), () -> { }); + tester.nodeRepository.nodes().write(host2.withHostTTL(Duration.ofHours(1)), () -> { }); + Node host11 = tester.addNode("host1-1", Optional.of(host1.hostname()), NodeType.tenant, State.active, DynamicProvisioningTester.tenantApp); + + // Host is not marked for deprovisioning by maintainer, because child is present + tester.maintain(); + assertFalse(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToDeprovision()); + assertEquals(Optional.empty(), tester.nodeRepository.nodes().node(host1.hostname()).get().hostEmptyAt()); + + // Child is set to deprovision, but turns active + tester.nodeRepository.nodes().park(host11.hostname(), true, Agent.operator, "not good"); + tester.nodeRepository.nodes().reactivate(host11.hostname(), Agent.operator, "all good"); + assertTrue(tester.nodeRepository.nodes().node(host11.hostname()).get().status().wantToDeprovision()); + assertEquals(State.active, tester.nodeRepository.nodes().node(host11.hostname()).get().state()); + tester.maintain(); + assertFalse(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToDeprovision()); + assertEquals(Optional.empty(), tester.nodeRepository.nodes().node(host1.hostname()).get().hostEmptyAt()); + + // Child is parked, to make the host effectively empty + tester.nodeRepository.nodes().park(host11.hostname(), true, Agent.operator, "not good"); + tester.maintain(); + assertFalse(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToDeprovision()); + assertEquals(Optional.of(clock.instant().truncatedTo(ChronoUnit.MILLIS)), + tester.nodeRepository.nodes().node(host1.hostname()).get().hostEmptyAt()); + + // Some time passes, but not enough for host1 to be deprovisioned + clock.advance(Duration.ofDays(1).minusSeconds(1)); + tester.maintain(); + assertFalse(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToDeprovision()); + assertEquals(Optional.of(clock.instant().minus(Duration.ofDays(1).minusSeconds(1)).truncatedTo(ChronoUnit.MILLIS)), + tester.nodeRepository.nodes().node(host1.hostname()).get().hostEmptyAt()); + assertTrue(tester.nodeRepository.nodes().node(host2.hostname()).get().status().wantToDeprovision()); + assertTrue(tester.nodeRepository.nodes().node(host2.hostname()).get().status().wantToRetire()); + assertEquals(State.active, tester.nodeRepository.nodes().node(host2.hostname()).get().state()); + assertEquals(Optional.of(clock.instant().minus(Duration.ofDays(1).minusSeconds(1)).truncatedTo(ChronoUnit.MILLIS)), + tester.nodeRepository.nodes().node(host2.hostname()).get().hostEmptyAt()); + + // Some more time passes, but child is reactivated on host1, rendering the host non-empty again + clock.advance(Duration.ofDays(1)); + tester.nodeRepository.nodes().reactivate(host11.hostname(), Agent.operator, "all good"); + tester.maintain(); + assertFalse(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToDeprovision()); + assertEquals(Optional.empty(), tester.nodeRepository.nodes().node(host1.hostname()).get().hostEmptyAt()); + + // Child is removed, and host is marked as empty + tester.nodeRepository.database().writeTo(State.deprovisioned, host11, Agent.operator, Optional.empty()); + tester.nodeRepository.nodes().forget(tester.nodeRepository.nodes().node(host11.hostname()).get()); + assertEquals(Optional.empty(), tester.nodeRepository.nodes().node(host11.hostname())); + tester.maintain(); + assertFalse(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToDeprovision()); + assertEquals(Optional.of(clock.instant().truncatedTo(ChronoUnit.MILLIS)), + tester.nodeRepository.nodes().node(host1.hostname()).get().hostEmptyAt()); + + // Enough time passes for the host to be deprovisioned + clock.advance(Duration.ofDays(1)); + tester.maintain(); + assertTrue(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToDeprovision()); + assertTrue(tester.nodeRepository.nodes().node(host1.hostname()).get().status().wantToRetire()); + assertEquals(State.active, tester.nodeRepository.nodes().node(host1.hostname()).get().state()); + assertEquals(Optional.of(clock.instant().minus(Duration.ofDays(1)).truncatedTo(ChronoUnit.MILLIS)), + tester.nodeRepository.nodes().node(host1.hostname()).get().hostEmptyAt()); + + // Let tenant host app redeploy, retiring the obsolete host. + tester.provisioningTester.activateTenantHosts(); + clock.advance(Duration.ofHours(1)); + new RetiredExpirer(tester.nodeRepository, + new MockDeployer(tester.nodeRepository), + new NullMetric(), + Duration.ofHours(1), + Duration.ofHours(1)).maintain(); + + // Host and children can now be removed. + tester.provisioningTester.activateTenantHosts(); + tester.maintain(); + assertEquals(List.of(), tester.nodeRepository.nodes().list().asList()); + } + + @Test public void deprovision_parked_node_with_allocation() { var tester = new DynamicProvisioningTester(); tester.hostProvisioner.with(Behaviour.failProvisioning); - Node host4 = tester.addNode("host4", Optional.empty(), NodeType.host, Node.State.parked); + Node host4 = tester.addNode("host4", Optional.empty(), NodeType.host, Node.State.parked, null, Duration.ofDays(1)); Node host41 = tester.addNode("host4-1", Optional.of("host4"), NodeType.tenant, Node.State.parked, DynamicProvisioningTester.tenantApp); Node host42 = tester.addNode("host4-2", Optional.of("host4"), NodeType.tenant, Node.State.active, DynamicProvisioningTester.tenantApp); Node host43 = tester.addNode("host4-3", Optional.of("host4"), NodeType.tenant, Node.State.failed, DynamicProvisioningTester.tenantApp); - // Host and children are marked for deprovisioning + // Host and children are marked for deprovisioning, bypassing host TTL. tester.nodeRepository.nodes().deprovision("host4", Agent.operator, Instant.now()); for (var node : List.of(host4, host41, host42, host43)) { assertTrue(tester.nodeRepository.nodes().node(node.hostname()).map(n -> n.status().wantToDeprovision()).get()); @@ -522,7 +614,7 @@ public class HostCapacityMaintainerTest { // Last child is parked tester.nodeRepository.nodes().park(host42.hostname(), false, Agent.system, getClass().getSimpleName()); - // Host and children can now be removed + // Host and children can now be removed. tester.maintain(); for (var node : List.of(host4, host41, host42, host43)) { assertTrue(node.hostname() + " removed", tester.nodeRepository.nodes().node(node.hostname()).isEmpty()); @@ -633,17 +725,22 @@ public class HostCapacityMaintainerTest { return cfghost; } - private Node addNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state) { - return addNode(hostname, parentHostname, nodeType, state, null); + private Node addNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, ApplicationId application) { + return addNode(hostname, parentHostname, nodeType, state, application, null); } - private Node addNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, ApplicationId application) { - Node node = createNode(hostname, parentHostname, nodeType, state, application); + private Node addNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, ApplicationId application, Duration hostTTL) { + Node node = createNode(hostname, parentHostname, nodeType, state, application, hostTTL); return nodeRepository.database().addNodesInState(List.of(node), node.state(), Agent.system).get(0); } private Node createNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, ApplicationId application, String... additionalHostnames) { + return createNode(hostname, parentHostname, nodeType, state, application, null, additionalHostnames); + } + + private Node createNode(String hostname, Optional<String> parentHostname, NodeType nodeType, + Node.State state, ApplicationId application, Duration hostTTL, String... additionalHostnames) { Flavor flavor = nodeRepository.flavors().getFlavor(parentHostname.isPresent() ? "docker" : "host3").orElseThrow(); Optional<Allocation> allocation = Optional.ofNullable(application) .map(app -> new Allocation( @@ -654,7 +751,8 @@ public class HostCapacityMaintainerTest { false)); List<com.yahoo.config.provision.HostName> hostnames = Stream.of(additionalHostnames).map(com.yahoo.config.provision.HostName::of).toList(); Node.Builder builder = Node.create("fake-id-" + hostname, hostname, flavor, state, nodeType) - .ipConfig(IP.Config.of(state == Node.State.active ? Set.of("::1") : Set.of(), Set.of(), hostnames)); + .ipConfig(IP.Config.of(state == Node.State.active ? Set.of("::1") : Set.of(), Set.of(), hostnames)) + .hostTTL(hostTTL); parentHostname.ifPresent(builder::parentHostname); allocation.ifPresent(builder::allocation); if (hostname.equals("host2-1")) @@ -664,7 +762,7 @@ public class HostCapacityMaintainerTest { private long provisionedHostsMatching(NodeResources resources) { return hostProvisioner.provisionedHosts().stream() - .filter(host -> host.generateHost().resources().compatibleWith(resources)) + .filter(host -> host.generateHost(Duration.ZERO).resources().compatibleWith(resources)) .count(); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java index c429f88cfa1..c997da543ea 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/ApplicationSerializerTest.java @@ -68,7 +68,7 @@ public class ApplicationSerializerTest { Load.zero(), Load.one(), Autoscaling.Metrics.zero()), - new ClusterInfo.Builder().bcpDeadline(Duration.ofMinutes(33)).build(), + new ClusterInfo.Builder().bcpDeadline(Duration.ofMinutes(33)).hostTTL(Duration.ofSeconds(321)).build(), new BcpGroupInfo(0.1, 0.2, 0.3), List.of(new ScalingEvent(new ClusterResources(10, 5, minResources), new ClusterResources(12, 6, minResources), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java index 1086f2026a8..ccb13ea5ada 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java @@ -37,6 +37,7 @@ import org.junit.Test; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -476,13 +477,19 @@ public class NodeSerializerTest { nodeFlavors.getFlavorOrThrow("default"), NodeType.host); Node node = nodeSerializer.fromJson(nodeSerializer.toJson(builder.build())); assertFalse(node.exclusiveToApplicationId().isPresent()); + assertFalse(node.hostTTL().isPresent()); assertFalse(node.exclusiveToClusterType().isPresent()); ApplicationId exclusiveToApp = ApplicationId.from("tenant1", "app1", "instance1"); ClusterSpec.Type exclusiveToCluster = ClusterSpec.Type.admin; - node = builder.exclusiveToApplicationId(exclusiveToApp).exclusiveToClusterType(exclusiveToCluster).build(); + node = builder.exclusiveToApplicationId(exclusiveToApp) + .hostTTL(Duration.ofDays(1)) + .hostEmptyAt(clock.instant().minus(Duration.ofDays(1)).truncatedTo(MILLIS)) + .exclusiveToClusterType(exclusiveToCluster).build(); node = nodeSerializer.fromJson(nodeSerializer.toJson(node)); assertEquals(exclusiveToApp, node.exclusiveToApplicationId().get()); + assertEquals(Duration.ofDays(1), node.hostTTL().get()); + assertEquals(clock.instant().minus(Duration.ofDays(1)).truncatedTo(MILLIS), node.hostEmptyAt().get()); assertEquals(exclusiveToCluster, node.exclusiveToClusterType().get()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java index 382d2840377..d4a911fa1ab 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java @@ -21,6 +21,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.Node.State; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -31,6 +32,7 @@ import org.junit.Test; import java.time.Instant; import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -116,6 +118,55 @@ public class DynamicProvisioningTest { } @Test + public void empty_exclusive_to_hosts_reused_iff_new_allocation_fits_perfectly() { + var tester = tester(true); + + NodeResources highResources = new NodeResources(4, 80, 100, 1); + NodeResources lowResources = new NodeResources(2, 20, 50, 1); + + ApplicationId application = ProvisioningTester.applicationId(); + prepareAndActivate(application, clusterSpec("mycluster", true), 2, 1, highResources, tester); + + // Total of 4 nodes should now be in node-repo, 2 active hosts and 2 active nodes. + assertEquals(4, tester.nodeRepository().nodes().list().size()); + assertEquals(2, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.tenant).size()); + + // Redeploying the application causes no changes at all. + prepareAndActivate(application, clusterSpec("mycluster", true), 2, 1, highResources, tester); + assertEquals(4, tester.nodeRepository().nodes().list().size()); + assertEquals(2, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.tenant).size()); + + // Deploying with a smaller node flavour causes new, smaller hosts to be provisioned. + prepareAndActivate(application, clusterSpec("mycluster", true), 2, 1, lowResources, tester); + + // Total of 8 nodes should now be in node-repo, 4 active hosts and 4 active nodes, of which 2 are retired. + NodeList nodes = tester.nodeRepository().nodes().list(); + assertEquals(8, nodes.size()); + assertEquals(4, nodes.nodeType(NodeType.host).state(Node.State.active).size()); + assertEquals(4, nodes.nodeType(NodeType.tenant).state(Node.State.active).size()); + assertEquals(2, nodes.retired().size()); + + // Remove the child nodes, and redeploy with the original flavour. This should reuse the existing hosts. + tester.nodeRepository().database().writeTo(State.deprovisioned, nodes.retired().asList(), Agent.operator, Optional.empty()); + tester.nodeRepository().nodes().list().state(State.deprovisioned).forEach(tester.nodeRepository().nodes()::forget); + + // Total of 6 nodes should now be in node-repo, 4 active hosts and 2 active nodes. + nodes = tester.nodeRepository().nodes().list(); + assertEquals(6, nodes.size()); + assertEquals(4, nodes.nodeType(NodeType.host).state(Node.State.active).size()); + assertEquals(2, nodes.nodeType(NodeType.tenant).state(Node.State.active).size()); + assertEquals(0, nodes.retired().size()); + + // Deploy again with high resources. + prepareAndActivate(application, clusterSpec("mycluster", true), 2, 1, highResources, tester); + // Total of 8 nodes should now be in node-repo, 4 active hosts and 4 active nodes. + nodes = tester.nodeRepository().nodes().list(); + assertEquals(8, nodes.size()); + assertEquals(4, nodes.nodeType(NodeType.host).state(Node.State.active).size()); + assertEquals(4, nodes.nodeType(NodeType.tenant).state(Node.State.active).size()); + } + + @Test public void avoids_allocating_to_empty_hosts() { var tester = tester(false); tester.makeReadyHosts(6, new NodeResources(12, 12, 200, 12)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index 3107d9738a9..f0794fdc4a1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -86,7 +86,7 @@ public class ProvisioningTest { // deploy another application SystemState state1App2 = prepare(application2, 2, 2, 3, 3, defaultResources, tester); - assertFalse("Hosts to different apps are disjunct", state1App2.allHosts.removeAll(state1.allHosts)); + assertFalse("Hosts to different apps are disjoint", state1App2.allHosts.removeAll(state1.allHosts)); tester.activate(application2, state1App2.allHosts); // prepare twice diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java index 022822fd3ec..40895e25f2f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java @@ -1016,15 +1016,25 @@ public class NodesV2ApiTest { assertResponse(new Request(url, Utf8.toBytes("{\"exclusiveToApplicationId\": \"t1:a1:i1\"}"), Request.Method.PATCH), "{\"message\":\"Updated dockerhost1.yahoo.com\"}"); - tester.assertPartialResponse(new Request(url), "exclusiveTo\":\"t1:a1:i1\",", true); + tester.assertPartialResponse(new Request(url), "\"exclusiveTo\":\"t1:a1:i1\",", true); + + assertResponse(new Request(url, Utf8.toBytes("{\"hostTTL\": 86400000}"), Request.Method.PATCH), + "{\"message\":\"Updated dockerhost1.yahoo.com\"}"); + tester.assertPartialResponse(new Request(url), "\"hostTTL\":86400000", true); + + assertResponse(new Request(url, Utf8.toBytes("{\"hostEmptyAt\": 789}"), Request.Method.PATCH), + "{\"message\":\"Updated dockerhost1.yahoo.com\"}"); + tester.assertPartialResponse(new Request(url), "\"hostEmptyAt\":789", true); assertResponse(new Request(url, Utf8.toBytes("{\"exclusiveToClusterType\": \"admin\"}"), Request.Method.PATCH), "{\"message\":\"Updated dockerhost1.yahoo.com\"}"); - tester.assertPartialResponse(new Request(url), "exclusiveTo\":\"t1:a1:i1\",\"exclusiveToClusterType\":\"admin\",", true); + tester.assertPartialResponse(new Request(url), "\"exclusiveToClusterType\":\"admin\",", true); - assertResponse(new Request(url, Utf8.toBytes("{\"exclusiveTo\": null, \"exclusiveToClusterType\": null}"), Request.Method.PATCH), + assertResponse(new Request(url, Utf8.toBytes("{\"exclusiveTo\": null, \"hostTTL\":null, \"hostEmptyAt\":null, \"exclusiveToClusterType\": null}"), Request.Method.PATCH), "{\"message\":\"Updated dockerhost1.yahoo.com\"}"); - tester.assertPartialResponse(new Request(url), "exclusiveTo", false); + tester.assertPartialResponse(new Request(url), "\"exclusiveTo", false); + tester.assertPartialResponse(new Request(url), "\"hostTTL\"", false); + tester.assertPartialResponse(new Request(url), "\"hostEmptyAt\"", false); } @Test diff --git a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java index 6acd0679da2..b08346f7cec 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java +++ b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java @@ -126,20 +126,24 @@ public class SlimeUtils { return Duration.ofMillis(field.asLong()); } + public static boolean isPresent(Inspector field) { + return field.valid() && field.type() != Type.NIX; + } + public static Optional<String> optionalString(Inspector inspector) { - return Optional.of(inspector.asString()).filter(s -> !s.isEmpty()); + return Optional.of(inspector).filter(SlimeUtils::isPresent).map(Inspector::asString); } public static OptionalLong optionalLong(Inspector field) { - return field.valid() ? OptionalLong.of(field.asLong()) : OptionalLong.empty(); + return isPresent(field) ? OptionalLong.of(field.asLong()) : OptionalLong.empty(); } public static OptionalInt optionalInteger(Inspector field) { - return field.valid() ? OptionalInt.of((int) field.asLong()) : OptionalInt.empty(); + return isPresent(field) ? OptionalInt.of((int) field.asLong()) : OptionalInt.empty(); } public static OptionalDouble optionalDouble(Inspector field) { - return field.valid() ? OptionalDouble.of(field.asDouble()) : OptionalDouble.empty(); + return isPresent(field) ? OptionalDouble.of(field.asDouble()) : OptionalDouble.empty(); } public static Optional<Instant> optionalInstant(Inspector field) { |