diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2020-05-18 17:32:17 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2020-05-19 09:58:05 +0200 |
commit | 8d608fd9754dc12b6460ec3a5a2955085b4000b9 (patch) | |
tree | 437867540f202ecac2098a307e159ed649bf8747 /controller-server | |
parent | 38e14d3a40775f2e010046ae36cf0505f7b6e5a6 (diff) |
Handle when an instance has only declared tests
Diffstat (limited to 'controller-server')
5 files changed, 90 insertions, 12 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java index 773c927b588..2a1874140a8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java @@ -66,10 +66,17 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL return matching(id -> instance(id).productionDeployments().size() > 0); } - /** Returns the subset of instances which have at least one deployment on a lower version than the given one */ + /** Returns the subset of instances which contain declared jobs */ + public InstanceList withDeclaredJobs() { + return matching(id -> statuses.get(id).jobSteps().values().stream() + .anyMatch(job -> job.isDeclared() && job.job().get().application().equals(id))); + } + + /** Returns the subset of instances which have at least one deployment on a lower version than the given one, or which have no production deployments */ public InstanceList onLowerVersionThan(Version version) { - return matching(id -> instance(id).productionDeployments().values().stream() - .anyMatch(deployment -> deployment.version().isBefore(version))); + return matching(id -> instance(id).productionDeployments().isEmpty() + || instance(id).productionDeployments().values().stream() + .anyMatch(deployment -> deployment.version().isBefore(version))); } /** Returns the subset of instances which have changes left to deploy; blocked, or deploying */ 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 3d5a181b987..07dc8dbe7e8 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 @@ -151,13 +151,31 @@ public class DeploymentStatus { ImmutableMap::copyOf)); } - /** The set of jobs that need to run for the given change to be considered complete. */ + /** The set of jobs that need to run for the given changes to be considered complete. */ public Map<JobId, List<Versions>> jobsToRun(Map<InstanceName, Change> changes) { Map<JobId, Versions> productionJobs = new LinkedHashMap<>(); changes.forEach((instance, change) -> productionJobs.putAll(productionJobs(instance, change))); Map<JobId, List<Versions>> testJobs = testJobs(productionJobs); Map<JobId, List<Versions>> jobs = new LinkedHashMap<>(testJobs); productionJobs.forEach((job, versions) -> jobs.put(job, List.of(versions))); + // Add runs for idle, declared test jobs if they have no successes on their instance's change's versions. + jobSteps.forEach((job, step) -> { + if ( ! step.isDeclared() || jobs.containsKey(job)) + return; + + Change change = changes.get(job.application().instance()); + 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), systemVersion); + if (step.completedAt(change, firstProductionJobWithDeployment).isEmpty()) + jobs.merge(job, List.of(versions), DeploymentStatus::union); + }); return ImmutableMap.copyOf(jobs); } @@ -237,6 +255,7 @@ public class DeploymentStatus { && testJobs.get(test).contains(versions))) testJobs.merge(firstDeclaredOrElseImplicitTest(testType), List.of(versions), DeploymentStatus::union); }); + // Add runs for declared tests in instances without production jobs, if no successes exist for given change. } return ImmutableMap.copyOf(testJobs); } @@ -312,7 +331,7 @@ 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); @@ -458,20 +477,26 @@ 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; } - /** Time of completion of its dependencies, if all parts of the given change are contained in the change for this instance. */ + /** + * Time of completion of its dependencies, if all parts of the given change are contained in the change + * for this instance, or if no more jobs should run for this instance for the given change. + */ @Override public Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { - return (change.platform().isEmpty() || change.platform().equals(instance.change().platform())) - && (change.application().isEmpty() || change.application().equals(instance.change().application())) + return ( (change.platform().isEmpty() || change.platform().equals(instance.change().platform())) + && (change.application().isEmpty() || change.application().equals(instance.change().application())) + || status.jobsToRun(Map.of(instance.name(), change)).isEmpty()) ? dependenciesCompletedAt(change, dependent) : Optional.empty(); } 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 e2fb74ec0b4..0ac9e17c24f 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 @@ -420,8 +420,10 @@ public class DeploymentTrigger { @Override public String toString() { - return jobType + " for " + instanceId + " on (" + versions.targetPlatform() + ", " + - versions.targetApplication().id() + "), ready since " + availableSince; + return jobType + " for " + instanceId + + " on (" + versions.targetPlatform() + versions.sourcePlatform().map(version -> " <-- " + version).orElse("") + + ", " + versions.targetApplication().id() + versions.sourceApplication().map(version -> " <-- " + version.id()).orElse("") + + "), ready since " + availableSince; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index bdb7b0860e5..5f0f2e4ba4e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -107,7 +107,7 @@ public class Upgrader extends ControllerMaintainer { /** Returns a list of all production application instances, except those which are pinned, which we should not manipulate here. */ private InstanceList instances() { return InstanceList.from(controller().jobController().deploymentStatuses(ApplicationList.from(controller().applications().readable()))) - .withProductionDeployment() + .withDeclaredJobs() .unpinned(); } 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 2b5e8b3e91c..6417119618b 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 @@ -1128,4 +1128,48 @@ public class DeploymentTriggerTest { .runJob(productionCdAwsUsEast1a); } + @Test + public void testsInSeparateInstance() { + String deploymentSpec = + "<deployment version='1.0'>\n" + + " <instance id='canary'>\n" + + " <upgrade policy='canary' />\n" + + " <test />\n" + + " <staging />\n" + + " </instance>\n" + + " <instance id='default'>\n" + + " <prod>\n" + + " <region active='true'>eu-west-1</region>\n" + + " <test>eu-west-1</test>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>\n"; + + ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(deploymentSpec); + var canary = tester.newDeploymentContext("t", "a", "canary").submit(applicationPackage); + var conservative = tester.newDeploymentContext("t", "a", "default"); + + canary.runJob(systemTest) + .runJob(stagingTest); + conservative.runJob(productionEuWest1) + .runJob(testEuWest1); + + canary.submit(applicationPackage) + .runJob(systemTest) + .runJob(stagingTest); + tester.outstandingChangeDeployer().run(); + conservative.runJob(productionEuWest1) + .runJob(testEuWest1); + + tester.controllerTester().upgradeSystem(new Version("7.7.7")); + tester.upgrader().maintain(); + + canary.runJob(systemTest) + .runJob(stagingTest); + tester.upgrader().maintain(); + conservative.runJob(productionEuWest1) + .runJob(testEuWest1); + + } + } |