diff options
author | jonmv <venstad@gmail.com> | 2022-06-28 08:17:31 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2022-06-28 08:17:31 +0200 |
commit | d17974c4f3d646b835a489e68bcf30081457e7f7 (patch) | |
tree | a3960466016502180083689659850378ee8880b3 | |
parent | c2142b2fe5129b50f75a235eb1a4a92e31ddb8c5 (diff) |
Revert "Merge pull request #23247 from vespa-engine/revert-23218-jonmv/multiple-test-and-staging-zones"
This reverts commit 94a5a63b92cf7a05ed987a5f01fab5bc8d56bd2d, reversing
changes made to 24c7eee36b9c251fc754e6ca51c921e97be44aeb.
16 files changed, 489 insertions, 300 deletions
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/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..3ecf350b0b5 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,24 @@ 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().map(zone -> deploymentTo(zone.getId())) + .findFirst().orElseThrow(() -> new IllegalArgumentException("no zones in " + environment + " among " + + zones.zones().controllerUpgraded().zones())); } /** A deployment to the given dev region. */ @@ -118,27 +128,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..ba54793a891 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(null), versions); + Optional<Instant> stagingTestedAt = testedAt(job.application(), stagingTest(null), 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())); } /** @@ -947,6 +999,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 -> dependent.isEmpty() || status.findCloud(dependent.get().type()).equals(status.findCloud(run.id().type()))) .asList().stream() .map(run -> run.end().get()) .max(naturalOrder()); @@ -961,16 +1014,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 +1043,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..eb6b06f9bec 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 @@ -382,7 +382,7 @@ public class DeploymentTrigger { .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(), + versionsList.get(0).type(), status.instanceJobs(job.application().instance()).get(job.type()), readyAt)) .ifPresent(jobs::add); 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/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; |