diff options
52 files changed, 820 insertions, 534 deletions
diff --git a/config-model/src/main/java/com/yahoo/schema/derived/SummaryMap.java b/config-model/src/main/java/com/yahoo/schema/derived/SummaryMap.java index df9174a12ed..40d763eb897 100644 --- a/config-model/src/main/java/com/yahoo/schema/derived/SummaryMap.java +++ b/config-model/src/main/java/com/yahoo/schema/derived/SummaryMap.java @@ -57,9 +57,10 @@ public class SummaryMap extends Derived implements SummarymapConfig.Producer { // This works, but is suboptimal. We could consolidate to a minimal set and // use the right value from the minimal set as the third parameter here, // and add "override" commands to multiple static values + boolean useFieldNameAsArgument = summaryField.getTransform().isDynamic() || summaryField.getTransform() == SummaryTransform.TEXTEXTRACTOR; resultTransforms.put(summaryField.getName(), new FieldResultTransform(summaryField.getName(), summaryField.getTransform(), - summaryField.getName())); + useFieldNameAsArgument ? summaryField.getName() : "")); } } } @@ -99,20 +100,8 @@ public class SummaryMap extends Derived implements SummarymapConfig.Producer { for (FieldResultTransform frt : resultTransforms.values()) { SummarymapConfig.Override.Builder oB = new SummarymapConfig.Override.Builder() .field(frt.getFieldName()) - .command(getCommand(frt.getTransform())); - if (frt.getTransform().isDynamic() || - frt.getTransform().equals(SummaryTransform.ATTRIBUTE) || - frt.getTransform().equals(SummaryTransform.DISTANCE) || - frt.getTransform().equals(SummaryTransform.GEOPOS) || - frt.getTransform().equals(SummaryTransform.POSITIONS) || - frt.getTransform().equals(SummaryTransform.TEXTEXTRACTOR) || - frt.getTransform().equals(SummaryTransform.MATCHED_ELEMENTS_FILTER) || - frt.getTransform().equals(SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER)) - { - oB.arguments(frt.getArgument()); - } else { - oB.arguments(""); - } + .command(getCommand(frt.getTransform())) + .arguments(frt.getArgument()); builder.override(oB); } } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneFilter.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneFilter.java index 058be998478..e65340aa59b 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneFilter.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneFilter.java @@ -1,13 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision.zone; -import com.yahoo.config.provision.CloudName; - /** * A ZoneId list which can be filtered in various ways; elements can be accessed after at least one filter. * * The methods here return instances of {@link ZoneList}, which extends ZoneFilter, but with accessors and additional filters. * This forces the developer to consider which of the filters in this class to apply, prior to accessing any zones. + * Note: Do not add further filters, as this is only meant for the levels of configuration of the zone, not other properties. * * @author jonmv */ @@ -16,24 +15,13 @@ public interface ZoneFilter { /** Negates the next filter. */ ZoneFilter not(); - /** Zones which are upgraded by the controller. */ - ZoneList controllerUpgraded(); - - /** Zones where traffic is routed using given method */ - ZoneList routingMethod(RoutingMethod method); - /** Zones where config servers are up and running. */ ZoneList reachable(); - /** Zones where hosts must be reprovisioned to upgrade their OS */ - ZoneList reprovisionToUpgradeOs(); + /** Zones which are upgraded by the controller. */ + ZoneList controllerUpgraded(); /** All zones from the initial pool. */ ZoneList all(); - /** Zones in the specified cloud */ - default ZoneList ofCloud(CloudName cloud) { - return all(); // Not implemented in this repo. - } - } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java index c6ace00b90c..0a6bdd3b6b8 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java @@ -1,10 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision.zone; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; /** @@ -27,9 +29,23 @@ public interface ZoneList extends ZoneFilter { /** Zones in one of the given regions. */ ZoneList in(RegionName... regions); + /** Zones in one of the given clouds. */ + ZoneList in(CloudName... clouds); + /** Only the given zones — combine with not() for best effect! */ ZoneList among(ZoneId... zones); + /** Zones where hosts must be reprovisioned to upgrade their OS */ + ZoneList reprovisionToUpgradeOs(); + + /** Zones where traffic is routed using given method */ + ZoneList routingMethod(RoutingMethod method); + + /** Returns the zone with the given id, if this exists. */ + default Optional<? extends ZoneApi> get(ZoneId id) { + return among(id).zones().stream().findFirst(); + } + /** Returns the ZoneApi of all zones in this list. */ List<? extends ZoneApi> zones(); diff --git a/configgen/src/main/java/com/yahoo/config/codegen/JavaClassBuilder.java b/configgen/src/main/java/com/yahoo/config/codegen/JavaClassBuilder.java index 2560b218e54..fe1566aa133 100644 --- a/configgen/src/main/java/com/yahoo/config/codegen/JavaClassBuilder.java +++ b/configgen/src/main/java/com/yahoo/config/codegen/JavaClassBuilder.java @@ -47,7 +47,6 @@ public class JavaClassBuilder implements ClassBuilder { try (PrintStream out = new PrintStream(new FileOutputStream(outFile))) { out.print(getConfigClass(className)); } - System.err.println(outFile.getPath() + " successfully written."); } catch (FileNotFoundException e) { throw new CodegenRuntimeException(e); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java index 361d966ebc5..cce6429f84a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java @@ -92,9 +92,6 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { Instant start = Instant.now(); log.log(Level.FINE, () -> "Will build models for " + applicationId); Set<Version> versions = modelFactoryRegistry.allVersions(); - if (applicationPackage.getMajorVersion().isPresent() && applicationPackage.getMajorVersion().get() != wantedNodeVespaVersion.getMajor()) - throw new IllegalArgumentException("requested node version (" + wantedNodeVespaVersion + ") has a different major version " + - "than specified in deployment.xml (" + applicationPackage.getMajorVersion().get() + ")"); // If the application specifies a major, skip models on a newer major Optional<Integer> requestedMajorVersion = applicationPackage.getMajorVersion(); diff --git a/container-disc/abi-spec.json b/container-disc/abi-spec.json index aae37552283..968cef8e88f 100644 --- a/container-disc/abi-spec.json +++ b/container-disc/abi-spec.json @@ -17,6 +17,7 @@ "public abstract java.lang.String getAccessToken(java.lang.String)", "public abstract java.lang.String getAccessToken(java.lang.String, java.util.List)", "public abstract java.util.List getIdentityCertificate()", + "public abstract java.security.cert.X509Certificate getRoleCertificate(java.lang.String, java.lang.String)", "public abstract java.security.PrivateKey getPrivateKey()", "public abstract java.nio.file.Path trustStorePath()" ], diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java index 33d357e8b6b..f04e2291ee8 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/AthenzIdentityProviderProvider.java @@ -76,6 +76,11 @@ public class AthenzIdentityProviderProvider implements Provider<AthenzIdentityPr } @Override + public X509Certificate getRoleCertificate(String domain, String role) { + throw new UnsupportedOperationException(message); + } + + @Override public PrivateKey getPrivateKey() { throw new UnsupportedOperationException(message); } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java index c1c60612b37..af5133eceac 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/athenz/AthenzIdentityProvider.java @@ -21,6 +21,7 @@ public interface AthenzIdentityProvider { String getAccessToken(String domain); String getAccessToken(String domain, List<String> roles); List<X509Certificate> getIdentityCertificate(); + X509Certificate getRoleCertificate(String domain, String role); PrivateKey getPrivateKey(); Path trustStorePath(); diff --git a/container-search/src/test/java/com/yahoo/search/handler/JSONSearchHandlerTestCase.java b/container-search/src/test/java/com/yahoo/search/handler/JSONSearchHandlerTestCase.java index b92bf68e099..b0b88bf8190 100644 --- a/container-search/src/test/java/com/yahoo/search/handler/JSONSearchHandlerTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/handler/JSONSearchHandlerTestCase.java @@ -12,7 +12,6 @@ import com.yahoo.container.jdisc.RequestHandlerTestDriver; import com.yahoo.container.protect.Error; import com.yahoo.io.IOUtils; import com.yahoo.net.HostName; -import com.yahoo.search.handler.SearchHandler; import com.yahoo.search.searchchain.config.test.SearchChainConfigurerTestCase; import com.yahoo.slime.Inspector; import com.yahoo.slime.SlimeUtils; @@ -20,6 +19,7 @@ import com.yahoo.test.json.JsonTestHelper; import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -28,11 +28,12 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.HashMap; import java.util.Map; +import java.util.Random; import static com.yahoo.jdisc.http.HttpRequest.Method.GET; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; @@ -52,7 +53,6 @@ public class JSONSearchHandlerTestCase { private static final String selfHostname = HostName.getLocalhost(); private static String tempDir = ""; - private static String configId = null; private static final String uri = "http://localhost?"; private static final String JSON_CONTENT_TYPE = "application/json"; @@ -67,7 +67,7 @@ public class JSONSearchHandlerTestCase { public void startUp() throws IOException { File cfgDir = tempfolder.newFolder("SearchHandlerTestCase"); tempDir = cfgDir.getAbsolutePath(); - configId = "dir:" + tempDir; + String configId = "dir:" + tempDir; IOUtils.copyDirectory(new File(testDir), cfgDir, 1); // make configs active generateComponentsConfigForActive(); @@ -535,4 +535,27 @@ public class JSONSearchHandlerTestCase { assertOkResult(driver.sendRequest(uri, com.yahoo.jdisc.http.HttpRequest.Method.POST, json.toString(), "Application/JSON; charset=utf-8"), jsonResult); } + private static String createBenchmarkRequest(int num) { + Random rand = new Random(); + StringBuilder sb = new StringBuilder("{\"yql\": \"select id from vectors where {targetHits:10, approximate:true}nearestNeighbor(vector,q);\", \"input.query(q)\":["); + sb.append(rand.nextDouble()); + for (int i=1; i < num; i++) { + sb.append(','); + sb.append(rand.nextDouble()); + } + sb.append("]}"); + return sb.toString(); + } + @Ignore + public void benchmarkJsonParsing() { + String request = createBenchmarkRequest(768); + for (int i=0; i < 10000; i++) { + RequestHandlerTestDriver.MockResponseHandler responseHandler = + driver.sendRequest(uri, com.yahoo.jdisc.http.HttpRequest.Method.POST, request, JSON_CONTENT_TYPE); + String response = responseHandler.readAll(); + assertEquals(200, responseHandler.getStatus()); + assertFalse(response.isEmpty()); + } + } + } diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index 83d04c4cc6d..7ddc087feb7 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -116,8 +116,10 @@ public class JsonRendererTestCase { @After public void deconstructClone() { - renderer.deconstruct(); - renderer = null; + if (renderer != null) { + renderer.deconstruct(); + renderer = null; + } } @AfterClass @@ -436,10 +438,7 @@ public class JsonRendererTestCase { e2.search(subQuery); subQuery.trace("yellow", 1); q.trace("marker", 1); - ByteArrayOutputStream bs = new ByteArrayOutputStream(); - CompletableFuture<Boolean> f = renderer.renderResponse(bs, r, execution, null); - assertTrue(f.get()); - String summary = Utf8.toString(bs.toByteArray()); + String summary = render(execution, r); assertEqualJson(expected, summary); } @@ -478,10 +477,7 @@ public class JsonRendererTestCase { execution.search(q); new Execution(new Chain<>(), execution.context()); - ByteArrayOutputStream bs = new ByteArrayOutputStream(); - CompletableFuture<Boolean> f = renderer.renderResponse(bs, r, execution, null); - assertTrue(f.get()); - String summary = Utf8.toString(bs.toByteArray()); + String summary = render(execution, r); ObjectMapper m = new ObjectMapper(); Map<String, Object> exp = m.readValue(expected, Map.class); @@ -1478,10 +1474,15 @@ public class JsonRendererTestCase { } private String render(Execution execution, Result r) throws InterruptedException, ExecutionException { - ByteArrayOutputStream bs = new ByteArrayOutputStream(); - CompletableFuture<Boolean> f = renderer.renderResponse(bs, r, execution, null); - assertTrue(f.get()); - return Utf8.toString(bs.toByteArray()); + if (renderer == null) createClone(); + try { + ByteArrayOutputStream bs = new ByteArrayOutputStream(); + CompletableFuture<Boolean> f = renderer.renderResponse(bs, r, execution, null); + assertTrue(f.get()); + return Utf8.toString(bs.toByteArray()); + } finally { + deconstructClone(); + } } @SuppressWarnings("unchecked") diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/ZtsClientMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/ZtsClientMock.java index 0efa225a437..7322c8e15f8 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/ZtsClientMock.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/ZtsClientMock.java @@ -65,12 +65,12 @@ public class ZtsClientMock implements ZtsClient { } @Override - public ZToken getRoleToken(AthenzDomain domain) { + public ZToken getRoleToken(AthenzDomain domain, Duration expiry) { throw new UnsupportedOperationException(); } @Override - public ZToken getRoleToken(AthenzRole athenzRole) { + public ZToken getRoleToken(AthenzRole athenzRole, Duration expiry) { throw new UnsupportedOperationException(); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java index 77879699ab9..079383dd808 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java @@ -1,9 +1,13 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.deployment; +import com.yahoo.config.provision.Cloud; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import java.util.Comparator; @@ -38,18 +42,25 @@ public final class JobType implements Comparable<JobType> { } /** A system test in a test zone, or throws if no test zones are present.. */ - public static JobType systemTest(ZoneRegistry zones) { - return testIn(test, zones); + public static JobType systemTest(ZoneRegistry zones, CloudName cloud) { + return testIn(test, zones, cloud); } /** A staging test in a staging zone, or throws if no staging zones are present. */ - public static JobType stagingTest(ZoneRegistry zones){ - return testIn(staging, zones); + public static JobType stagingTest(ZoneRegistry zones, CloudName cloud){ + return testIn(staging, zones, cloud); } - private static JobType testIn(Environment environment, ZoneRegistry zones) { - return zones.zones().controllerUpgraded().in(environment).zones().stream().map(zone -> deploymentTo(zone.getId())) - .findFirst().orElseThrow(() -> new IllegalArgumentException("no zones in " + environment + " among " + zones.zones().controllerUpgraded().zones())); + /** Returns a test job in the given environment, preferring the given cloud, is possible; using the system cloud otherwise. */ + private static JobType testIn(Environment environment, ZoneRegistry zones, CloudName cloud) { + ZoneList candidates = zones.zones().controllerUpgraded().in(environment); + if (cloud == null || candidates.in(cloud).zones().isEmpty()) + cloud = zones.systemZone().getCloudName(); + + return candidates.in(cloud).zones().stream().findFirst() + .map(zone -> deploymentTo(zone.getId())) + .orElseThrow(() -> new IllegalArgumentException("no zones in " + environment + " among " + + zones.zones().controllerUpgraded().zones())); } /** A deployment to the given dev region. */ @@ -118,27 +129,33 @@ public final class JobType implements Comparable<JobType> { throw new IllegalArgumentException("illegal serialized job type '" + raw + "'"); } - /** Creates a new job type from a job name, and a zone registry for looking up zones for the special system and staging test types. */ + /** + * Creates a new job type from a job name, and a zone registry for looking up zones for the special system and staging test types. + * Note: system and staging tests retrieved by job name always use the default cloud for the system! + */ public static JobType fromJobName(String jobName, ZoneRegistry zones) { - String[] parts = jobName.split("-", 2); - if (parts.length != 2) throw new IllegalArgumentException("job names must be 'system-test', 'staging-test', or environment and region parts, separated by '-', but got: " + jobName); - switch (parts[0]) { - case "system": return systemTest(zones); - case "staging": return stagingTest(zones); - case "production": return prod(parts[1]); - case "test": return test(parts[1]); - case "dev": return dev(parts[1]); - case "perf": return perf(parts[1]); - default: throw new IllegalArgumentException("job names must begin with one of: system, staging, production, test, dev, perf; but got: " + jobName); + switch (jobName) { + case "system-test": return systemTest(zones, null); + case "staging-test": return stagingTest(zones, null); } + String[] parts = jobName.split("-", 2); + if (parts.length == 2) + switch (parts[0]) { + case "production": return prod(parts[1]); + case "test": return test(parts[1]); + case "dev": return dev(parts[1]); + case "perf": return perf(parts[1]); + } + throw new IllegalArgumentException("job names must be 'system-test', 'staging-test', or <test|environment>-<region>, but got: " + jobName); } public static List<JobType> allIn(ZoneRegistry zones) { return zones.zones().reachable().zones().stream() .flatMap(zone -> zone.getEnvironment().isProduction() ? Stream.of(deploymentTo(zone.getId()), productionTestOf(zone.getId())) : Stream.of(deploymentTo(zone.getId()))) + .distinct() .sorted(naturalOrder()) - .collect(toUnmodifiableList()); + .toList(); } /** A serialized form of this: {@code <environment>.<region>[.test]}; the inverse of {@link #ofSerialized(String)} */ diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java index 6ff52bd5f03..bbe0c2bd458 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java @@ -48,4 +48,4 @@ public class JobTypeTest { assertTrue(JobType.test("snohetta").isProduction()); } -} +}
\ No newline at end of file diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java index 9f36495254f..0df70cd9c53 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java @@ -63,7 +63,7 @@ public class EndpointCertificates { if (duration.toSeconds() > 30) log.log(Level.INFO, Text.format("Getting endpoint certificate metadata for %s took %d seconds!", instance.id().serializedForm(), duration.toSeconds())); - if (controller.zoneRegistry().zones().ofCloud(CloudName.from("gcp")).ids().contains(zone)) { // Until CKMS is available from GCP + if (controller.zoneRegistry().zones().all().in(CloudName.from("gcp")).ids().contains(zone)) { // Until CKMS is available from GCP if(metadata.isPresent()) { // Validate metadata before copying cert to GCP. This will ensure we don't bug out on the first deployment, but will take more time certificateValidator.validate(metadata.get(), instance.id().serializedForm(), zone, controller.routing().certificateDnsNames(new DeploymentId(instance.id(), zone), deploymentSpec)); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index 7578c133fc6..9f93033a1a2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -11,8 +11,10 @@ import com.yahoo.config.application.api.DeploymentSpec.DeclaredTest; import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone; import com.yahoo.config.application.api.DeploymentSpec.UpgradeRollout; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.stream.CustomCollectors; import com.yahoo.vespa.hosted.controller.Application; @@ -31,6 +33,7 @@ import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -55,8 +58,11 @@ import static java.util.Comparator.naturalOrder; import static java.util.Objects.requireNonNull; import static java.util.function.BinaryOperator.maxBy; import static java.util.stream.Collectors.collectingAndThen; +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.mapping; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; import static java.util.stream.Collectors.toUnmodifiableList; /** @@ -72,8 +78,6 @@ public class DeploymentStatus { private final Application application; private final JobList allJobs; - private final JobType systemTest; - private final JobType stagingTest; private final VersionStatus versionStatus; private final Version systemVersion; private final Function<InstanceName, VersionCompatibility> versionCompatibility; @@ -86,8 +90,6 @@ public class DeploymentStatus { Version systemVersion, Function<InstanceName, VersionCompatibility> versionCompatibility, Instant now) { this.application = requireNonNull(application); this.zones = zones; - this.systemTest = JobType.systemTest(zones); - this.stagingTest = JobType.stagingTest(zones); this.versionStatus = requireNonNull(versionStatus); this.systemVersion = requireNonNull(systemVersion); this.versionCompatibility = versionCompatibility; @@ -99,6 +101,14 @@ public class DeploymentStatus { this.allJobs = JobList.from(jobSteps.keySet().stream().map(allJobs).collect(toList())); } + private JobType systemTest(JobType dependent) { + return JobType.systemTest(zones, dependent == null ? null : findCloud(dependent)); + } + + private JobType stagingTest(JobType dependent) { + return JobType.stagingTest(zones, dependent == null ? null : findCloud(dependent)); + } + /** The application this deployment status concerns. */ public Application application() { return application; @@ -113,7 +123,7 @@ public class DeploymentStatus { private boolean hasFailures(StepStatus dependency, StepStatus dependent) { Set<StepStatus> dependents = new HashSet<>(); fillDependents(dependency, new HashSet<>(), dependents, dependent); - Set<JobId> criticalJobs = dependents.stream().flatMap(step -> step.job().stream()).collect(Collectors.toSet()); + Set<JobId> criticalJobs = dependents.stream().flatMap(step -> step.job().stream()).collect(toSet()); return ! allJobs.matching(job -> criticalJobs.contains(job.id())) .failingHard() @@ -206,16 +216,26 @@ public class DeploymentStatus { if (change == null || ! change.hasTargets()) return; - Optional<JobId> firstProductionJobWithDeployment = jobSteps.keySet().stream() - .filter(jobId -> jobId.type().isProduction() && jobId.type().isDeployment()) - .filter(jobId -> deploymentFor(jobId).isPresent()) - .findFirst(); - Versions versions = Versions.from(change, - application, - firstProductionJobWithDeployment.flatMap(this::deploymentFor), - fallbackPlatform(change, job)); - if (step.completedAt(change, Optional.empty()).isEmpty()) - jobs.merge(job, List.of(new Job(job.type(), versions, step.readyAt(change), change)), DeploymentStatus::union); + Collection<Optional<JobId>> firstProductionJobsWithDeployment = jobSteps.keySet().stream() + .filter(jobId -> jobId.type().isProduction() && jobId.type().isDeployment()) + .filter(jobId -> deploymentFor(jobId).isPresent()) + .collect(groupingBy(jobId -> findCloud(jobId.type()), + Collectors.reducing((o, n) -> o))) // Take the first. + .values(); + if (firstProductionJobsWithDeployment.isEmpty()) + firstProductionJobsWithDeployment = List.of(Optional.empty()); + + for (Optional<JobId> firstProductionJobWithDeploymentInCloud : firstProductionJobsWithDeployment) { + Versions versions = Versions.from(change, + application, + firstProductionJobWithDeploymentInCloud.flatMap(this::deploymentFor), + fallbackPlatform(change, job)); + if (step.completedAt(change, firstProductionJobWithDeploymentInCloud).isEmpty()) { + JobType actualType = job.type().isSystemTest() ? systemTest(firstProductionJobWithDeploymentInCloud.map(JobId::type).orElse(null)) + : stagingTest(firstProductionJobWithDeploymentInCloud.map(JobId::type).orElse(null)); + jobs.merge(job, List.of(new Job(actualType, versions, step.readyAt(change), change)), DeploymentStatus::union); + } + } }); return Collections.unmodifiableMap(jobs); } @@ -262,14 +282,7 @@ public class DeploymentStatus { /** The step status for all relevant steps in the deployment spec of this, in the same order as in the deployment spec. */ public List<StepStatus> allSteps() { - if (allSteps.isEmpty()) - return List.of(); - - List<JobId> firstTestJobs = List.of(firstDeclaredOrElseImplicitTest(systemTest), - firstDeclaredOrElseImplicitTest(stagingTest)); - return allSteps.stream() - .filter(step -> step.isDeclared() || firstTestJobs.contains(step.job().orElseThrow())) - .collect(toUnmodifiableList()); + return allSteps; } public Optional<Deployment> deploymentFor(JobId job) { @@ -278,13 +291,33 @@ public class DeploymentStatus { } private <T extends Comparable<T>> Optional<T> newestTested(InstanceName instance, Function<Run, T> runMapper) { - return instanceJobs().get(application.id().instance(instance)) - .type(systemTest, stagingTest) - .asList().stream().flatMap(jobs -> jobs.runs().values().stream()) - .filter(Run::hasSucceeded) - .map(runMapper) - .max(naturalOrder()); + Set<CloudName> clouds = jobSteps.keySet().stream() + .filter(job -> job.type().isProduction()) + .map(job -> findCloud(job.type())) + .collect(toSet()); + List<ZoneId> testZones = new ArrayList<>(); + if (application.deploymentSpec().requireInstance(instance).concerns(test)) { + if (clouds.isEmpty()) testZones.add(JobType.systemTest(zones, null).zone()); + else for (CloudName cloud: clouds) testZones.add(JobType.systemTest(zones, cloud).zone()); + } + if (application.deploymentSpec().requireInstance(instance).concerns(staging)) { + if (clouds.isEmpty()) testZones.add(JobType.stagingTest(zones, null).zone()); + else for (CloudName cloud: clouds) testZones.add(JobType.stagingTest(zones, cloud).zone()); + } + + Map<ZoneId, Optional<T>> newestPerZone = instanceJobs().get(application.id().instance(instance)) + .type(systemTest(null), stagingTest(null)) + .asList().stream().flatMap(jobs -> jobs.runs().values().stream()) + .filter(Run::hasSucceeded) + .collect(groupingBy(run -> run.id().type().zone(), + mapping(runMapper, Collectors.maxBy(naturalOrder())))); + return newestPerZone.keySet().containsAll(testZones) + ? testZones.stream().map(newestPerZone::get) + .reduce((o, n) -> o.isEmpty() || n.isEmpty() ? Optional.empty() : n.get().compareTo(o.get()) < 0 ? n : o) + .orElse(Optional.empty()) + : Optional.empty(); } + /** * The change to a revision which all dependencies of the given instance has completed, * which does not downgrade any deployments in the instance, @@ -349,8 +382,8 @@ public class DeploymentStatus { .filter(run -> run.versions().equals(versions)) .findFirst()) .map(Run::start); - Optional<Instant> systemTestedAt = testedAt(job.application(), systemTest, versions); - Optional<Instant> stagingTestedAt = testedAt(job.application(), stagingTest, versions); + Optional<Instant> systemTestedAt = testedAt(job.application(), systemTest(job.type()), versions); + Optional<Instant> stagingTestedAt = testedAt(job.application(), stagingTest(job.type()), versions); if (systemTestedAt.isEmpty() || stagingTestedAt.isEmpty()) return triggeredAt; Optional<Instant> testedAt = systemTestedAt.get().isAfter(stagingTestedAt.get()) ? systemTestedAt : stagingTestedAt; return triggeredAt.isPresent() && triggeredAt.get().isBefore(testedAt.get()) ? triggeredAt : testedAt; @@ -359,14 +392,15 @@ public class DeploymentStatus { /** Earliest instant when versions were tested for the given instance */ private Optional<Instant> testedAt(ApplicationId instance, JobType type, Versions versions) { return declaredTest(instance, type).map(__ -> allJobs.instance(instance.instance())) - .orElse(allJobs) - .type(type).asList().stream() - .flatMap(status -> RunList.from(status) - .on(versions) - .matching(Run::hasSucceeded) - .asList().stream() - .map(Run::start)) - .min(naturalOrder()); + .orElse(allJobs) + .type(type).asList().stream() + .flatMap(status -> RunList.from(status) + .on(versions) + .matching(run -> run.id().type().zone().equals(type.zone())) + .matching(Run::hasSucceeded) + .asList().stream() + .map(Run::start)) + .min(naturalOrder()); } private Map<JobId, List<Job>> productionJobs(InstanceName instance, Change change, boolean assumeUpgradesSucceed) { @@ -486,7 +520,7 @@ public class DeploymentStatus { // Both changes are ready for this step, and we look to the specified rollout to decide. boolean platformReadyFirst = platformReadyAt.get().isBefore(revisionReadyAt.get()); boolean revisionReadyFirst = revisionReadyAt.get().isBefore(platformReadyAt.get()); - boolean failingUpgradeOnlyTests = ! jobs().type(systemTest, stagingTest) + boolean failingUpgradeOnlyTests = ! jobs().type(systemTest(job.type()), stagingTest(job.type())) .failingHardOn(Versions.from(change.withoutApplication(), application, deploymentFor(job), systemVersion)) .isEmpty(); switch (rollout) { @@ -509,12 +543,12 @@ public class DeploymentStatus { /** The test jobs that need to run prior to the given production deployment jobs. */ public Map<JobId, List<Job>> testJobs(Map<JobId, List<Job>> jobs) { Map<JobId, List<Job>> testJobs = new LinkedHashMap<>(); - for (JobType testType : List.of(systemTest, stagingTest)) { - jobs.forEach((job, versionsList) -> { + jobs.forEach((job, versionsList) -> { + for (JobType testType : List.of(systemTest(job.type()), stagingTest(job.type()))) { if (job.type().isProduction() && job.type().isDeployment()) { declaredTest(job.application(), testType).ifPresent(testJob -> { for (Job productionJob : versionsList) - if (allJobs.successOn(productionJob.versions()).get(testJob).isEmpty()) + if (allJobs.successOn(testType, productionJob.versions()).asList().isEmpty()) testJobs.merge(testJob, List.of(new Job(testJob.type(), productionJob.versions(), jobSteps().get(testJob).readyAt(productionJob.change), @@ -522,13 +556,15 @@ public class DeploymentStatus { DeploymentStatus::union); }); } - }); - jobs.forEach((job, versionsList) -> { + } + }); + jobs.forEach((job, versionsList) -> { + for (JobType testType : List.of(systemTest(job.type()), stagingTest(job.type()))) { for (Job productionJob : versionsList) if ( job.type().isProduction() && job.type().isDeployment() - && allJobs.successOn(productionJob.versions()).type(testType).isEmpty() + && allJobs.successOn(testType, productionJob.versions()).asList().isEmpty() && testJobs.keySet().stream() - .noneMatch(test -> test.type().equals(testType) + .noneMatch(test -> test.type().equals(testType) && test.type().zone().equals(testType.zone()) && testJobs.get(test).stream().anyMatch(testJob -> testJob.versions().equals(productionJob.versions())))) { JobId testJob = firstDeclaredOrElseImplicitTest(testType); testJobs.merge(testJob, @@ -538,11 +574,15 @@ public class DeploymentStatus { productionJob.change)), DeploymentStatus::union); } - }); - } + } + }); return Collections.unmodifiableMap(testJobs); } + private CloudName findCloud(JobType job) { + return zones.zones().all().get(job.zone()).map(ZoneApi::getCloudName).orElse(null); + } + private JobId firstDeclaredOrElseImplicitTest(JobType testJob) { return application.deploymentSpec().instanceNames().stream() .map(name -> new JobId(application.id().instance(name), testJob)) @@ -598,7 +638,7 @@ public class DeploymentStatus { JobId jobId; StepStatus stepStatus; if (step.concerns(test) || step.concerns(staging)) { - jobType = step.concerns(test) ? systemTest : stagingTest; + jobType = step.concerns(test) ? systemTest(null) : stagingTest(null); jobId = new JobId(application.id().instance(instance), jobType); stepStatus = JobStepStatus.ofTestDeployment((DeclaredZone) step, List.of(), this, jobs.apply(jobId), true); previous = new ArrayList<>(previous); @@ -624,19 +664,19 @@ public class DeploymentStatus { if (step instanceof DeploymentInstanceSpec) { DeploymentInstanceSpec spec = ((DeploymentInstanceSpec) step); - StepStatus instanceStatus = new InstanceStatus(spec, previous, now, application.require(spec.name())); + StepStatus instanceStatus = new InstanceStatus(spec, previous, now, application.require(spec.name()), this); instance = spec.name(); allSteps.add(instanceStatus); previous = List.of(instanceStatus); if (instance.equals(implicitSystemTest)) { - JobId job = new JobId(application.id().instance(instance), systemTest); + JobId job = new JobId(application.id().instance(instance), systemTest(null)); JobStepStatus testStatus = JobStepStatus.ofTestDeployment(new DeclaredZone(test), List.of(), this, jobs.apply(job), false); dependencies.put(job, testStatus); allSteps.add(testStatus); } if (instance.equals(implicitStagingTest)) { - JobId job = new JobId(application.id().instance(instance), stagingTest); + JobId job = new JobId(application.id().instance(instance), stagingTest(null)); JobStepStatus testStatus = JobStepStatus.ofTestDeployment(new DeclaredZone(staging), List.of(), this, jobs.apply(job), false); dependencies.put(job, testStatus); @@ -777,13 +817,25 @@ public class DeploymentStatus { private final DeploymentInstanceSpec spec; private final Instant now; private final Instance instance; + private final DeploymentStatus status; private InstanceStatus(DeploymentInstanceSpec spec, List<StepStatus> dependencies, Instant now, - Instance instance) { + Instance instance, DeploymentStatus status) { super(StepType.instance, spec, dependencies, spec.name()); this.spec = spec; this.now = now; this.instance = instance; + this.status = status; + } + + /** The time at which this step is ready to run the specified change and / or versions. */ + @Override + public Optional<Instant> readyAt(Change change) { + return status.jobSteps.keySet().stream() + .filter(job -> job.type().isProduction() && job.application().instance().equals(instance.name())) + .map(job -> super.readyAt(change, Optional.of(job))) + .reduce((o, n) -> o.isEmpty() || n.isEmpty() ? Optional.empty() : n.get().isBefore(o.get()) ? n : o) + .orElseGet(() -> super.readyAt(change, Optional.empty())); } /** @@ -938,6 +990,8 @@ public class DeploymentStatus { return new JobStepStatus(StepType.test, step, dependencies, job, status) { @Override Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { + Optional<ZoneId> requiredTestZone = dependent.map(dep -> job.id().type().isSystemTest() ? status.systemTest(dep.type()).zone() + : status.stagingTest(dep.type()).zone()); return RunList.from(job) .matching(run -> dependent.flatMap(status::deploymentFor) .map(deployment -> run.versions().targetsMatch(Versions.from(change, @@ -947,6 +1001,7 @@ public class DeploymentStatus { .orElseGet(() -> (change.platform().isEmpty() || change.platform().get().equals(run.versions().targetPlatform())) && (change.revision().isEmpty() || change.revision().get().equals(run.versions().targetRevision())))) .matching(Run::hasSucceeded) + .matching(run -> requiredTestZone.isEmpty() || requiredTestZone.get().equals(run.id().type().zone())) .asList().stream() .map(run -> run.end().get()) .max(naturalOrder()); @@ -961,16 +1016,22 @@ public class DeploymentStatus { public static class Job { + private final JobType type; private final Versions versions; private final Optional<Instant> readyAt; private final Change change; public Job(JobType type, Versions versions, Optional<Instant> readyAt, Change change) { + this.type = type; this.versions = type.isSystemTest() ? versions.withoutSources() : versions; this.readyAt = readyAt; this.change = change; } + public JobType type() { + return type; + } + public Versions versions() { return versions; } @@ -984,12 +1045,12 @@ public class DeploymentStatus { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Job job = (Job) o; - return versions.equals(job.versions) && readyAt.equals(job.readyAt) && change.equals(job.change); + return type.zone().equals(job.type.zone()) && versions.equals(job.versions) && readyAt.equals(job.readyAt) && change.equals(job.change); } @Override public int hashCode() { - return Objects.hash(versions, readyAt, change); + return Objects.hash(type.zone(), versions, readyAt, change); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 32bb849a45b..c28f94bc4d7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -7,6 +7,7 @@ import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.TenantName; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; @@ -24,6 +25,7 @@ import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import java.math.BigDecimal; import java.time.Clock; import java.time.Duration; import java.time.Instant; @@ -366,6 +368,7 @@ public class DeploymentTrigger { .withDeploymentSpec()) .withChanges() .asList().stream() + .filter(status -> ! hasExceededQuota(status.application().id().tenant())) .map(this::computeReadyJobs) .flatMap(Collection::stream) .collect(toList()); @@ -375,21 +378,25 @@ public class DeploymentTrigger { private List<Job> computeReadyJobs(DeploymentStatus status) { List<Job> jobs = new ArrayList<>(); Map<JobId, List<DeploymentStatus.Job>> jobsToRun = status.jobsToRun(); - jobsToRun.forEach((job, versionsList) -> { - versionsList.get(0).readyAt() - .filter(readyAt -> ! clock.instant().isBefore(readyAt)) - .filter(__ -> ! (job.type().isProduction() && isUnhealthyInAnotherZone(status.application(), job))) - .filter(__ -> abortIfRunning(status, jobsToRun, job)) // Abort and trigger this later if running with outdated parameters. - .map(readyAt -> deploymentJob(status.application().require(job.application().instance()), - versionsList.get(0).versions(), - job.type(), - status.instanceJobs(job.application().instance()).get(job.type()), - readyAt)) - .ifPresent(jobs::add); + jobsToRun.forEach((jobId, jobsList) -> { + DeploymentStatus.Job job = jobsList.get(0); + if ( job.readyAt().isPresent() + && ! clock.instant().isBefore(job.readyAt().get()) + && ! (jobId.type().isProduction() && isUnhealthyInAnotherZone(status.application(), jobId)) + && abortIfRunning(status, jobsToRun, jobId)) // Abort and trigger this later if running with outdated parameters. + jobs.add(deploymentJob(status.application().require(jobId.application().instance()), + job.versions(), + job.type(), + status.instanceJobs(jobId.application().instance()).get(jobId.type()), + job.readyAt().get())); }); return Collections.unmodifiableList(jobs); } + private boolean hasExceededQuota(TenantName tenant) { + return controller.serviceRegistry().billingController().getQuota(tenant).budget().equals(Optional.of(BigDecimal.ZERO)); + } + /** Returns whether the application is healthy in all other production zones. */ private boolean isUnhealthyInAnotherZone(Application application, JobId job) { for (Deployment deployment : application.require(job.application().instance()).productionDeployments().values()) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobList.java index 387ea755414..551f841233e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobList.java @@ -119,8 +119,12 @@ public class JobList extends AbstractFilteringList<JobStatus, JobList> { } /** Returns the jobs with successful runs matching the given versions — targets only for system test, everything present otherwise. */ - public JobList successOn(Versions versions) { - return matching(job -> ! RunList.from(job).matching(Run::hasSucceeded).on(versions).isEmpty()); + public JobList successOn(JobType type, Versions versions) { + return matching(job -> job.id().type().equals(type) + && ! RunList.from(job) + .matching(run -> run.hasSucceeded() && run.id().type().zone().equals(type.zone())) + .on(versions) + .isEmpty()); } // ----------------------------------- JobRun filtering diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java index 51b40a9a4c7..3bd1c7bb358 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java @@ -65,7 +65,7 @@ public class OsUpgradeScheduler extends ControllerMaintainer { } private Release releaseIn(CloudName cloud) { - boolean useTaggedRelease = controller().zoneRegistry().zones().reprovisionToUpgradeOs().ofCloud(cloud) + boolean useTaggedRelease = controller().zoneRegistry().zones().all().reprovisionToUpgradeOs().in(cloud) .zones().isEmpty(); if (useTaggedRelease) { return new TaggedRelease(controller().system(), controller().serviceRegistry().artifactRepository()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java index 7b9c24df0fa..3588ae53a74 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceTagMaintainer.java @@ -34,8 +34,8 @@ public class ResourceTagMaintainer extends ControllerMaintainer { @Override public double maintain() { controller().zoneRegistry().zones() - .ofCloud(CloudName.from("aws")) .reachable() + .in(CloudName.from("aws")) .zones().forEach(zone -> { Map<HostName, ApplicationId> applicationOfHosts = getTenantOfParentHosts(zone.getId()); int taggedResources = resourceTagger.tagResources(zone, applicationOfHosts); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java index b36b2b9cad8..31f79c78ad5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/CostCalculator.java @@ -48,7 +48,7 @@ public class CostCalculator { // Sum up allocations Map<Property, ResourceAllocation> allocationByProperty = new HashMap<>(); var nodes = controller.zoneRegistry().zones() - .reachable().in(Environment.prod).ofCloud(cloudName).zones().stream() + .reachable().in(Environment.prod).in(cloudName).zones().stream() .flatMap(zone -> uncheck(() -> nodeRepository.list(zone.getId(), NodeFilter.all()).stream())) .filter(node -> node.owner().isPresent() && !node.owner().get().tenant().equals(SystemApplication.TENANT)) .collect(Collectors.toList()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 8c49df43c30..e28bf89e734 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -151,13 +151,7 @@ class JobControllerApiHandlerHelper { Optional<String> testReport = jobController.getTestReports(runId); testReport.map(SlimeUtils::jsonToSlime) .map(Slime::get) - .ifPresent(reportArrayCursor -> { - reportArrayCursor.traverse((ArrayTraverser) (i, reportCursor) -> { - if (i > 0) return; - SlimeUtils.copyObject(reportCursor, detailsObject.setObject("testReport")); - }); - SlimeUtils.copyArray(reportArrayCursor, detailsObject.setArray("testReports")); - }); + .ifPresent(reportArrayCursor -> SlimeUtils.copyArray(reportArrayCursor, detailsObject.setArray("testReports"))); return new SlimeJsonResponse(slime); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java index df31883b1d5..1691f902358 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java @@ -108,7 +108,7 @@ public class EndpointCertificatesTest { @Before public void setUp() { tester.zoneRegistry().exclusiveRoutingIn(tester.zoneRegistry().zones().all().zones()); - testZone = tester.zoneRegistry().zones().routingMethod(RoutingMethod.exclusive).in(Environment.prod).zones().stream().findFirst().orElseThrow().getId(); + testZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.prod).zones().stream().findFirst().orElseThrow().getId(); clock.setInstant(Instant.EPOCH); testCertificate = makeTestCert(expectedSans); testCertificate2 = makeTestCert(expectedCombinedSans); @@ -116,7 +116,7 @@ public class EndpointCertificatesTest { @Test public void provisions_new_certificate_in_dev() { - ZoneId testZone = tester.zoneRegistry().zones().routingMethod(RoutingMethod.exclusive).in(Environment.dev).zones().stream().findFirst().orElseThrow().getId(); + ZoneId testZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.dev).zones().stream().findFirst().orElseThrow().getId(); Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); assertTrue(endpointCertificateMetadata.isPresent()); assertTrue(endpointCertificateMetadata.get().keyName().matches("vespa.tls.default.default.*-key")); @@ -190,7 +190,7 @@ public class EndpointCertificatesTest { @Test public void reprovisions_certificate_with_added_sans_when_deploying_to_new_zone() { - ZoneId testZone = tester.zoneRegistry().zones().routingMethod(RoutingMethod.exclusive).in(Environment.prod).zones().stream().skip(1).findFirst().orElseThrow().getId(); + ZoneId testZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.prod).zones().stream().skip(1).findFirst().orElseThrow().getId(); mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "original-request-uuid", Optional.of("leaf-request-uuid"), expectedSans, "mockCa", Optional.empty(), Optional.empty())); secretStore.setSecret("vespa.tls.default.default.default-key", KeyUtils.toPem(testKeyPair.getPrivate()), -1); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java index 347f1d4ab15..ad236fcd4de 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java @@ -384,13 +384,13 @@ public class DeploymentContext { /** Runs and returns all remaining jobs for the application, at most once, and asserts the current change is rolled out. */ public DeploymentContext completeRollout(boolean multiInstance) { triggerJobs(); - Map<ApplicationId, Map<JobType, Versions>> jobsByInstance = new HashMap<>(); + Map<ApplicationId, Map<JobType, Run>> runsByInstance = new HashMap<>(); List<Run> activeRuns; while ( ! (activeRuns = this.jobs.active(applicationId)).isEmpty()) for (Run run : activeRuns) { - Map<JobType, Versions> jobs = jobsByInstance.computeIfAbsent(run.id().application(), k -> new HashMap<>()); - Versions previous = jobs.put(run.id().type(), run.versions()); - if (run.versions().equals(previous)) { + Map<JobType, Run> runs = runsByInstance.computeIfAbsent(run.id().application(), k -> new HashMap<>()); + Run previous = runs.put(run.id().type(), run); + if (previous != null && run.versions().equals(previous.versions()) && run.id().type().zone().equals(previous.id().type().zone())) { throw new AssertionError("Job '" + run.id() + "' was run twice on same versions"); } runJob(run.id().type(), run.id().application()); @@ -451,8 +451,8 @@ public class DeploymentContext { /** Pulls the ready job trigger, and then runs the whole of job for the given instance, successfully. */ private DeploymentContext runJob(JobType type, ApplicationId instance) { - var job = new JobId(instance, type); triggerJobs(); + var job = currentRun(new JobId(instance, type)).id().job(); doDeploy(job); if (job.type().isDeployment()) { doUpgrade(job); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 67ddc767b39..e5000a85f71 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -2,23 +2,34 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.component.Version; +import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.zone.RoutingMethod; +import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; +import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; +import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import org.junit.Assert; import org.junit.Test; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -981,36 +992,38 @@ public class DeploymentTriggerTest { DeploymentContext i3 = tester.newDeploymentContext("t", "a", "i3"); DeploymentContext i4 = tester.newDeploymentContext("t", "a", "i4"); ApplicationPackage applicationPackage = ApplicationPackageBuilder - .fromDeploymentXml("<deployment version='1'>\n" + - " <upgrade revision-change='when-failing' />\n" + - " <parallel>\n" + - " <instance id='i1'>\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <delay hours='6' />\n" + - " </prod>\n" + - " </instance>\n" + - " <instance id='i2'>\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " </prod>\n" + - " </instance>\n" + - " </parallel>\n" + - " <instance id='i3'>\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <delay hours='18' />\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - " <instance id='i4'>\n" + - " <test />\n" + - " <staging />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " </prod>\n" + - " </instance>\n" + - "</deployment>\n"); + .fromDeploymentXml(""" + <deployment version='1'> + <upgrade revision-change='when-failing' /> + <parallel> + <instance id='i1'> + <prod> + <region>us-east-3</region> + <delay hours='6' /> + </prod> + </instance> + <instance id='i2'> + <prod> + <region>us-east-3</region> + </prod> + </instance> + </parallel> + <instance id='i3'> + <prod> + <region>us-east-3</region> + <delay hours='18' /> + <test>us-east-3</test> + </prod> + </instance> + <instance id='i4'> + <test /> + <staging /> + <prod> + <region>us-east-3</region> + </prod> + </instance> + </deployment> + """); // Package is submitted, and change propagated to the two first instances. i1.submit(applicationPackage); @@ -1085,20 +1098,22 @@ public class DeploymentTriggerTest { @Test public void testMultipleInstancesWithRevisionCatchingUpToUpgrade() { - String spec = "<deployment>\n" + - " <instance id='alpha'>\n" + - " <upgrade rollout=\"simultaneous\" revision-target=\"next\" />\n" + - " <test />\n" + - " <staging />\n" + - " </instance>\n" + - " <instance id='beta'>\n" + - " <upgrade rollout=\"simultaneous\" revision-change=\"when-clear\" revision-target=\"next\" />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - "</deployment>\n"; + String spec = """ + <deployment> + <instance id='alpha'> + <upgrade rollout="simultaneous" revision-target="next" /> + <test /> + <staging /> + </instance> + <instance id='beta'> + <upgrade rollout="simultaneous" revision-change="when-clear" revision-target="next" /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + </deployment> + """; ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(spec); DeploymentContext alpha = tester.newDeploymentContext("t", "a", "alpha"); DeploymentContext beta = tester.newDeploymentContext("t", "a", "beta"); @@ -1236,67 +1251,69 @@ public class DeploymentTriggerTest { @Test public void testDeployComplicatedDeploymentSpec() { String complicatedDeploymentSpec = - "<deployment version='1.0' athenz-domain='domain' athenz-service='service'>\n" + - " <parallel>\n" + - " <instance id='instance' athenz-service='in-service'>\n" + - " <staging />\n" + - " <prod>\n" + - " <parallel>\n" + - " <region active='true'>us-west-1</region>\n" + - " <steps>\n" + - " <region active='true'>us-east-3</region>\n" + - " <delay hours='2' />\n" + - " <region active='true'>eu-west-1</region>\n" + - " <delay hours='2' />\n" + - " </steps>\n" + - " <steps>\n" + - " <delay hours='3' />\n" + - " <region active='true'>aws-us-east-1a</region>\n" + - " <parallel>\n" + - " <region active='true' athenz-service='no-service'>ap-northeast-1</region>\n" + - " <region active='true'>ap-northeast-2</region>\n" + - " <test>aws-us-east-1a</test>\n" + - " </parallel>\n" + - " </steps>\n" + - " <delay hours='3' minutes='30' />\n" + - " </parallel>\n" + - " <parallel>\n" + - " <test>ap-northeast-2</test>\n" + - " <test>ap-northeast-1</test>\n" + - " </parallel>\n" + - " <test>us-east-3</test>\n" + - " <region active='true'>ap-southeast-1</region>\n" + - " </prod>\n" + - " <endpoints>\n" + - " <endpoint id='foo' container-id='bar'>\n" + - " <region>us-east-3</region>\n" + - " </endpoint>\n" + - " <endpoint id='nalle' container-id='frosk' />\n" + - " <endpoint container-id='quux' />\n" + - " </endpoints>\n" + - " </instance>\n" + - " <instance id='other'>\n" + - " <upgrade policy='conservative' />\n" + - " <test />\n" + - " <block-change revision='true' version='false' days='sat' hours='0-23' time-zone='CET' />\n" + - " <prod>\n" + - " <region active='true'>eu-west-1</region>\n" + - " <test>eu-west-1</test>\n" + - " </prod>\n" + - " <notifications when='failing'>\n" + - " <email role='author' />\n" + - " <email address='john@dev' when='failing-commit' />\n" + - " <email address='jane@dev' />\n" + - " </notifications>\n" + - " </instance>\n" + - " </parallel>\n" + - " <instance id='last'>\n" + - " <upgrade policy='conservative' />\n" + - " <prod>\n" + - " <region active='true'>eu-west-1</region>\n" + - " </prod>\n" + - " </instance>\n" + - "</deployment>\n"; + """ + <deployment version='1.0' athenz-domain='domain' athenz-service='service'> + <parallel> + <instance id='instance' athenz-service='in-service'> + <staging /> + <prod> + <parallel> + <region active='true'>us-west-1</region> + <steps> + <region active='true'>us-east-3</region> + <delay hours='2' /> + <region active='true'>eu-west-1</region> + <delay hours='2' /> + </steps> + <steps> + <delay hours='3' /> + <region active='true'>aws-us-east-1a</region> + <parallel> + <region active='true' athenz-service='no-service'>ap-northeast-1</region> + <region active='true'>ap-northeast-2</region> + <test>aws-us-east-1a</test> + </parallel> + </steps> + <delay hours='3' minutes='30' /> + </parallel> + <parallel> + <test>ap-northeast-2</test> + <test>ap-northeast-1</test> + </parallel> + <test>us-east-3</test> + <region active='true'>ap-southeast-1</region> + </prod> + <endpoints> + <endpoint id='foo' container-id='bar'> + <region>us-east-3</region> + </endpoint> + <endpoint id='nalle' container-id='frosk' /> + <endpoint container-id='quux' /> + </endpoints> + </instance> + <instance id='other'> + <upgrade policy='conservative' /> + <test /> + <block-change revision='true' version='false' days='sat' hours='0-23' time-zone='CET' /> + <prod> + <region active='true'>eu-west-1</region> + <test>eu-west-1</test> + </prod> + <notifications when='failing'> + <email role='author' /> + <email address='john@dev' when='failing-commit' /> + <email address='jane@dev' /> + </notifications> + </instance> + </parallel> + <instance id='last'> + <upgrade policy='conservative' /> + <prod> + <region active='true'>eu-west-1</region> + </prod> + </instance> + </deployment> + """; tester.atMondayMorning(); ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(complicatedDeploymentSpec); @@ -1640,31 +1657,33 @@ public class DeploymentTriggerTest { @Test public void testVeryLengthyPipelineRevisions() { String lengthyDeploymentSpec = - "<deployment version='1.0'>\n" + - " <instance id='alpha'>\n" + - " <test />\n" + - " <staging />\n" + - " <upgrade revision-change='always' />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - " <instance id='beta'>\n" + - " <upgrade revision-change='when-failing' />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - " <instance id='gamma'>\n" + - " <upgrade revision-change='when-clear' revision-target='next' min-risk='3' max-risk='6' />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - "</deployment>\n"; + """ + <deployment version='1.0'> + <instance id='alpha'> + <test /> + <staging /> + <upgrade revision-change='always' /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + <instance id='beta'> + <upgrade revision-change='when-failing' /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + <instance id='gamma'> + <upgrade revision-change='when-clear' revision-target='next' min-risk='3' max-risk='6' /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + </deployment> + """; var appPackage = ApplicationPackageBuilder.fromDeploymentXml(lengthyDeploymentSpec); var alpha = tester.newDeploymentContext("t", "a", "alpha"); var beta = tester.newDeploymentContext("t", "a", "beta"); @@ -1783,31 +1802,33 @@ public class DeploymentTriggerTest { @Test public void testVeryLengthyPipelineUpgrade() { String lengthyDeploymentSpec = - "<deployment version='1.0'>\n" + - " <instance id='alpha'>\n" + - " <test />\n" + - " <staging />\n" + - " <upgrade rollout='simultaneous' />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - " <instance id='beta'>\n" + - " <upgrade rollout='simultaneous' />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - " <instance id='gamma'>\n" + - " <upgrade rollout='separate' />\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " <test>us-east-3</test>\n" + - " </prod>\n" + - " </instance>\n" + - "</deployment>\n"; + """ + <deployment version='1.0'> + <instance id='alpha'> + <test /> + <staging /> + <upgrade rollout='simultaneous' /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + <instance id='beta'> + <upgrade rollout='simultaneous' /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + <instance id='gamma'> + <upgrade rollout='separate' /> + <prod> + <region>us-east-3</region> + <test>us-east-3</test> + </prod> + </instance> + </deployment> + """; var appPackage = ApplicationPackageBuilder.fromDeploymentXml(lengthyDeploymentSpec); var alpha = tester.newDeploymentContext("t", "a", "alpha"); var beta = tester.newDeploymentContext("t", "a", "beta"); @@ -1986,19 +2007,21 @@ public class DeploymentTriggerTest { @Test public void testsInSeparateInstance() { String deploymentSpec = - "<deployment version='1.0' athenz-domain='domain' athenz-service='service'>\n" + - " <instance id='canary'>\n" + - " <upgrade policy='canary' />\n" + - " <test />\n" + - " <staging />\n" + - " </instance>\n" + - " <instance id='default'>\n" + - " <prod>\n" + - " <region>eu-west-1</region>\n" + - " <test>eu-west-1</test>\n" + - " </prod>\n" + - " </instance>\n" + - "</deployment>\n"; + """ + <deployment version='1.0' athenz-domain='domain' athenz-service='service'> + <instance id='canary'> + <upgrade policy='canary' /> + <test /> + <staging /> + </instance> + <instance id='default'> + <prod> + <region>eu-west-1</region> + <test>eu-west-1</test> + </prod> + </instance> + </deployment> + """; ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(deploymentSpec); var canary = tester.newDeploymentContext("t", "a", "canary").submit(applicationPackage); @@ -2157,29 +2180,60 @@ public class DeploymentTriggerTest { } @Test - public void testInstanceWithOnlySystemTest() { - String spec = "<deployment>\n" + - " <instance id='tests'>" + - " <test />\n" + - " <upgrade revision-target='next' />" + - " </instance>\n" + - " <instance id='main'>\n" + - " <prod>\n" + - " <region>us-east-3</region>\n" + - " </prod>\n" + - " <upgrade revision-target='next' />" + - " </instance>\n" + - "</deployment>\n"; + public void testInstanceWithOnlySystemTestInTwoClouds() { + String spec = """ + <deployment> + <instance id='tests'> + <test /> + <upgrade revision-target='next' /> + </instance> + <instance id='main'> + <prod> + <region>us-east-3</region> + <region>alpha-centauri</region> + </prod> + <upgrade revision-target='next' /> + </instance> + </deployment> + """; + + RegionName alphaCentauri = RegionName.from("alpha-centauri"); + ZoneApiMock.Builder builder = ZoneApiMock.newBuilder().withCloud("centauri").withSystem(tester.controller().system()); + ZoneApi testAlphaCentauri = builder.with(ZoneId.from(Environment.test, alphaCentauri)).build(); + ZoneApi stagingAlphaCentauri = builder.with(ZoneId.from(Environment.staging, alphaCentauri)).build(); + ZoneApi prodAlphaCentauri = builder.with(ZoneId.from(Environment.prod, alphaCentauri)).build(); + + tester.controllerTester().zoneRegistry().addZones(testAlphaCentauri, stagingAlphaCentauri, prodAlphaCentauri); + tester.controllerTester().setRoutingMethod(tester.controllerTester().zoneRegistry().zones().all().ids(), RoutingMethod.sharedLayer4); + tester.configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(), SystemApplication.notController()); + ApplicationPackage appPackage = ApplicationPackageBuilder.fromDeploymentXml(spec); DeploymentContext tests = tester.newDeploymentContext("tenant", "application", "tests"); DeploymentContext main = tester.newDeploymentContext("tenant", "application", "main"); Version version1 = new Version("7"); tester.controllerTester().upgradeSystem(version1); - tests.submit(appPackage).deploy(); + tests.submit(appPackage); Optional<RevisionId> revision1 = tests.lastSubmission(); JobId systemTestJob = new JobId(tests.instanceId(), systemTest); JobId stagingTestJob = new JobId(tests.instanceId(), stagingTest); JobId mainJob = new JobId(main.instanceId(), productionUsEast3); + JobId centauriJob = new JobId(main.instanceId(), JobType.deploymentTo(prodAlphaCentauri.getId())); + + assertEquals(Set.of(systemTestJob, stagingTestJob, mainJob, centauriJob), tests.deploymentStatus().jobsToRun().keySet()); + tests.runJob(systemTest).runJob(stagingTest).triggerJobs(); + + assertEquals(Set.of(systemTestJob, stagingTestJob, mainJob, centauriJob), tests.deploymentStatus().jobsToRun().keySet()); + tests.triggerJobs(); + assertEquals(3, tester.jobs().active().size()); + + tests.runJob(systemTest); + tester.outstandingChangeDeployer().run(); + + assertEquals(2, tester.jobs().active().size()); + main.assertRunning(productionUsEast3); + + tests.runJob(stagingTest); + main.runJob(productionUsEast3).runJob(centauriJob.type()); assertEquals(Change.empty(), tests.instance().change()); assertEquals(Change.empty(), main.instance().change()); @@ -2197,25 +2251,28 @@ public class DeploymentTriggerTest { assertEquals(Change.of(version2), tests.instance().change()); assertEquals(Change.empty(), main.instance().change()); assertEquals(Set.of(systemTestJob), tests.deploymentStatus().jobsToRun().keySet()); + assertEquals(2, tests.deploymentStatus().jobsToRun().get(systemTestJob).size()); Version version3 = new Version("9"); tester.controllerTester().upgradeSystem(version3); - tests.failDeployment(systemTest); + tests.runJob(systemTest) // Success in default cloud. + .failDeployment(systemTest); // Failure in centauri cloud. tester.upgrader().run(); assertEquals(Change.of(version3), tests.instance().change()); assertEquals(Change.empty(), main.instance().change()); assertEquals(Set.of(systemTestJob), tests.deploymentStatus().jobsToRun().keySet()); - tests.runJob(systemTest); + tests.runJob(systemTest).runJob(systemTest); tester.upgrader().run(); - tests.runJob(stagingTest); + tests.runJob(stagingTest).runJob(stagingTest); assertEquals(Change.empty(), tests.instance().change()); assertEquals(Change.of(version3), main.instance().change()); - assertEquals(Set.of(mainJob), tests.deploymentStatus().jobsToRun().keySet()); + assertEquals(Set.of(mainJob, centauriJob), tests.deploymentStatus().jobsToRun().keySet()); main.runJob(productionUsEast3); + main.runJob(centauriJob.type()); assertEquals(Change.empty(), tests.instance().change()); assertEquals(Change.empty(), main.instance().change()); @@ -2237,6 +2294,7 @@ public class DeploymentTriggerTest { assertEquals(Change.of(revision2.get()), tests.instance().change()); assertEquals(Change.empty(), main.instance().change()); assertEquals(Set.of(systemTestJob), tests.deploymentStatus().jobsToRun().keySet()); + assertEquals(2, tests.deploymentStatus().jobsToRun().get(systemTestJob).size()); tests.submit(appPackage); Optional<RevisionId> revision3 = tests.lastSubmission(); @@ -2253,15 +2311,34 @@ public class DeploymentTriggerTest { assertEquals(Set.of(systemTestJob), tests.deploymentStatus().jobsToRun().keySet()); tests.runJob(systemTest); + assertEquals(Change.of(revision3.get()), tests.instance().change()); + assertEquals(Change.empty(), main.instance().change()); + assertEquals(Set.of(systemTestJob, stagingTestJob), tests.deploymentStatus().jobsToRun().keySet()); + + tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); + tests.runJob(stagingTest); + + assertEquals(Change.of(revision3.get()), tests.instance().change()); + assertEquals(Change.empty(), main.instance().change()); + assertEquals(Set.of(systemTestJob, stagingTestJob), tests.deploymentStatus().jobsToRun().keySet()); + + tests.runJob(systemTest); tester.outstandingChangeDeployer().run(); tester.outstandingChangeDeployer().run(); + + assertEquals(Change.empty(), tests.instance().change()); + assertEquals(Change.of(revision3.get()), main.instance().change()); + assertEquals(Set.of(stagingTestJob, mainJob, centauriJob), tests.deploymentStatus().jobsToRun().keySet()); + tests.runJob(stagingTest); assertEquals(Change.empty(), tests.instance().change()); assertEquals(Change.of(revision3.get()), main.instance().change()); - assertEquals(Set.of(mainJob), tests.deploymentStatus().jobsToRun().keySet()); + assertEquals(Set.of(mainJob, centauriJob), tests.deploymentStatus().jobsToRun().keySet()); main.runJob(productionUsEast3); + main.runJob(centauriJob.type()); tester.outstandingChangeDeployer().run(); assertEquals(Change.empty(), tests.instance().change()); @@ -2287,4 +2364,28 @@ public class DeploymentTriggerTest { assertTrue(tester.jobs().last(app.instanceId(), stagingTest).get().hasSucceeded()); } + @Test + public void testJobNames() { + ZoneRegistryMock zones = new ZoneRegistryMock(SystemName.main); + List<ZoneApi> existing = new ArrayList<>(zones.zones().all().zones()); + existing.add(ZoneApiMock.newBuilder().withCloud("pink-clouds").withId("test.zone").build()); + zones.setZones(existing); + + JobType defaultSystemTest = JobType.systemTest(zones, CloudName.defaultName()); + JobType pinkSystemTest = JobType.systemTest(zones, CloudName.from("pink-clouds")); + + // Job name is identity, used for looking up run history, etc.. + assertEquals(defaultSystemTest, pinkSystemTest); + + assertEquals(defaultSystemTest, JobType.systemTest(zones, null)); + assertEquals(defaultSystemTest, JobType.systemTest(zones, CloudName.from("dark-clouds"))); + assertEquals(defaultSystemTest, JobType.fromJobName("system-test", zones)); + + assertEquals(ZoneId.from("test", "us-east-1"), defaultSystemTest.zone()); + assertEquals(ZoneId.from("staging", "us-east-3"), JobType.stagingTest(zones, null).zone()); + + assertEquals(ZoneId.from("test", "zone"), pinkSystemTest.zone()); + assertEquals(ZoneId.from("staging", "us-east-3"), JobType.stagingTest(zones, CloudName.from("pink-clouds")).zone()); + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java index c17b2fcb36a..c87ac229c4b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java @@ -81,6 +81,11 @@ public class ZoneFilterMock implements ZoneList { } @Override + public ZoneList in(CloudName... clouds) { + return filter(zone -> Set.of(clouds).contains(zone.getCloudName())); + } + + @Override public ZoneList among(ZoneId... zones) { return filter(zone -> Set.of(zones).contains(zone.getId())); } @@ -90,11 +95,6 @@ public class ZoneFilterMock implements ZoneList { return List.copyOf(zones); } - @Override - public ZoneList ofCloud(CloudName cloud) { - return filter(zone -> zone.getCloudName().equals(cloud)); - } - private ZoneFilterMock filter(Predicate<ZoneApi> condition) { return new ZoneFilterMock( zones.stream() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java index 849503ae8d1..4131bfd8b9f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java @@ -26,6 +26,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import java.net.URI; import java.time.Duration; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -45,8 +46,6 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry private final Set<ZoneApi> reprovisionToUpgradeOs = new HashSet<>(); private final SystemName system; // Don't even think about making it non-final! ƪ(`▿▿▿▿´ƪ) - - private List<? extends ZoneApi> zones; private UpgradePolicy upgradePolicy = null; @@ -100,6 +99,12 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry return setZones(List.of(zone)); } + public ZoneRegistryMock addZones(ZoneApi... zones) { + List<ZoneApi> allZones = new ArrayList<>(this.zones); + Collections.addAll(allZones, zones); + return setZones(allZones); + } + public ZoneRegistryMock setUpgradePolicy(UpgradePolicy upgradePolicy) { this.upgradePolicy = upgradePolicy; return this; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json index e31058d349b..5bf6822baff 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json @@ -392,12 +392,6 @@ "startMillis": 0 } }, - "testReport": { - "summary": { - "success": 1, - "failed": 0 - } - }, "testReports": [ { "summary": { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/handler/UnsafeContentInputStream.java b/jdisc_core/src/main/java/com/yahoo/jdisc/handler/UnsafeContentInputStream.java index 3855f053b76..731e28b556c 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/handler/UnsafeContentInputStream.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/handler/UnsafeContentInputStream.java @@ -19,7 +19,7 @@ import java.util.Objects; public class UnsafeContentInputStream extends InputStream { private final ReadableContentChannel content; - private ByteBuffer buf = ByteBuffer.allocate(0); + private ByteBuffer currBuf = ByteBuffer.allocate(0); private byte [] marked; private int readSinceMarked; @@ -34,13 +34,10 @@ public class UnsafeContentInputStream extends InputStream { @Override public int read() { - while (buf != null && buf.remaining() == 0) { - buf = content.read(); - } - if (buf == null) { - return -1; - } - byte b = buf.get(); + fetchNonEmptyBuffer(); + if (currBuf == null) return -1; + + byte b = currBuf.get(); if (marked != null) { if (readSinceMarked < marked.length) { marked[readSinceMarked++] = b; @@ -51,34 +48,45 @@ public class UnsafeContentInputStream extends InputStream { return ((int)b) & 0xFF; } + private boolean fetchNonEmptyBuffer() { + while (currBuf != null && currBuf.remaining() == 0) { + currBuf = content.read(); + } + return (currBuf != null && currBuf.hasRemaining()); + } + @Override public int read(byte buf[], int off, int len) { Objects.requireNonNull(buf, "buf"); if (off < 0 || len < 0 || len > buf.length - off) { throw new IndexOutOfBoundsException(); } - if (len == 0) { - return 0; - } - int c = read(); - if (c == -1) { - return -1; + if (len == 0) return 0; + + if ( ! fetchNonEmptyBuffer() ) return -1; + int read = 0; + while ((available() > 0) && fetchNonEmptyBuffer() && ((len - read) > 0)) { + int toRead = Math.min(currBuf.remaining(), (len - read)); + currBuf.get(buf, off + read, toRead); + read += toRead; } - buf[off] = (byte)c; - int cnt = 1; - for (; cnt < len && available() > 0; ++cnt) { - if ((c = read()) == -1) { - break; + if (marked != null) { + if (readSinceMarked + len < marked.length) { + for (int i=0; i < len; i++) { + marked[readSinceMarked++] = buf[off+i]; + } + } else { + marked = null; } - buf[off + cnt] = (byte)c; + } - return cnt; + return read; } @Override public int available() { - if (buf != null && buf.remaining() > 0) { - return buf.remaining(); + if (currBuf != null && currBuf.remaining() > 0) { + return currBuf.remaining(); } return content.available(); } @@ -102,11 +110,11 @@ public class UnsafeContentInputStream extends InputStream { if (marked == null) { throw new IOException("mark has not been called, or too much has been read since marked."); } - ByteBuffer newBuf = ByteBuffer.allocate(readSinceMarked + buf.remaining()); + ByteBuffer newBuf = ByteBuffer.allocate(readSinceMarked + currBuf.remaining()); newBuf.put(marked, 0, readSinceMarked); - newBuf.put(buf); + newBuf.put(currBuf); newBuf.flip(); - buf = newBuf; + currBuf = newBuf; marked = null; } 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 e91963ab621..cde7f300f2b 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 @@ -304,7 +304,7 @@ public final class Node implements Nodelike { /** Returns a copy of this with removable set to the given value */ public Node removable(boolean removable) { - return with(requireAllocation("Cannot set removable").removable(removable)); + return with(requireAllocation("Cannot set removable").removable(removable, false)); } /** Returns a copy of this with the restart generation set to generation */ @@ -385,9 +385,8 @@ public final class Node implements Nodelike { /** Returns a copy of this with allocation set as specified. <code>node.state</code> is *not* changed. */ public Node allocate(ApplicationId owner, ClusterMembership membership, NodeResources requestedResources, Instant at) { - return this - .with(new Allocation(owner, membership, requestedResources, new Generation(0, 0), false)) - .with(history.with(new History.Event(History.Event.Type.reserved, Agent.application, at))); + return this.with(new Allocation(owner, membership, requestedResources, new Generation(0, 0), false)) + .with(history.with(new History.Event(History.Event.Type.reserved, Agent.application, at))); } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java index 9a110427223..dd4d5aa213f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java @@ -9,7 +9,6 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.node.ClusterId; -import com.yahoo.vespa.hosted.provision.node.Report; import java.util.Comparator; import java.util.EnumSet; @@ -57,7 +56,12 @@ public class NodeList extends AbstractFilteringList<Node, NodeList> { /** Returns the subset of nodes which are removable */ public NodeList removable() { - return matching(node -> node.allocation().isPresent() && node.allocation().get().isRemovable()); + return matching(node -> node.allocation().isPresent() && node.allocation().get().removable()); + } + + /** Returns the subset of nodes which are reusable immediately after removal */ + public NodeList reusable() { + return matching(node -> node.allocation().isPresent() && node.allocation().get().reusable()); } /** Returns the subset of nodes having exactly the given resources */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java index 9623406767a..c2e66d39861 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java @@ -115,7 +115,7 @@ public class Autoscaler { // The cluster is processing recent changes if (clusterNodes.stream().anyMatch(node -> node.status().wantToRetire() || node.allocation().get().membership().retired() || - node.allocation().get().isRemovable())) + node.allocation().get().removable())) return false; // A deployment is ongoing diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java index 73c9a1ab55a..3143fb5bcfe 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java @@ -14,9 +14,7 @@ import com.yahoo.vespa.orchestrator.OrchestrationException; import com.yahoo.yolean.Exceptions; import java.time.Duration; -import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** * Maintenance job which deactivates retired nodes, if given permission by orchestrator, or @@ -53,42 +51,48 @@ public class RetiredExpirer extends NodeRepositoryMaintainer { for (Map.Entry<ApplicationId, NodeList> entry : retiredNodesByApplication.entrySet()) { ApplicationId application = entry.getKey(); NodeList retiredNodes = entry.getValue(); - List<Node> nodesToRemove = retiredNodes.stream().filter(n -> canRemove(n, activeNodes)).collect(Collectors.toList()); - if (nodesToRemove.isEmpty()) continue; - - attempts++; - try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { - if ( ! deployment.isValid()) continue; - - nodeRepository().nodes().setRemovable(application, nodesToRemove); - boolean success = deployment.activate().isPresent(); - if ( ! success) continue; - String nodeList = nodesToRemove.stream().map(Node::hostname).collect(Collectors.joining(", ")); - log.info("Redeployed " + application + " to deactivate retired nodes: " + nodeList); - successes++; + Map<Removal, NodeList> nodesByRemovalReason = retiredNodes.groupingBy(node -> removalOf(node, activeNodes)); + if (nodesByRemovalReason.isEmpty()) continue; + + for (var kv : nodesByRemovalReason.entrySet()) { + Removal removal = kv.getKey(); + if (removal.equals(Removal.none())) continue; + + NodeList nodes = kv.getValue(); + attempts++; + try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { + if (!deployment.isValid()) continue; + + nodeRepository().nodes().setRemovable(application, nodes.asList(), removal.isReusable()); + boolean success = deployment.activate().isPresent(); + if (!success) continue; + String nodeList = String.join(", ", nodes.mapToList(Node::hostname)); + log.info("Redeployed " + application + " to deactivate retired nodes: " + nodeList); + successes++; + } } } return attempts == 0 ? 1.0 : ((double)successes / attempts); } /** - * Checks if the node can be removed: - * if the node is a host, it will only be removed if it has no children, - * or all its children are parked or failed. + * Returns the removal action for given node. + * + * If the node is a host, it will only be removed if it has no children, or all its children are parked or failed. + * * Otherwise, a removal is allowed if either of these are true: * - The node has been in state {@link History.Event.Type#retired} for longer than {@link #retiredExpiry} * - Orchestrator allows it */ - private boolean canRemove(Node node, NodeList activeNodes) { + private Removal removalOf(Node node, NodeList activeNodes) { if (node.type().isHost()) { if (nodeRepository().nodes().list().childrenOf(node).asList().stream() .allMatch(child -> child.state() == Node.State.parked || child.state() == Node.State.failed)) { log.info("Allowing removal of " + node + ": host has no non-parked/failed children"); - return true; + return Removal.reusable(); // Hosts have no state that needs to be recoverable } - - return false; + return Removal.none(); } if (node.type().isConfigServerLike()) { @@ -114,24 +118,32 @@ public class RetiredExpirer extends NodeRepositoryMaintainer { // with node states across all config servers. As this would require some work, // we will instead verify here that there are 3 active config servers before // allowing the removal of any config server. - return false; + return Removal.none(); } } else if (node.history().hasEventBefore(History.Event.Type.retired, clock().instant().minus(retiredExpiry))) { log.warning("Node " + node + " has been retired longer than " + retiredExpiry + ": Allowing removal. This may cause data loss"); - return true; + return Removal.recoverable(); } try { nodeRepository().orchestrator().acquirePermissionToRemove(new HostName(node.hostname())); log.info("Node " + node + " has been granted permission to be removed"); - return true; + return Removal.reusable(); // Node is fully retired } catch (UncheckedTimeoutException e) { log.warning("Timed out trying to acquire permission to remove " + node.hostname() + ": " + Exceptions.toMessageString(e)); - return false; + return Removal.none(); } catch (OrchestrationException e) { log.info("Did not get permission to remove retired " + node + ": " + Exceptions.toMessageString(e)); - return false; + return Removal.none(); } } + private record Removal(boolean isRemovable, boolean isReusable) { + + private static Removal recoverable() { return new Removal(true, false); } + private static Removal reusable() { return new Removal(true, true); } + private static Removal none() { return new Removal(false, false); } + + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Allocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Allocation.java index 9c71d621c69..ec1c6cd4b1a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Allocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Allocation.java @@ -27,25 +27,33 @@ public class Allocation { */ private final Generation restartGeneration; - /** This node can (and should) be removed from the cluster on the next deployment */ + /** This node should be removed from the cluster on the next deployment */ private final boolean removable; + /** This node can be reused (dirtied) immediately after being removed from the cluster */ + private final boolean reusable; + private final Optional<NetworkPorts> networkPorts; public Allocation(ApplicationId owner, ClusterMembership clusterMembership, NodeResources requestedResources, Generation restartGeneration, boolean removable) { - this(owner, clusterMembership, requestedResources, restartGeneration, removable, Optional.empty()); + this(owner, clusterMembership, requestedResources, restartGeneration, removable, false, Optional.empty()); } public Allocation(ApplicationId owner, ClusterMembership clusterMembership, NodeResources requestedResources, - Generation restartGeneration, boolean removable, Optional<NetworkPorts> networkPorts) { + Generation restartGeneration, boolean removable, boolean reusable, + Optional<NetworkPorts> networkPorts) { this.owner = owner; this.clusterMembership = clusterMembership; this.requestedResources = requestedResources; this.restartGeneration = restartGeneration; this.removable = removable; + this.reusable = reusable; this.networkPorts = networkPorts; + if (!removable && reusable) { + throw new IllegalArgumentException("Allocation must be removable in order to be reusable"); + } } /** Returns the id of the application this is allocated to */ @@ -65,37 +73,45 @@ public class Allocation { /** Returns a copy of this which is retired */ public Allocation retire() { - return new Allocation(owner, clusterMembership.retire(), requestedResources, restartGeneration, removable, networkPorts); + return new Allocation(owner, clusterMembership.retire(), requestedResources, restartGeneration, removable, reusable, networkPorts); } /** Returns a copy of this which is not retired */ public Allocation unretire() { - return new Allocation(owner, clusterMembership.unretire(), requestedResources, restartGeneration, removable, networkPorts); + return new Allocation(owner, clusterMembership.unretire(), requestedResources, restartGeneration, removable, reusable, networkPorts); } - /** Return whether this node is ready to be removed from the application */ - public boolean isRemovable() { return removable; } + /** Returns whether this node is ready to be removed from the application */ + public boolean removable() { return removable; } + + /** Returns whether this node has fully retired and can be reused immediately */ + public boolean reusable() { + return reusable; + } public Allocation withRequestedResources(NodeResources resources) { - return new Allocation(owner, clusterMembership, resources, restartGeneration, removable, networkPorts); + return new Allocation(owner, clusterMembership, resources, restartGeneration, removable, reusable, networkPorts); } /** Returns a copy of this with the current restart generation set to generation */ public Allocation withRestart(Generation generation) { - return new Allocation(owner, clusterMembership, requestedResources, generation, removable, networkPorts); + return new Allocation(owner, clusterMembership, requestedResources, generation, removable, reusable, networkPorts); } - /** Returns a copy of this allocation where removable is set to the given value */ - public Allocation removable(boolean removable) { - return new Allocation(owner, clusterMembership, requestedResources, restartGeneration, removable, networkPorts); + /** + * Returns a copy of this allocation where removable and reusable are set to the given values. A node which is + * reusable may be moved directly to {@link com.yahoo.vespa.hosted.provision.Node.State#dirty} after removal. + */ + public Allocation removable(boolean removable, boolean reusable) { + return new Allocation(owner, clusterMembership, requestedResources, restartGeneration, removable, reusable, networkPorts); } public Allocation with(ClusterMembership newMembership) { - return new Allocation(owner, newMembership, requestedResources, restartGeneration, removable, networkPorts); + return new Allocation(owner, newMembership, requestedResources, restartGeneration, removable, reusable, networkPorts); } public Allocation withNetworkPorts(NetworkPorts ports) { - return new Allocation(owner, clusterMembership, requestedResources, restartGeneration, removable, Optional.of(ports)); + return new Allocation(owner, clusterMembership, requestedResources, restartGeneration, removable, reusable, Optional.of(ports)); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index 11d3e03e494..d750a3ef737 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -226,13 +226,14 @@ public class Nodes { * Sets a list of nodes to have their allocation removable (active to inactive) in the node repository. * * @param application the application the nodes belong to - * @param nodes the nodes to make removable. These nodes MUST be in the active state. + * @param nodes the nodes to make removable. These nodes MUST be in the active state + * @param reusable move the node directly to {@link Node.State#dirty} after removal */ - public void setRemovable(ApplicationId application, List<Node> nodes) { + public void setRemovable(ApplicationId application, List<Node> nodes, boolean reusable) { try (Mutex lock = lock(application)) { List<Node> removableNodes = nodes.stream() - .map(node -> node.with(node.allocation().get().removable(true))) - .collect(Collectors.toList()); + .map(node -> node.with(node.allocation().get().removable(true, reusable))) + .toList(); write(removableNodes, lock); } } @@ -247,9 +248,12 @@ public class Nodes { var stateless = NodeList.copyOf(nodes).stateless(); var stateful = NodeList.copyOf(nodes).stateful(); + var statefulToInactive = stateful.not().reusable(); + var statefulToDirty = stateful.reusable(); List<Node> written = new ArrayList<>(); written.addAll(deallocate(stateless.asList(), Agent.application, "Deactivated by application", transaction.nested())); - written.addAll(db.writeTo(Node.State.inactive, stateful.asList(), Agent.application, Optional.empty(), transaction.nested())); + written.addAll(deallocate(statefulToDirty.asList(), Agent.application, "Deactivated by application (recycled)", transaction.nested())); + written.addAll(db.writeTo(Node.State.inactive, statefulToInactive.asList(), Agent.application, Optional.empty(), transaction.nested())); return written; } 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 083b707b3c5..24c52f7b2e0 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 @@ -115,6 +115,7 @@ public class NodeSerializer { private static final String restartGenerationKey = "restartGeneration"; private static final String currentRestartGenerationKey = "currentRestartGeneration"; private static final String removableKey = "removable"; + private static final String reusableKey = "reusable"; // Saved as part of allocation instead of serviceId, since serviceId serialized form is not easily extendable. private static final String wantedVespaVersionKey = "wantedVespaVersion"; private static final String wantedContainerImageRepoKey = "wantedDockerImageRepo"; @@ -217,7 +218,8 @@ public class NodeSerializer { object.setString(serviceIdKey, allocation.membership().stringValue()); object.setLong(restartGenerationKey, allocation.restartGeneration().wanted()); object.setLong(currentRestartGenerationKey, allocation.restartGeneration().current()); - object.setBool(removableKey, allocation.isRemovable()); + object.setBool(removableKey, allocation.removable()); + object.setBool(reusableKey, allocation.reusable()); object.setString(wantedVespaVersionKey, allocation.membership().cluster().vespaVersion().toString()); allocation.membership().cluster().dockerImageRepo().ifPresent(repo -> object.setString(wantedContainerImageRepoKey, repo.untagged())); allocation.networkPorts().ifPresent(ports -> NetworkPortsSerializer.toSlime(ports, object.setArray(networkPortsKey))); @@ -330,6 +332,7 @@ public class NodeSerializer { .orElse(assignedResources), generationFromSlime(object, restartGenerationKey, currentRestartGenerationKey), object.field(removableKey).asBool(), + object.field(reusableKey).asBool(), NetworkPortsSerializer.fromSlime(object.field(networkPortsKey)))); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java index 3da0506f2e1..5d73c4929af 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java @@ -88,8 +88,7 @@ class Activator { validateParentHosts(application, allNodes, reserved); NodeList activeToRemove = oldActive.matching(node -> ! hostnames.contains(node.hostname())); - activeToRemove = NodeList.copyOf(activeToRemove.mapToList(Node::unretire)); // only active nodes can be retired. TODO: Move this line to deactivate - deactivate(activeToRemove, transaction); // TODO: Pass activation time in this call and next line + remove(activeToRemove, transaction); // TODO: Pass activation time in this call and next line nodeRepository.nodes().activate(newActive.asList(), transaction.nested()); // activate also continued active to update node state rememberResourceChange(transaction, generation, activationTime, @@ -99,9 +98,10 @@ class Activator { return newActive; } - private void deactivate(NodeList toDeactivate, ApplicationTransaction transaction) { - nodeRepository.nodes().deactivate(toDeactivate.not().failing().asList(), transaction); - nodeRepository.nodes().fail(toDeactivate.failing().asList(), transaction); + private void remove(NodeList nodes, ApplicationTransaction transaction) { + nodes = NodeList.copyOf(nodes.mapToList(Node::unretire)); // clear retire flag when moving to non-active state + nodeRepository.nodes().deactivate(nodes.not().failing().asList(), transaction); + nodeRepository.nodes().fail(nodes.failing().asList(), transaction); } private void rememberResourceChange(ApplicationTransaction transaction, long generation, Instant at, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index a26df62be27..9557354725b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -112,7 +112,7 @@ class NodeAllocation { if ( ! allocation.owner().equals(application)) continue; // wrong application if ( ! membership.cluster().satisfies(cluster)) continue; // wrong cluster id/type if ((! candidate.isSurplus || saturated()) && ! membership.cluster().group().equals(cluster.group())) continue; // wrong group and we can't or have no reason to change it - if ( candidate.state() == Node.State.active && allocation.isRemovable()) continue; // don't accept; causes removal + if ( candidate.state() == Node.State.active && allocation.removable()) continue; // don't accept; causes removal if ( candidate.state() == Node.State.active && candidate.wantToFail()) continue; // don't accept; causes failing if ( indexes.contains(membership.index())) continue; // duplicate index (just to be sure) diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java index 0c164328086..94b938fc886 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java @@ -124,7 +124,7 @@ class AutoscalingTester { try (Mutex lock = nodeRepository().nodes().lock(application)){ for (Node node : nodeRepository().nodes().list(Node.State.active).owner(application)) { if (node.allocation().get().membership().retired()) - nodeRepository().nodes().write(node.with(node.allocation().get().removable(true)), lock); + nodeRepository().nodes().write(node.with(node.allocation().get().removable(true, true)), lock); } } deploy(application, cluster, resources); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index 30d0f673fe1..905fdc57813 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -457,7 +457,7 @@ public class DynamicProvisioningMaintainerTest { // Initial config server hosts are provisioned manually List<Node> provisionedHosts = tester.makeReadyNodes(3, "default", hostType, 1).stream() .sorted(Comparator.comparing(Node::hostname)) - .collect(Collectors.toList()); + .toList(); tester.prepareAndActivateInfraApplication(hostApp, hostType); // Provision config servers @@ -485,18 +485,14 @@ public class DynamicProvisioningMaintainerTest { assertTrue("Redeployment retires node", nodeToRemove.get().allocation().get().membership().retired()); // Config server becomes removable (done by RetiredExpirer in a real system) and redeployment moves it - // to inactive - tester.nodeRepository().nodes().setRemovable(configSrvApp, List.of(nodeToRemove.get())); - tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); - assertEquals("Node moves to inactive", Node.State.inactive, nodeToRemove.get().state()); - - // Node is parked (done by InactiveExpirer and host-admin in a real system) + // to parked int removedIndex = nodeToRemove.get().allocation().get().membership().index(); - Node parkedConfigServer = tester.nodeRepository().nodes().deallocate(nodeToRemove.get(), Agent.system, getClass().getSimpleName()); - assertSame("Node moves to parked", Node.State.parked, parkedConfigServer.state()); + tester.nodeRepository().nodes().setRemovable(configSrvApp, List.of(nodeToRemove.get()), true); + tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); + assertSame("Node moves to expected state", Node.State.parked, nodeToRemove.get().state()); assertEquals(2, tester.nodeRepository().nodes().list().nodeType(hostType.childNodeType()).state(Node.State.active).size()); - // ... same for host + // Host is parked (done by DynamicProvisioningMaintainer in a real system) tester.nodeRepository().nodes().deallocate(hostToRemove.get(), Agent.system, getClass().getSimpleName()); assertSame("Host moves to parked", Node.State.parked, hostToRemove.get().state()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java index 1237ede5345..bf98a875e42 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java @@ -112,7 +112,7 @@ public class RetiredExpirerTest { } @Test - public void ensure_early_inactivation() throws OrchestrationException { + public void retired_nodes_are_dellocated() throws OrchestrationException { tester.makeReadyNodes(7, nodeResources); tester.makeReadyHosts(4, hostResources); @@ -148,27 +148,35 @@ public class RetiredExpirerTest { RetiredExpirer retiredExpirer = createRetiredExpirer(deployer); retiredExpirer.run(); assertEquals(5, nodeRepository.nodes().list(Node.State.active).owner(applicationId).size()); - assertEquals(2, nodeRepository.nodes().list(Node.State.inactive).owner(applicationId).size()); + assertEquals(2, nodeRepository.nodes().list(Node.State.dirty).owner(applicationId).size()); assertEquals(1, deployer.redeployments); verify(orchestrator, times(4)).acquirePermissionToRemove(any()); // Running it again has no effect retiredExpirer.run(); assertEquals(5, nodeRepository.nodes().list(Node.State.active).owner(applicationId).size()); - assertEquals(2, nodeRepository.nodes().list(Node.State.inactive).owner(applicationId).size()); + assertEquals(2, nodeRepository.nodes().list(Node.State.dirty).owner(applicationId).size()); assertEquals(1, deployer.redeployments); verify(orchestrator, times(6)).acquirePermissionToRemove(any()); + // Running it again deactivates nodes that have exceeded max retirement period clock.advance(RETIRED_EXPIRATION.plusMinutes(1)); retiredExpirer.run(); assertEquals(3, nodeRepository.nodes().list(Node.State.active).owner(applicationId).size()); - assertEquals(4, nodeRepository.nodes().list(Node.State.inactive).owner(applicationId).size()); + assertEquals(2, nodeRepository.nodes().list(Node.State.dirty).owner(applicationId).size()); + assertEquals(2, nodeRepository.nodes().list(Node.State.inactive).owner(applicationId).size()); assertEquals(2, deployer.redeployments); verify(orchestrator, times(6)).acquirePermissionToRemove(any()); - // inactivated nodes are not retired - for (Node node : nodeRepository.nodes().list(Node.State.inactive).owner(applicationId)) + // Removed nodes are not retired + for (Node node : nodeRepository.nodes().list(Node.State.inactive, Node.State.dirty).owner(applicationId)) { + if (node.state() == Node.State.inactive) { + assertFalse(node + " node is reusable", node.allocation().get().reusable()); + } else { + assertTrue(node + " is reusable", node.allocation().get().reusable()); + } assertFalse(node.allocation().get().membership().retired()); + } } @Test @@ -207,19 +215,19 @@ public class RetiredExpirerTest { assertTrue(activeConfigServerHostnames.contains(retiredNode.hostname())); activeConfigServerHostnames.remove(retiredNode.hostname()); assertEquals(activeConfigServerHostnames, configServerHostnames(duperModel)); - assertEquals(1, tester.nodeRepository().nodes().list(Node.State.inactive).nodeType(NodeType.config).size()); + assertEquals(1, tester.nodeRepository().nodes().list(Node.State.parked).nodeType(NodeType.config).size()); assertEquals(2, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.config).size()); - // no changes while 1 cfg is inactive + // no changes while 1 cfg is dirty retiredExpirer.run(); assertEquals(activeConfigServerHostnames, configServerHostnames(duperModel)); - assertEquals(1, tester.nodeRepository().nodes().list(Node.State.inactive).nodeType(NodeType.config).size()); + NodeList parked = tester.nodeRepository().nodes().list(Node.State.parked).nodeType(NodeType.config); + assertEquals(1, parked.size()); assertEquals(2, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.config).size()); - // The node will eventually expire from inactive, and be removed by DynamicProvisioningMaintainer - // (depending on its host), and these events should not affect the 2 active config servers. - nodeRepository.nodes().deallocate(retiredNode, Agent.InactiveExpirer, "expired"); - retiredNode = tester.nodeRepository().nodes().list(Node.State.parked).nodeType(NodeType.config).asList().get(0); + // Node is removed by DynamicProvisioningMaintainer (depending on its host), and these events should not affect + // the 2 active config servers. + retiredNode = parked.first().get(); nodeRepository.nodes().removeRecursively(retiredNode, true); infraDeployer.activateAllSupportedInfraApplications(true); retiredExpirer.run(); @@ -228,7 +236,7 @@ public class RetiredExpirerTest { assertEquals(2, tester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.config).size()); // Provision and ready new config server - MockNameResolver nameResolver = (MockNameResolver)tester.nodeRepository().nameResolver(); + MockNameResolver nameResolver = (MockNameResolver) tester.nodeRepository().nameResolver(); String ipv4 = "127.0.1.4"; nameResolver.addRecord(retiredNode.hostname(), ipv4); Node node = Node.create(retiredNode.hostname(), new IP.Config(Set.of(ipv4), Set.of()), retiredNode.hostname(), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java index 26be6d95336..97a8ac0d655 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java @@ -469,7 +469,7 @@ public class OsVersionsTest { deployApplication(application); List<Node> retired = tester.nodeRepository().nodes().list().owner(application).retired().asList(); assertFalse("At least one node is retired", retired.isEmpty()); - tester.nodeRepository().nodes().setRemovable(application, retired); + tester.nodeRepository().nodes().setRemovable(application, retired, false); // Redeploy to deactivate removable nodes and allocate new ones deployApplication(application); 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 bc7d3104a2f..f22abf4c9e3 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 @@ -114,7 +114,7 @@ public class NodeSerializerTest { assertEquals(node.allocation().get().owner(), copy.allocation().get().owner()); assertEquals(node.allocation().get().membership(), copy.allocation().get().membership()); assertEquals(node.allocation().get().requestedResources(), copy.allocation().get().requestedResources()); - assertEquals(node.allocation().get().isRemovable(), copy.allocation().get().isRemovable()); + assertEquals(node.allocation().get().removable(), copy.allocation().get().removable()); assertEquals(4, copy.history().events().size()); assertEquals(5, copy.history().log().size()); assertEquals(Instant.ofEpochMilli(1), copy.history().event(History.Event.Type.reserved).get().at()); @@ -167,7 +167,7 @@ public class NodeSerializerTest { assertEquals(4, node.allocation().get().restartGeneration().current()); assertEquals(List.of(History.Event.Type.provisioned, History.Event.Type.reserved), node.history().events().stream().map(History.Event::type).collect(Collectors.toList())); - assertTrue(node.allocation().get().isRemovable()); + assertTrue(node.allocation().get().removable()); assertEquals(NodeType.tenant, node.type()); } @@ -193,9 +193,10 @@ public class NodeSerializerTest { (copy.history().event(History.Event.Type.retired).get()).agent()); assertTrue(copy.allocation().get().membership().retired()); - Node removable = copy.with(node.allocation().get().removable(true)); + Node removable = copy.with(node.allocation().get().removable(true, true)); Node removableCopy = nodeSerializer.fromJson(Node.State.provisioned, nodeSerializer.toJson(removable)); - assertTrue(removableCopy.allocation().get().isRemovable()); + assertTrue(removableCopy.allocation().get().removable()); + assertTrue(removableCopy.allocation().get().reusable()); } @Test diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java index 4f2360590c4..5149b234ccf 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java @@ -125,7 +125,7 @@ public class InfraDeployerImplTest { addNode(5, Node.State.dirty, Optional.empty()); addNode(6, Node.State.ready, Optional.empty()); Node node7 = addNode(7, Node.State.active, Optional.of(target)); - nodeRepository.nodes().setRemovable(application.getApplicationId(), List.of(node7)); + nodeRepository.nodes().setRemovable(application.getApplicationId(), List.of(node7), false); infraDeployer.getDeployment(application.getApplicationId()).orElseThrow().activate(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java index 798ed1e45a7..a1c55833862 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java @@ -75,7 +75,7 @@ public class VirtualNodeProvisioningTest { // Go down to 3 nodes in container cluster List<HostSpec> containerHosts2 = tester.prepare(applicationId, containerClusterSpec, containerNodeCount - 1, groups, resources1); - tester.activate(applicationId, containerHosts2); + tester.activate(applicationId, concat(containerHosts2, contentHosts)); NodeList nodes2 = tester.getNodes(applicationId, Node.State.active); assertDistinctParentHosts(nodes2, ClusterSpec.Type.container, containerNodeCount - 1); @@ -637,7 +637,7 @@ public class VirtualNodeProvisioningTest { tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, r))); // Deactivate any retired nodes - usually done by the RetiredExpirer - tester.nodeRepository().nodes().setRemovable(app1, tester.getNodes(app1).retired().asList()); + tester.nodeRepository().nodes().setRemovable(app1, tester.getNodes(app1).retired().asList(), false); tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, r))); if (expectedReuse) { @@ -691,7 +691,7 @@ public class VirtualNodeProvisioningTest { } private static <T> List<T> concat(List<T> list1, List<T> list2) { - return Stream.concat(list1.stream(), list2.stream()).collect(Collectors.toList()); + return Stream.concat(list1.stream(), list2.stream()).toList(); } } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp index 9e05acb5935..24642c418fd 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp @@ -87,11 +87,12 @@ DynamicDocsumConfig::createFieldWriter(const string & fieldName, const string & } else if (overrideName == "attributecombiner") { if (getEnvironment() && getEnvironment()->getAttributeManager()) { auto attr_ctx = getEnvironment()->getAttributeManager()->createContext(); - fieldWriter = AttributeCombinerDFW::create(fieldName, *attr_ctx, false, std::shared_ptr<MatchingElementsFields>()); + const string& source_field = argument.empty() ? fieldName : argument; + fieldWriter = AttributeCombinerDFW::create(source_field, *attr_ctx, false, std::shared_ptr<MatchingElementsFields>()); rc = static_cast<bool>(fieldWriter); } } else if (overrideName == "matchedattributeelementsfilter") { - string source_field = argument.empty() ? fieldName : argument; + const string& source_field = argument.empty() ? fieldName : argument; if (getEnvironment() && getEnvironment()->getAttributeManager()) { auto attr_ctx = getEnvironment()->getAttributeManager()->createContext(); if (attr_ctx->getAttribute(source_field) != nullptr) { @@ -102,7 +103,7 @@ DynamicDocsumConfig::createFieldWriter(const string & fieldName, const string & rc = static_cast<bool>(fieldWriter); } } else if (overrideName == "matchedelementsfilter") { - string source_field = argument.empty() ? fieldName : argument; + const string& source_field = argument.empty() ? fieldName : argument; if (getEnvironment() && getEnvironment()->getAttributeManager()) { auto attr_ctx = getEnvironment()->getAttributeManager()->createContext(); fieldWriter = MatchedElementsFilterDFW::create(source_field, resultConfig.GetFieldNameEnum().Lookup(source_field.c_str()), diff --git a/storage/src/vespa/storage/config/stor-communicationmanager.def b/storage/src/vespa/storage/config/stor-communicationmanager.def index 6090e6bcd08..c22cf7b8b01 100644 --- a/storage/src/vespa/storage/config/stor-communicationmanager.def +++ b/storage/src/vespa/storage/config/stor-communicationmanager.def @@ -65,11 +65,6 @@ mbus.skip_request_thread bool default=false restart ## Experimental skip_thread bool default=false -## Whether to use direct P2P RPC protocol for all StorageAPI communication -## instead of going via MessageBus. -## Deprecated, and will soon be gone as it is default on. -use_direct_storageapi_rpc bool default=true - ## The number of network (FNET) threads used by the shared rpc resource. rpc.num_network_threads int default=2 restart diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java index 13a61d65d78..197af753442 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java @@ -116,22 +116,23 @@ public class DefaultZtsClient extends ClientBase implements ZtsClient { } @Override - public ZToken getRoleToken(AthenzDomain domain) { - return getRoleToken(domain, null); + public ZToken getRoleToken(AthenzDomain domain, Duration expiry) { + return getRoleToken(domain, null, expiry); } @Override - public ZToken getRoleToken(AthenzRole athenzRole) { - return getRoleToken(athenzRole.domain(), athenzRole.roleName()); + public ZToken getRoleToken(AthenzRole athenzRole, Duration expiry) { + return getRoleToken(athenzRole.domain(), athenzRole.roleName(), expiry); } - private ZToken getRoleToken(AthenzDomain domain, String roleName) { + private ZToken getRoleToken(AthenzDomain domain, String roleName, Duration expiry) { URI uri = ztsUrl.resolve(String.format("domain/%s/token", domain.getName())); RequestBuilder requestBuilder = RequestBuilder.get(uri) .addHeader("Content-Type", "application/json"); if (roleName != null) { requestBuilder.addParameter("role", roleName); } + requestBuilder.addParameter("maxExpiryTime", Long.toString(expiry.getSeconds())); HttpUriRequest request = requestBuilder.build(); return execute(request, response -> { RoleTokenResponseEntity roleTokenResponseEntity = readEntity(response, RoleTokenResponseEntity.class); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java index e52abc4193b..30c8ab2fd50 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java @@ -68,15 +68,37 @@ public interface ZtsClient extends AutoCloseable { * @param domain Target domain * @return A role token */ - ZToken getRoleToken(AthenzDomain domain); + default ZToken getRoleToken(AthenzDomain domain) { + return getRoleToken(domain, Duration.ofHours(1)); + } + + /** + * Fetch a role token for the target domain + * + * @param domain Target domain + * @param tokenExpiry Token expiry + * @return A role token + */ + ZToken getRoleToken(AthenzDomain domain, Duration tokenExpiry); + + /** + * Fetch a role token for the target role + * + * @param athenzRole Target role + * @return A role token + */ + default ZToken getRoleToken(AthenzRole athenzRole) { + return getRoleToken(athenzRole, Duration.ofHours(1)); + } /** * Fetch a role token for the target role * * @param athenzRole Target role + * @param tokenExpiry Token expiry * @return A role token */ - ZToken getRoleToken(AthenzRole athenzRole); + ZToken getRoleToken(AthenzRole athenzRole, Duration tokenExpiry); /** * Fetch an access token for the target domain diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java index 11be0daf2d4..c92f7259e77 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java @@ -68,7 +68,8 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen static final Duration UPDATE_PERIOD = Duration.ofDays(1); static final Duration AWAIT_TERMINTATION_TIMEOUT = Duration.ofSeconds(90); private final static Duration ROLE_SSL_CONTEXT_EXPIRY = Duration.ofHours(2); - private final static Duration ROLE_TOKEN_EXPIRY = Duration.ofMinutes(30); + // TODO CMS expects 10min or less token ttl. Use 10min default until we have configurable expiry + private final static Duration ROLE_TOKEN_EXPIRY = Duration.ofMinutes(10); // TODO Make path to trust store paths config private static final Path CLIENT_TRUST_STORE = Paths.get("/opt/yahoo/share/ssl/certs/yahoo_certificate_bundle.pem"); @@ -209,7 +210,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen try { AthenzRole athenzRole = new AthenzRole(new AthenzDomain(domain), role); // Make sure to request a certificate which triggers creating a new key manager for this role - X509Certificate x509Certificate = roleSslCertCache.get(athenzRole); + X509Certificate x509Certificate = getRoleCertificate(athenzRole); MutableX509KeyManager keyManager = roleKeyManagerCache.get(athenzRole); return new SslContextBuilder() .withKeyManager(keyManager) @@ -274,6 +275,19 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen return Collections.singletonList(credentials.getCertificate()); } + @Override + public X509Certificate getRoleCertificate(String domain, String role) { + return getRoleCertificate(new AthenzRole(new AthenzDomain(domain), role)); + } + + private X509Certificate getRoleCertificate(AthenzRole athenzRole) { + try { + return roleSslCertCache.get(athenzRole); + } catch (Exception e) { + throw new AthenzIdentityProviderException("Could not retrieve role certificate: " + e.getMessage(), e); + } + } + private void updateIdentityCredentials(AthenzCredentials credentials) { this.credentials = credentials; this.identityKeyManager.updateKeystore( @@ -304,13 +318,13 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen private ZToken createRoleToken(AthenzRole athenzRole) { try (ZtsClient client = createZtsClient()) { - return client.getRoleToken(athenzRole); + return client.getRoleToken(athenzRole, ROLE_TOKEN_EXPIRY); } } private ZToken createRoleToken(AthenzDomain domain) { try (ZtsClient client = createZtsClient()) { - return client.getRoleToken(domain); + return client.getRoleToken(domain, ROLE_TOKEN_EXPIRY); } } diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java index 4bf40323193..b1ca6c84b75 100644 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java @@ -197,7 +197,7 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler { nodeObject.setString("name", node.name()); nodeObject.setString("status", node.status().name()); nodeObject.setLong("start", node.start().toEpochMilli()); - nodeObject.setLong("end", node.duration().toMillis()); + nodeObject.setLong("duration", node.duration().toMillis()); } static void toSlime(Cursor nodeObject, TestNode node) { @@ -205,7 +205,7 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler { nodeObject.setString("name", node.name()); nodeObject.setString("status", node.status().name()); nodeObject.setLong("start", node.start().toEpochMilli()); - nodeObject.setLong("end", node.duration().toMillis()); + nodeObject.setLong("duration", node.duration().toMillis()); } static void toSlime(Cursor nodeObject, OutputNode node) { diff --git a/vespa-osgi-testrunner/src/test/resources/report.json b/vespa-osgi-testrunner/src/test/resources/report.json index 69b11b40ed9..9c41a83a6b5 100644 --- a/vespa-osgi-testrunner/src/test/resources/report.json +++ b/vespa-osgi-testrunner/src/test/resources/report.json @@ -4,28 +4,28 @@ "name": "Production test", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "container", "name": "JUnit Jupiter", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "container", "name": "SampleTest", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "test", "name": "aborted()", "status": "aborted", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -44,7 +44,7 @@ "name": "error()", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -68,7 +68,7 @@ "name": "failing()", "status": "failed", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -97,14 +97,14 @@ "name": "ignored()", "status": "skipped", "start": 0, - "end": 0 + "duration": 0 }, { "type": "test", "name": "inconclusive(TestReporter)", "status": "inconclusive", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -133,7 +133,7 @@ "name": "successful()", "status": "successful", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -168,14 +168,14 @@ "name": "Inner", "status": "failed", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "container", "name": "others()", "status": "failed", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -192,7 +192,7 @@ "name": "second", "status": "successful", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -211,7 +211,7 @@ "name": "third", "status": "failed", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "failure", @@ -227,7 +227,7 @@ "name": "first()", "status": "successful", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "output", @@ -248,14 +248,14 @@ "name": "Skipped", "status": "skipped", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "test", "name": "disabled()", "status": "skipped", "start": 0, - "end": 0 + "duration": 0 } ] } @@ -268,21 +268,21 @@ "name": "JUnit Jupiter", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "container", "name": "FailingTestAndBothAftersTest", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "test", "name": "test()", "status": "failed", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "failure", @@ -303,7 +303,7 @@ "name": "WrongBeforeAllTest", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "failure", @@ -315,14 +315,14 @@ "name": "fest()", "status": "skipped", "start": 0, - "end": 0 + "duration": 0 }, { "type": "test", "name": "test()", "status": "skipped", "start": 0, - "end": 0 + "duration": 0 } ] }, @@ -331,14 +331,14 @@ "name": "FailingExtensionTest", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "test", "name": "test()", "status": "error", "start": 0, - "end": 0, + "duration": 0, "children": [ { "type": "failure", diff --git a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java index 0d06a18b096..62639b10684 100644 --- a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java +++ b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java @@ -5,7 +5,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; @@ -21,7 +20,7 @@ import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; -import static java.util.stream.Collectors.toUnmodifiableList; +import static java.util.stream.Collectors.toList; /** * Abstract, immutable list for subclassing with concrete types and domain specific filters. @@ -51,7 +50,7 @@ public abstract class AbstractFilteringList<Type, ListType extends AbstractFilte /** Returns a new list which is the result of filtering with the -- possibly negated -- condition. */ public final ListType matching(Predicate<Type> condition) { - return constructor.apply(items.stream().filter(negate ? condition.negate() : condition).collect(toUnmodifiableList()), false); + return constructor.apply(items.stream().filter(negate ? condition.negate() : condition).toList(), false); } /** Returns the first n items in this list, or everything except those if negated. */ @@ -72,7 +71,7 @@ public abstract class AbstractFilteringList<Type, ListType extends AbstractFilte /** Returns the union of the two lists. */ public ListType and(ListType others) { - return constructor.apply(Stream.concat(items.stream(), others.asList().stream()).collect(toUnmodifiableList()), false); + return constructor.apply(Stream.concat(items.stream(), others.asList().stream()).toList(), false); } /** Returns the items in this as an immutable list. */ @@ -83,19 +82,19 @@ public abstract class AbstractFilteringList<Type, ListType extends AbstractFilte /** Returns the items in this as an immutable list after mapping with the given function. */ public final <OtherType> List<OtherType> mapToList(Function<Type, OtherType> mapper) { - return items.stream().map(mapper).collect(toUnmodifiableList()); + return items.stream().map(mapper).toList(); } /** Returns the items sorted by the given comparator. */ public final ListType sortedBy(Comparator<? super Type> comparator) { - return constructor.apply(items.stream().sorted(comparator).collect(toUnmodifiableList()), false); + return constructor.apply(items.stream().sorted(comparator).toList(), false); } /** Returns the items grouped by the given classifier. */ public final <OtherType> Map<OtherType, ListType> groupingBy(Function<Type, OtherType> classifier) { return items.stream().collect(Collectors.groupingBy(classifier, LinkedHashMap::new, - Collectors.collectingAndThen(toUnmodifiableList(), + Collectors.collectingAndThen(toList(), (list) -> constructor.apply(list, false)))); } |