diff options
6 files changed, 33 insertions, 31 deletions
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 079383dd808..09ddcf4fa5e 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,18 +1,14 @@ // 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; import java.util.List; -import java.util.stream.Collectors; import java.util.stream.Stream; import static ai.vespa.validation.Validation.require; @@ -22,7 +18,6 @@ import static com.yahoo.config.provision.Environment.prod; import static com.yahoo.config.provision.Environment.staging; import static com.yahoo.config.provision.Environment.test; import static java.util.Comparator.naturalOrder; -import static java.util.stream.Collectors.toUnmodifiableList; /** * Specification for a deployment and/or test job to run: what zone, and whether it is a production test. @@ -31,6 +26,8 @@ import static java.util.stream.Collectors.toUnmodifiableList; */ public final class JobType implements Comparable<JobType> { + private static final RegionName unknown = RegionName.from("unknown"); + private final String jobName; private final ZoneId zone; private final boolean isProductionTest; @@ -53,8 +50,11 @@ public final class JobType implements Comparable<JobType> { /** 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) { + if (cloud == null) + return deploymentTo(ZoneId.from(environment, unknown)); + ZoneList candidates = zones.zones().controllerUpgraded().in(environment); - if (cloud == null || candidates.in(cloud).zones().isEmpty()) + if (candidates.in(cloud).zones().isEmpty()) cloud = zones.systemZone().getCloudName(); return candidates.in(cloud).zones().stream().findFirst() @@ -152,7 +152,8 @@ public final class JobType implements Comparable<JobType> { 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()))) + : zone.getEnvironment().isTest() ? Stream.of(deploymentTo(ZoneId.from(zone.getEnvironment(), unknown))) + : Stream.of(deploymentTo(zone.getId()))) .distinct() .sorted(naturalOrder()) .toList(); @@ -169,6 +170,10 @@ public final class JobType implements Comparable<JobType> { /** Returns the zone for this job. */ public ZoneId zone() { + // sigh ... but the alternative is worse. + if (zone.region() == unknown) + throw new IllegalStateException("this job type was not initiated with a proper zone, programming error"); + return zone; } @@ -203,7 +208,7 @@ public final class JobType implements Comparable<JobType> { @Override public int compareTo(JobType other) { int result; - if (0 != (result = environment().compareTo(other.environment()))) return -result; + if (0 != (result = environment().compareTo(other.environment())) || environment().isTest()) return -result; if (0 != (result = zone.region().compareTo(other.zone.region()))) return result; return Boolean.compare(isProductionTest, other.isProductionTest); } 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 9f93033a1a2..24cd92d005f 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 @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.google.common.collect.ImmutableMap; -import com.yahoo.collections.Iterables; import com.yahoo.component.Version; import com.yahoo.component.VersionCompatibility; import com.yahoo.config.application.api.DeploymentInstanceSpec; @@ -231,9 +230,9 @@ public class DeploymentStatus { 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); + CloudName cloud = firstProductionJobWithDeploymentInCloud.map(JobId::type).map(this::findCloud).orElse(zones.systemZone().getCloudName()); + JobType typeWithZone = job.type().isSystemTest() ? JobType.systemTest(zones, cloud) : JobType.stagingTest(zones, cloud); + jobs.merge(job, List.of(new Job(typeWithZone, versions, step.readyAt(change), change)), DeploymentStatus::union); } } }); @@ -291,19 +290,16 @@ public class DeploymentStatus { } private <T extends Comparable<T>> Optional<T> newestTested(InstanceName instance, Function<Run, T> runMapper) { - Set<CloudName> clouds = jobSteps.keySet().stream() - .filter(job -> job.type().isProduction()) - .map(job -> findCloud(job.type())) - .collect(toSet()); + Set<CloudName> clouds = Stream.concat(Stream.of(zones.systemZone().getCloudName()), + 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()); - } + if (application.deploymentSpec().requireInstance(instance).concerns(test)) + for (CloudName cloud: clouds) testZones.add(JobType.systemTest(zones, cloud).zone()); + if (application.deploymentSpec().requireInstance(instance).concerns(staging)) + 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)) @@ -580,7 +576,7 @@ public class DeploymentStatus { } private CloudName findCloud(JobType job) { - return zones.zones().all().get(job.zone()).map(ZoneApi::getCloudName).orElse(null); + return zones.zones().all().get(job.zone()).map(ZoneApi::getCloudName).orElse(zones.systemZone().getCloudName()); } private JobId firstDeclaredOrElseImplicitTest(JobType testJob) { 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 e28bf89e734..9c016eccd27 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 @@ -325,7 +325,7 @@ class JobControllerApiHandlerHelper { "/job/" + job.type().jobName()).normalize(); stepObject.setString("url", baseUriForJob.toString()); stepObject.setString("environment", job.type().environment().value()); - stepObject.setString("region", job.type().zone().value()); + if ( ! job.type().environment().isTest()) stepObject.setString("region", job.type().zone().value()); if (job.type().isProduction() && job.type().isDeployment()) { status.deploymentFor(job).ifPresent(deployment -> { 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 e5000a85f71..673cbf9708b 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 @@ -64,6 +64,7 @@ import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; /** @@ -2382,10 +2383,14 @@ public class DeploymentTriggerTest { 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("staging", "us-east-3"), JobType.stagingTest(zones, zones.systemZone().getCloudName()).zone()); assertEquals(ZoneId.from("test", "zone"), pinkSystemTest.zone()); assertEquals(ZoneId.from("staging", "us-east-3"), JobType.stagingTest(zones, CloudName.from("pink-clouds")).zone()); + + assertThrows(IllegalStateException.class, JobType.systemTest(zones, null)::zone); + assertThrows(IllegalStateException.class, JobType.fromJobName("system-test", zones)::zone); + assertThrows(IllegalStateException.class, JobType.fromJobName("staging-test", zones)::zone); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json index 481997102d2..a7e2d1913cf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json @@ -158,7 +158,6 @@ "jobName": "system-test", "url": "https://some.url:43/instance/default/job/system-test", "environment": "test", - "region": "test.us-east-1", "toRun": [ ], "runs": [ { @@ -348,7 +347,6 @@ "jobName": "staging-test", "url": "https://some.url:43/instance/default/job/staging-test", "environment": "staging", - "region": "staging.us-east-3", "toRun": [ { "versions": { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json index f39aab26d75..696f1ef0ba3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json @@ -62,7 +62,6 @@ "jobName": "system-test", "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/system-test", "environment": "test", - "region": "test.us-east-1", "toRun": [ ], "runs": [ { @@ -191,7 +190,6 @@ "jobName": "staging-test", "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/staging-test", "environment": "staging", - "region": "staging.us-east-3", "toRun": [ ], "runs": [ { |