From e072011cd0850d634aa9ca9e58c6bff9d3301b6e Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Wed, 6 Nov 2019 13:46:49 +0100 Subject: Let JobController provide lastSuccess, firstFailing and lastCompleted --- .../controller/deployment/JobController.java | 46 ++++++++++++++- .../hosted/controller/persistence/CuratorDb.java | 3 +- .../controller/persistence/RunSerializer.java | 5 +- .../controller/maintenance/JobRunnerTest.java | 69 +++++++++++++++++++--- 4 files changed, 108 insertions(+), 15 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 3deca255bad..41266e761a9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -38,9 +38,11 @@ import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.NavigableMap; import java.util.Optional; import java.util.Set; import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -215,10 +217,15 @@ public class JobController { } /** Returns an immutable map of all known runs for the given application and job type. */ - public Map runs(ApplicationId id, JobType type) { - SortedMap runs = curator.readHistoricRuns(id, type); + public NavigableMap runs(JobId id) { + return runs(id.application(), id.type()); + } + + /** Returns an immutable map of all known runs for the given application and job type. */ + public NavigableMap runs(ApplicationId id, JobType type) { + NavigableMap runs = curator.readHistoricRuns(id, type); last(id, type).ifPresent(run -> runs.put(run.id(), run)); - return ImmutableMap.copyOf(runs); + return Collections.unmodifiableNavigableMap(runs); } /** Returns the run with the given id, if it exists. */ @@ -238,6 +245,31 @@ public class JobController { return curator.readLastRun(id, type); } + /** Returns the last completed of the given job. */ + public Optional lastCompleted(JobId id) { + return Optional.ofNullable(curator.readHistoricRuns(id.application(), id.type()) + .lastEntry().getValue()); + } + + /** Returns the first failing of the given job. */ + public Optional firstFailing(JobId id) { + Run failed = null; + loop: for (Run run : runs(id).descendingMap().values()) + switch (run.status()) { + case running: continue loop; + case success: break loop; + default: failed = run; + } + return Optional.ofNullable(failed); + } + + /** Returns the last success of the given job. */ + public Optional lastSuccess(JobId id) { + return runs(id).descendingMap().values().stream() + .filter(run -> run.status() == RunStatus.success) + .findFirst(); + } + /** Returns the run with the given id, provided it is still active. */ public Optional active(RunId id) { return last(id.application(), id.type()) @@ -274,11 +306,19 @@ public class JobController { locked(id.application(), id.type(), runs -> { runs.put(run.id(), finishedRun); long last = id.number(); + long successes = runs.values().stream().filter(old -> old.status() == RunStatus.success).count(); var oldEntries = runs.entrySet().iterator(); for (var old = oldEntries.next(); old.getKey().number() <= last - historyLength || old.getValue().start().isBefore(controller.clock().instant().minus(maxHistoryAge)); old = oldEntries.next()) { + + // Make sure we keep the last success and the first failing + if (successes == 1 && old.getValue().status() == RunStatus.success) { + oldEntries.next(); + continue; + } + logs.delete(old.getKey()); oldEntries.remove(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index dbd52fc6d02..037fb4da987 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -39,6 +39,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.NavigableMap; import java.util.Optional; import java.util.Set; import java.util.SortedMap; @@ -381,7 +382,7 @@ public class CuratorDb { return readSlime(lastRunPath(id, type)).map(runSerializer::runFromSlime); } - public SortedMap readHistoricRuns(ApplicationId id, JobType type) { + public NavigableMap readHistoricRuns(ApplicationId id, JobType type) { return readSlime(runsPath(id, type)).map(runSerializer::runsFromSlime).orElse(new TreeMap<>(comparing(RunId::number))); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java index dda1fb881a7..b84df02e583 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java @@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.deployment.Versions; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.EnumMap; +import java.util.NavigableMap; import java.util.Optional; import java.util.SortedMap; import java.util.TreeMap; @@ -90,8 +91,8 @@ class RunSerializer { return runFromSlime(slime.get()); } - SortedMap runsFromSlime(Slime slime) { - SortedMap runs = new TreeMap<>(comparing(RunId::number)); + NavigableMap runsFromSlime(Slime slime) { + NavigableMap runs = new TreeMap<>(comparing(RunId::number)); Inspector runArray = slime.get(); runArray.traverse((ArrayTraverser) (__, runObject) -> { Run run = runFromSlime(runObject); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java index 0f6aec804e2..d3752fcaff4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; @@ -242,24 +243,74 @@ public class JobRunnerTest { inThreadExecutor(), (id, step) -> Optional.of(running)); TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id(); - ApplicationId id = appId.defaultInstance(); + ApplicationId instanceId = appId.defaultInstance(); + JobId jobId = new JobId(instanceId, systemTest); jobs.submit(appId, versions.targetApplication().source().get(), "a@b", 2, applicationPackage, new byte[0]); + assertFalse(jobs.lastSuccess(jobId).isPresent()); for (int i = 0; i < jobs.historyLength(); i++) { - jobs.start(id, systemTest, versions); + jobs.start(instanceId, systemTest, versions); runner.run(); } - assertEquals(256, jobs.runs(id, systemTest).size()); - assertTrue(jobs.details(new RunId(id, systemTest, 1)).isPresent()); + assertEquals(256, jobs.runs(jobId).size()); + assertTrue(jobs.details(new RunId(instanceId, systemTest, 1)).isPresent()); - jobs.start(id, systemTest, versions); + jobs.start(instanceId, systemTest, versions); runner.run(); - assertEquals(256, jobs.runs(id, systemTest).size()); - assertEquals(2, jobs.runs(id, systemTest).keySet().iterator().next().number()); - assertFalse(jobs.details(new RunId(id, systemTest, 1)).isPresent()); - assertTrue(jobs.details(new RunId(id, systemTest, 257)).isPresent()); + assertEquals(256, jobs.runs(jobId).size()); + assertEquals(2, jobs.runs(jobId).keySet().iterator().next().number()); + assertFalse(jobs.details(new RunId(instanceId, systemTest, 1)).isPresent()); + assertTrue(jobs.details(new RunId(instanceId, systemTest, 257)).isPresent()); + + JobRunner failureRunner = new JobRunner(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()), + inThreadExecutor(), (id, step) -> Optional.of(error)); + + // Make all but the oldest of the 256 jobs a failure. + for (int i = 0; i < jobs.historyLength() - 1; i++) { + jobs.start(instanceId, systemTest, versions); + failureRunner.run(); + } + assertEquals(256, jobs.runs(jobId).size()); + assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number()); + assertEquals(257, jobs.lastSuccess(jobId).get().id().number()); + assertEquals(258, jobs.firstFailing(jobId).get().id().number()); + + // Oldest success is kept even though it would normally overflow. + jobs.start(instanceId, systemTest, versions); + failureRunner.run(); + assertEquals(257, jobs.runs(jobId).size()); + assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number()); + assertEquals(257, jobs.lastSuccess(jobId).get().id().number()); + assertEquals(258, jobs.firstFailing(jobId).get().id().number()); + + // First failure after the last success is also kept. + jobs.start(instanceId, systemTest, versions); + failureRunner.run(); + assertEquals(258, jobs.runs(jobId).size()); + assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number()); + assertEquals(258, jobs.runs(jobId).keySet().stream().skip(1).iterator().next().number()); + assertEquals(257, jobs.lastSuccess(jobId).get().id().number()); + assertEquals(258, jobs.firstFailing(jobId).get().id().number()); + + // No other jobs are kept with repeated failures. + jobs.start(instanceId, systemTest, versions); + failureRunner.run(); + assertEquals(258, jobs.runs(jobId).size()); + assertEquals(257, jobs.runs(jobId).keySet().iterator().next().number()); + assertEquals(258, jobs.runs(jobId).keySet().stream().skip(1).iterator().next().number()); + assertEquals(260, jobs.runs(jobId).keySet().stream().skip(2).iterator().next().number()); + assertEquals(257, jobs.lastSuccess(jobId).get().id().number()); + assertEquals(258, jobs.firstFailing(jobId).get().id().number()); + + // history length returns to 256 when a new success is recorded. + jobs.start(instanceId, systemTest, versions); + runner.run(); + assertEquals(256, jobs.runs(jobId).size()); + assertEquals(261, jobs.runs(jobId).keySet().iterator().next().number()); + assertEquals(516, jobs.lastSuccess(jobId).get().id().number()); + assertFalse(jobs.firstFailing(jobId).isPresent()); } @Test -- cgit v1.2.3 From 6337fb0bcac006f321cae8689680eadc5977c4a4 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 7 Nov 2019 12:30:20 +0100 Subject: Delete job data for instances immediately when they are deleted --- .../java/com/yahoo/vespa/hosted/controller/ApplicationController.java | 1 + 1 file changed, 1 insertion(+) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 71cfc679ca7..881e793b0b3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -800,6 +800,7 @@ public class ApplicationController { }); }); curator.writeApplication(application.without(instanceId.instance()).get()); + controller.jobController().collectGarbage(); log.info("Deleted " + instanceId); }); -- cgit v1.2.3 From 89d5cc75be78bbffb8c1f8ae49a2f2f57f481ccd Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 7 Nov 2019 12:31:44 +0100 Subject: Report job completion after job has properly ended, from job runner --- .../hosted/controller/deployment/InternalStepRunner.java | 8 -------- .../vespa/hosted/controller/maintenance/JobRunner.java | 16 +++++++++++++++- 2 files changed, 15 insertions(+), 9 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 33365d20926..8cb5b08bcdb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -559,14 +559,6 @@ public class InternalStepRunner implements StepRunner { private Optional report(RunId id, DualLogger logger) { try { controller.jobController().active(id).ifPresent(run -> { - JobReport report = JobReport.ofJob(run.id().application(), - run.id().type(), - run.id().number(), - ! run.hasFailed() ? Optional.empty() - : Optional.of(run.status() == outOfCapacity ? DeploymentJobs.JobError.outOfCapacity - : DeploymentJobs.JobError.unknown)); - controller.applications().deploymentTrigger().notifyOfCompletion(report); - if (run.hasFailed()) sendNotification(run, logger); }); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java index fc311e2a2af..ffe90b8c44d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.google.inject.Inject; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.deployment.InternalStepRunner; import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; @@ -13,6 +14,7 @@ import com.yahoo.vespa.hosted.controller.deployment.StepRunner; import org.jetbrains.annotations.TestOnly; import java.time.Duration; +import java.util.Optional; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -21,6 +23,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; +import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.outOfCapacity; + /** * Advances the set of {@link Run}s for a {@link JobController}. * @@ -82,7 +86,17 @@ public class JobRunner extends Maintainer { private void finish(RunId id) { try { jobs.finish(id); - } + controller().jobController().run(id).ifPresent(run -> { + DeploymentJobs.JobReport report = DeploymentJobs.JobReport.ofJob(run.id().application(), + run.id().type(), + run.id().number(), + ! run.hasFailed() ? Optional.empty() + : Optional.of(run.status() == outOfCapacity ? DeploymentJobs.JobError.outOfCapacity + : DeploymentJobs.JobError.unknown)); + controller().applications().deploymentTrigger().notifyOfCompletion(report); + }); + + } catch (Exception e) { log.log(LogLevel.WARNING, "Exception finishing " + id, e); } -- cgit v1.2.3 From dceb2dbc3eed426a4a041b0e6bdef41ed9389620 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 7 Nov 2019 14:30:45 +0100 Subject: Add JobStatus aggregate of run list, and use it in DeploymentTrigger, JobCAHH --- .../controller/deployment/DeploymentTrigger.java | 201 +++++++++------------ .../controller/deployment/JobController.java | 33 ++-- .../hosted/controller/deployment/JobStatus.java | 107 +++++++++++ .../vespa/hosted/controller/deployment/Run.java | 1 - .../hosted/controller/deployment/RunStatus.java | 4 +- .../application/JobControllerApiHandlerHelper.java | 51 +++--- 6 files changed, 247 insertions(+), 150 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java (limited to 'controller-server') 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 ca2db96c14d..45bf8a43f07 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 @@ -13,14 +13,13 @@ import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.BuildService; -import com.yahoo.vespa.hosted.controller.api.integration.BuildService.JobState; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +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.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; -import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.JobStatus.JobRun; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; @@ -30,7 +29,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.EnumSet; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; @@ -44,9 +43,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob; -import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.JobState.idle; -import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.JobState.queued; -import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.JobState.running; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; import static java.util.Collections.emptyList; @@ -139,18 +135,17 @@ public class DeploymentTrigger { } applications().lockApplicationOrThrow(TenantAndApplicationId.from(report.applicationId()), application -> { - JobRun triggering; - Optional status = application.get().require(report.applicationId().instance()) - .deploymentJobs().statusOf(report.jobType()); - triggering = status.filter(job -> job.lastTriggered().isPresent() - && job.lastCompleted() - .map(completion -> ! completion.at().isAfter(job.lastTriggered().get().at())) - .orElse(true)) - .orElseThrow(() -> new IllegalStateException("Notified of completion of " + report.jobType().jobName() + " for " + - report.applicationId() + ", but that has not been triggered; last was " + - status.flatMap(job -> job.lastTriggered().map(run -> run.at().toString())) - .orElse("never"))) - .lastTriggered().get(); + var status = application.get().require(report.applicationId().instance()) + .deploymentJobs().statusOf(report.jobType()); + var triggering = status.filter(job -> job.lastTriggered().isPresent() + && job.lastCompleted() + .map(completion -> ! completion.at().isAfter(job.lastTriggered().get().at())) + .orElse(true)) + .orElseThrow(() -> new IllegalStateException("Notified of completion of " + report.jobType().jobName() + " for " + + report.applicationId() + ", but that has not been triggered; last was " + + status.flatMap(job -> job.lastTriggered().map(run -> run.at().toString())) + .orElse("never"))) + .lastTriggered().get(); application = application.with(report.applicationId().instance(), instance -> instance.withJobCompletion(report.jobType(), @@ -231,9 +226,10 @@ public class DeploymentTrigger { Versions versions = Versions.from(application.change(), application, deploymentFor(instance, jobType), controller.systemVersion()); String reason = "Job triggered manually by " + user; - return (jobType.isProduction() && ! isTested(instance, versions) - ? testJobs(application.deploymentSpec(), application.change(), instance, versions, reason, clock.instant(), __ -> true).stream() - : Stream.of(deploymentJob(instance, versions, application.change(), jobType, reason, clock.instant()))) + var jobStatus = jobs.jobStatus(applicationId, application.deploymentSpec()); + return (jobType.isProduction() && ! isTested(jobStatus, versions) + ? testJobs(application.deploymentSpec(), application.change(), instance, jobStatus, versions, reason, clock.instant(), __ -> true).stream() + : Stream.of(deploymentJob(instance, versions, application.change(), jobType, jobStatus.get(jobType), reason, clock.instant()))) .peek(this::trigger) .map(Job::jobType).collect(toList()); } @@ -289,9 +285,8 @@ public class DeploymentTrigger { return controller.applications(); } - private Optional successOn(Instance instance, JobType jobType, Versions versions) { - return instance.deploymentJobs().statusOf(jobType).flatMap(JobStatus::lastSuccess) - .filter(versions::targetsMatch); + private Optional successOn(JobStatus status, Versions versions) { + return status.lastSuccess().filter(run -> versions.targetsMatch(run.versions())); } private Optional deploymentFor(Instance instance, JobType jobType) { @@ -326,11 +321,12 @@ public class DeploymentTrigger { .flatMap(instance -> application.get(instance.name()).stream()) .collect(Collectors.toUnmodifiableList()); for (Instance instance : instances) { + var jobStatus = this.jobs.jobStatus(instance.id(), application.deploymentSpec()); Change change = application.change(); - Optional completedAt = max(instance.deploymentJobs().statusOf(systemTest) - .flatMap(job -> job.lastSuccess().map(JobRun::at)), - instance.deploymentJobs().statusOf(stagingTest) - .flatMap(job -> job.lastSuccess().map(JobRun::at))); + Optional completedAt = max(Optional.ofNullable(jobStatus.get(systemTest)) + .flatMap(job -> job.lastSuccess().map(run -> run.end().get())), + Optional.ofNullable(jobStatus.get(stagingTest)) + .flatMap(job -> job.lastSuccess().map(run -> run.end().get()))); String reason = "New change available"; List testJobs = null; // null means "uninitialised", while empty means "don't run any jobs". DeploymentSteps steps = steps(application.deploymentSpec().requireInstance(instance.name())); @@ -338,22 +334,22 @@ public class DeploymentTrigger { if (change.hasTargets()) { for (Step step : steps.production()) { List stepJobs = steps.toJobs(step); - List remainingJobs = stepJobs.stream().filter(job -> ! isComplete(change, change, instance, job)).collect(toList()); + List remainingJobs = stepJobs.stream().filter(job -> ! isComplete(change, change, instance, job, jobStatus.get(job))).collect(toList()); if ( ! remainingJobs.isEmpty()) { // Change is incomplete; trigger remaining jobs if ready, or their test jobs if untested. for (JobType job : remainingJobs) { Versions versions = Versions.from(change, application, deploymentFor(instance, job), controller.systemVersion()); - if (isTested(instance, versions)) { - if (completedAt.isPresent() && canTrigger(job, versions, instance, application.deploymentSpec(), stepJobs)) { - jobs.add(deploymentJob(instance, versions, change, job, reason, completedAt.get())); + if (isTested(jobStatus, versions)) { + if (completedAt.isPresent() && canTrigger(job, jobStatus, versions, instance, application.deploymentSpec(), stepJobs)) { + jobs.add(deploymentJob(instance, versions, change, job, jobStatus.get(job), reason, completedAt.get())); } - if ( ! alreadyTriggered(instance, versions) && testJobs == null) { + if ( ! alreadyTriggered(jobStatus, versions) && testJobs == null) { testJobs = emptyList(); } } else if (testJobs == null) { testJobs = testJobs(application.deploymentSpec(), - change, instance, versions, + change, instance, jobStatus, versions, String.format("Testing deployment for %s (%s)", job.jobName(), versions.toString()), completedAt.orElseGet(clock::instant)); @@ -367,14 +363,14 @@ public class DeploymentTrigger { reason += " after a delay of " + step.delay(); } else { - completedAt = stepJobs.stream().map(job -> instance.deploymentJobs().statusOf(job).get().lastCompleted().get().at()).max(naturalOrder()); + completedAt = stepJobs.stream().map(job -> jobStatus.get(job).lastCompleted().get().end().get()).max(naturalOrder()); reason = "Available change in " + stepJobs.stream().map(JobType::jobName).collect(joining(", ")); } } } } if (testJobs == null) { // If nothing to test, but outstanding commits, test those. - testJobs = testJobs(application.deploymentSpec(), change, instance, + testJobs = testJobs(application.deploymentSpec(), change, instance, jobStatus, Versions.from(application.outstandingChange().onTopOf(change), application, steps.sortedDeployments(instance.productionDeployments().values()).stream().findFirst(), @@ -388,21 +384,21 @@ public class DeploymentTrigger { } /** Returns whether given job should be triggered */ - private boolean canTrigger(JobType job, Versions versions, Instance instance, DeploymentSpec deploymentSpec, List parallelJobs) { - if (jobStateOf(instance, job) != idle) return false; + private boolean canTrigger(JobType job, Map status, Versions versions, Instance instance, DeploymentSpec deploymentSpec, List parallelJobs) { + if (status.get(job).isRunning()) return false; // Are we already running jobs which are not in the set which can run in parallel with this? - if (parallelJobs != null && ! parallelJobs.containsAll(runningProductionJobs(instance))) return false; + if (parallelJobs != null && ! parallelJobs.containsAll(runningProductionJobs(status))) return false; // Are there another suspended deployment such that we shouldn't simultaneously change this? if (job.isProduction() && isSuspendedInAnotherZone(instance, job.zone(controller.system()))) return false; - return triggerAt(clock.instant(), job, versions, instance, deploymentSpec); + return triggerAt(clock.instant(), job, status.get(job), versions, instance, deploymentSpec); } /** Returns whether given job should be triggered */ - private boolean canTrigger(JobType job, Versions versions, Instance instance, DeploymentSpec deploymentSpec) { - return canTrigger(job, versions, instance, deploymentSpec, null); + private boolean canTrigger(JobType job, Map status, Versions versions, Instance instance, DeploymentSpec deploymentSpec) { + return canTrigger(job, status, versions, instance, deploymentSpec, null); } private boolean isSuspendedInAnotherZone(Instance instance, ZoneId zone) { @@ -415,24 +411,23 @@ public class DeploymentTrigger { } /** Returns whether the given job can trigger at the given instant */ - public boolean triggerAt(Instant instant, JobType job, Versions versions, Instance instance, DeploymentSpec deploymentSpec) { - Optional jobStatus = instance.deploymentJobs().statusOf(job); - if (jobStatus.isEmpty()) return true; - if (jobStatus.get().pausedUntil().isPresent() && jobStatus.get().pausedUntil().getAsLong() > clock.instant().toEpochMilli()) return false; - if (jobStatus.get().isSuccess()) return true; // Success - if (jobStatus.get().lastCompleted().isEmpty()) return true; // Never completed - if (jobStatus.get().firstFailing().isEmpty()) return true; // Should not happen as firstFailing should be set for an unsuccessful job - if ( ! versions.targetsMatch(jobStatus.get().lastCompleted().get())) return true; // Always trigger as targets have changed + public boolean triggerAt(Instant instant, JobType job, JobStatus jobStatus, Versions versions, Instance instance, DeploymentSpec deploymentSpec) { + if (instance.deploymentJobs().statusOf(job).map(status -> status.pausedUntil().orElse(0)).orElse(0L) > clock.millis()) return false; + if (jobStatus.lastTriggered().isEmpty()) return true; + if (jobStatus.isSuccess()) return true; // Success + if (jobStatus.lastCompleted().isEmpty()) return true; // Never completed + if (jobStatus.firstFailing().isEmpty()) return true; // Should not happen as firstFailing should be set for an unsuccessful job + if ( ! versions.targetsMatch(jobStatus.lastCompleted().get().versions())) return true; // Always trigger as targets have changed if (deploymentSpec.requireInstance(instance.name()).upgradePolicy() == DeploymentSpec.UpgradePolicy.canary) return true; // Don't throttle canaries - Instant firstFailing = jobStatus.get().firstFailing().get().at(); - Instant lastCompleted = jobStatus.get().lastCompleted().get().at(); + Instant firstFailing = jobStatus.firstFailing().get().end().get(); + Instant lastCompleted = jobStatus.lastCompleted().get().end().get(); // Retry all errors immediately for 1 minute if (firstFailing.isAfter(instant.minus(Duration.ofMinutes(1)))) return true; // Retry out of capacity errors in test environments every minute - if (job.isTest() && jobStatus.get().isOutOfCapacity()) { + if (job.isTest() && jobStatus.isOutOfCapacity()) { return lastCompleted.isBefore(instant.minus(Duration.ofMinutes(1))); } @@ -445,27 +440,12 @@ public class DeploymentTrigger { // ---------- Job state helpers ---------- - private List runningProductionJobs(Instance instance) { - return instance.deploymentJobs().jobStatus().keySet().parallelStream() - .filter(JobType::isProduction) - .filter(job -> isRunning(instance, job)) - .collect(toList()); - } - - /** Returns whether the given job is currently running; false if completed since last triggered, asking the build service otherwise. */ - private boolean isRunning(Instance instance, JobType jobType) { - return ! instance.deploymentJobs().statusOf(jobType) - .flatMap(job -> job.lastCompleted().map(run -> run.at().isAfter(job.lastTriggered().get().at()))) - .orElse(false) - && EnumSet.of(running, queued).contains(jobStateOf(instance, jobType)); - } - - private JobState jobStateOf(Instance instance, JobType jobType) { - if (controller.applications().requireApplication(TenantAndApplicationId.from(instance.id())).internal()) { - Optional run = controller.jobController().last(instance.id(), jobType); - return run.isPresent() && ! run.get().hasEnded() ? JobState.running : JobState.idle; - } - return buildService.stateOf(BuildJob.of(instance.id(), 0, jobType.jobName())); + private List runningProductionJobs(Map status) { + return status.values().parallelStream() + .filter(job -> job.isRunning()) + .map(job -> job.job().type()) + .filter(JobType::isProduction) + .collect(toList()); } // ---------- Completion logic ---------- @@ -482,17 +462,18 @@ public class DeploymentTrigger { * Additionally, if the application is pinned to a Vespa version, and the given change has a (this) platform, * the deployment for the job must be on the pinned version. */ - public boolean isComplete(Change change, Change fullChange, Instance instance, JobType jobType) { + public boolean isComplete(Change change, Change fullChange, Instance instance, JobType jobType, + JobStatus status) { Optional existingDeployment = deploymentFor(instance, jobType); if ( change.isPinned() && change.platform().isPresent() && ! existingDeployment.map(Deployment::version).equals(change.platform())) return false; - return instance.deploymentJobs().statusOf(jobType).flatMap(JobStatus::lastSuccess) - .map(job -> change.platform().map(job.platform()::equals).orElse(true) - && change.application().map(job.application()::equals).orElse(true)) - .orElse(false) + return status.lastSuccess() + .map(run -> change.platform().map(run.versions().targetPlatform()::equals).orElse(true) + && change.application().map(run.versions().targetApplication()::equals).orElse(true)) + .orElse(false) || jobType.isProduction() && existingDeployment.map(deployment -> ! isUpgrade(change, deployment) && isDowngrade(fullChange, deployment)) .orElse(false); @@ -506,27 +487,28 @@ public class DeploymentTrigger { return change.downgrades(deployment.version()) || change.downgrades(deployment.applicationVersion()); } - private boolean isTested(Instance instance, Versions versions) { - return testedIn(instance, systemTest, versions) - && testedIn(instance, stagingTest, versions) - || alreadyTriggered(instance, versions); + private boolean isTested(Map status, Versions versions) { + return testedIn(systemTest, status.get(systemTest), versions) + && testedIn(stagingTest, status.get(stagingTest), versions) + || alreadyTriggered(status, versions); } - public boolean testedIn(Instance instance, JobType testType, Versions versions) { + public boolean testedIn(JobType testType, JobStatus status, Versions versions) { if (testType == systemTest) - return successOn(instance, systemTest, versions).isPresent(); + return successOn(status, versions).isPresent(); if (testType == stagingTest) - return successOn(instance, stagingTest, versions).filter(versions::sourcesMatchIfPresent).isPresent(); + return successOn(status, versions).map(Run::versions).filter(versions::sourcesMatchIfPresent).isPresent(); throw new IllegalArgumentException(testType + " is not a test job!"); } - public boolean alreadyTriggered(Instance instance, Versions versions) { - return instance.deploymentJobs().jobStatus().values().stream() - .filter(job -> job.type().isProduction()) + public boolean alreadyTriggered(Map status, Versions versions) { + return status.values().stream() + .filter(job -> job.job().type().isProduction()) .anyMatch(job -> job.lastTriggered() - .filter(versions::targetsMatch) - .filter(versions::sourcesMatchIfPresent) - .isPresent()); + .map(Run::versions) + .filter(versions::targetsMatch) + .filter(versions::sourcesMatchIfPresent) + .isPresent()); } // ---------- Change management o_O ---------- @@ -542,12 +524,11 @@ public class DeploymentTrigger { private Change remainingChange(Application application) { Change change = application.change(); - if (application.deploymentSpec().instances().stream() .allMatch(spec -> { DeploymentSteps steps = new DeploymentSteps(spec, controller::system); return (steps.productionJobs().isEmpty() ? steps.testJobs() : steps.productionJobs()) - .stream().allMatch(job -> isComplete(application.change().withoutApplication(), application.change(), application.require(spec.name()), job)); + .stream().allMatch(job -> isComplete(application.change().withoutApplication(), application.change(), application.require(spec.name()), job, jobs.jobStatus(new JobId(application.id().instance(spec.name()), job)))); })) change = change.withoutPlatform(); @@ -555,7 +536,7 @@ public class DeploymentTrigger { .allMatch(spec -> { DeploymentSteps steps = new DeploymentSteps(spec, controller::system); return (steps.productionJobs().isEmpty() ? steps.testJobs() : steps.productionJobs()) - .stream().allMatch(job -> isComplete(application.change().withoutPlatform(), application.change(), application.require(spec.name()), job)); + .stream().allMatch(job -> isComplete(application.change().withoutPlatform(), application.change(), application.require(spec.name()), job, jobs.jobStatus(new JobId(application.id().instance(spec.name()), job)))); })) change = change.withoutApplication(); @@ -567,42 +548,40 @@ public class DeploymentTrigger { /** * Returns the list of test jobs that should run now, and that need to succeed on the given versions for it to be considered tested. */ - private List testJobs(DeploymentSpec deploymentSpec, Change change, Instance instance, Versions versions, + private List testJobs(DeploymentSpec deploymentSpec, Change change, Instance instance, Map status, Versions versions, String reason, Instant availableSince) { - return testJobs(deploymentSpec, change, instance, versions, reason, availableSince, - jobType -> canTrigger(jobType, versions, instance, deploymentSpec)); + return testJobs(deploymentSpec, change, instance, status, versions, reason, availableSince, + jobType -> canTrigger(jobType, status, versions, instance, deploymentSpec)); } /** * Returns the list of test jobs that need to succeed on the given versions for it to be considered tested, filtered by the given condition. */ - private List testJobs(DeploymentSpec deploymentSpec, Change change, Instance instance, Versions versions, + private List testJobs(DeploymentSpec deploymentSpec, Change change, Instance instance, Map status, Versions versions, String reason, Instant availableSince, Predicate condition) { List jobs = new ArrayList<>(); for (JobType jobType : new DeploymentSteps(deploymentSpec.requireInstance(instance.name()), controller::system).testJobs()) { // TODO jonmv: Allow cross-instance validation - Optional completion = successOn(instance, jobType, versions) - .filter(run -> versions.sourcesMatchIfPresent(run) || jobType == systemTest); + Optional completion = successOn(status.get(jobType), versions) + .filter(run -> versions.sourcesMatchIfPresent(run.versions()) || jobType == systemTest); if (completion.isEmpty() && condition.test(jobType)) - jobs.add(deploymentJob(instance, versions, change, jobType, reason, availableSince)); + jobs.add(deploymentJob(instance, versions, change, jobType, status.get(jobType), reason, availableSince)); } return jobs; } - private Job deploymentJob(Instance instance, Versions versions, Change change, JobType jobType, String reason, Instant availableSince) { - boolean isRetry = instance.deploymentJobs().statusOf(jobType) - .map(JobStatus::isOutOfCapacity) - .orElse(false); - if (isRetry) reason += "; retrying on out of capacity"; + private Job deploymentJob(Instance instance, Versions versions, Change change, JobType jobType, JobStatus jobStatus, String reason, Instant availableSince) { + if (jobStatus.isOutOfCapacity()) reason += "; retrying on out of capacity"; - JobRun triggering = JobRun.triggering(versions.targetPlatform(), versions.targetApplication(), - versions.sourcePlatform(), versions.sourceApplication(), - reason, clock.instant()); - return new Job(instance, triggering, jobType, availableSince, isRetry, change.application().isPresent()); + var triggering = JobRun.triggering(versions.targetPlatform(), versions.targetApplication(), + versions.sourcePlatform(), versions.sourceApplication(), + reason, clock.instant()); + return new Job(instance, triggering, jobType, availableSince, jobStatus.isOutOfCapacity(), change.application().isPresent()); } // ---------- Data containers ---------- + // TODO jonmv: Replace with a JobSpec class not based on BuildJob. private static class Job extends BuildJob { private final JobType jobType; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 41266e761a9..84f14109c4b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.google.common.collect.ImmutableMap; import com.yahoo.component.Version; +import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.zone.ZoneId; @@ -33,8 +34,10 @@ import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; 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.List; import java.util.Map; @@ -247,27 +250,17 @@ public class JobController { /** Returns the last completed of the given job. */ public Optional lastCompleted(JobId id) { - return Optional.ofNullable(curator.readHistoricRuns(id.application(), id.type()) - .lastEntry().getValue()); + return JobStatus.lastCompleted(runs(id)); } /** Returns the first failing of the given job. */ public Optional firstFailing(JobId id) { - Run failed = null; - loop: for (Run run : runs(id).descendingMap().values()) - switch (run.status()) { - case running: continue loop; - case success: break loop; - default: failed = run; - } - return Optional.ofNullable(failed); + return JobStatus.firstFailing(runs(id)); } /** Returns the last success of the given job. */ public Optional lastSuccess(JobId id) { - return runs(id).descendingMap().values().stream() - .filter(run -> run.status() == RunStatus.success) - .findFirst(); + return JobStatus.lastSuccess(runs(id)); } /** Returns the run with the given id, provided it is still active. */ @@ -294,6 +287,20 @@ public class JobController { .iterator()); } + /** Returns the job status of the given job, possibly empty. */ + public JobStatus jobStatus(JobId id) { + return new JobStatus(id, runs(id)); + } + + /** Returns the job status of all declared jobs for the given instance id, indexed by job type. */ + public Map jobStatus(ApplicationId id, DeploymentSpec spec) { + return new DeploymentSteps(spec.requireInstance(id.instance()), controller::system) + .jobs().stream() + .map(type -> jobStatus(new JobId(id, type))) + .collect(Collectors.toUnmodifiableMap(status -> status.job().type(), + status -> status)); + } + /** Changes the status of the given step, for the given run, provided it is still active. */ public void update(RunId id, RunStatus status, LockedStep step) { locked(id, run -> run.with(status, step)); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java new file mode 100644 index 00000000000..a338a766727 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java @@ -0,0 +1,107 @@ +package com.yahoo.vespa.hosted.controller.deployment; + +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; + +import java.util.NavigableMap; +import java.util.Objects; +import java.util.Optional; + +/** + * Aggregates information about all known runs of a given job to provide the high level status. + * + * @author jonmv + */ +public class JobStatus { + + private final JobId id; + private final NavigableMap runs; + private final Optional lastTriggered; + private final Optional lastCompleted; + private final Optional lastSuccess; + private final Optional firstFailing; + + public JobStatus(JobId id, NavigableMap runs) { + this.id = Objects.requireNonNull(id); + this.runs = Objects.requireNonNull(runs); + this.lastTriggered = runs.descendingMap().values().stream().findFirst(); + this.lastCompleted = lastCompleted(runs); + this.lastSuccess = lastSuccess(runs); + this.firstFailing = firstFailing(runs); + } + + public JobId job() { + return id; + } + + public NavigableMap runs() { + return runs; + } + + public Optional lastTriggered() { + return lastTriggered; + } + + public Optional lastCompleted() { + return lastCompleted; + } + + public Optional lastSuccess() { + return lastSuccess; + } + + public Optional firstFailing() { + return firstFailing; + } + + public Optional lastStatus() { + return lastCompleted().map(Run::status); + } + + public boolean isSuccess() { + return lastStatus().isPresent() && lastStatus().get() == RunStatus.success; + } + + public boolean isRunning() { + return lastTriggered.isPresent() && ! lastTriggered.get().hasEnded(); + } + + public boolean isOutOfCapacity() { + return lastStatus().isPresent() && lastStatus().get() == RunStatus.outOfCapacity; + } + + @Override + public String toString() { + return "JobStatus{" + + "id=" + id + + ", lastTriggered=" + lastTriggered + + ", lastCompleted=" + lastCompleted + + ", lastSuccess=" + lastSuccess + + ", firstFailing=" + firstFailing + + '}'; + } + + static Optional lastCompleted(NavigableMap runs) { + return runs.descendingMap().values().stream() + .filter(run -> run.hasEnded()) + .findFirst(); + } + + static Optional lastSuccess(NavigableMap runs) { + return runs.descendingMap().values().stream() + .filter(run -> run.status() == RunStatus.success) + .findFirst(); + } + + static Optional firstFailing(NavigableMap runs) { + Run failed = null; + loop: for (Run run : runs.descendingMap().values()) + switch (run.status()) { + case running: continue loop; + case success: break loop; + default: failed = run; + } + return Optional.ofNullable(failed); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java index d7d6134ccb9..2f9c5ea9e08 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java @@ -179,7 +179,6 @@ public class Run { ", start=" + start + ", end=" + end + ", status=" + status + - ", steps=" + steps + '}'; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java index ea6cc983b71..4d0b7ef3b90 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java @@ -11,7 +11,7 @@ public enum RunStatus { /** Run is still proceeding normally, i.e., without failures. */ running, - /** Deployment was rejected due to missing capacity. */ + /** Deployment was rejected due to lack of capacity. */ outOfCapacity, /** Deployment of the real application was rejected. */ @@ -29,7 +29,7 @@ public enum RunStatus { /** Everything completed with great success! */ success, - /** Run has been abandoned, due to user intervention or timeout. */ + /** Run was abandoned, due to user intervention or job timeout. */ aborted } 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 9fb20ef4f81..cb4c1e71c43 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 @@ -16,16 +16,17 @@ import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.NotExistsException; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +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.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; -import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; import com.yahoo.vespa.hosted.controller.deployment.JobController; +import com.yahoo.vespa.hosted.controller.deployment.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.RunLog; import com.yahoo.vespa.hosted.controller.deployment.RunStatus; @@ -39,6 +40,7 @@ import java.net.URI; import java.util.ArrayDeque; import java.util.Comparator; import java.util.Deque; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -77,11 +79,12 @@ class JobControllerApiHandlerHelper { Instance instance = application.require(id.instance()); Change change = application.change(); DeploymentSteps steps = new DeploymentSteps(application.deploymentSpec().requireInstance(id.instance()), controller::system); + Map status = controller.jobController().jobStatus(id, application.deploymentSpec()); // The logic for pending runs imitates DeploymentTrigger logic; not good, but the trigger wiring must be re-written to reuse :S Map pendingProduction = steps.productionJobs().stream() - .filter(type -> ! controller.applications().deploymentTrigger().isComplete(change, change, instance, type)) + .filter(type -> ! controller.applications().deploymentTrigger().isComplete(change, change, instance, type, status.get(type))) .collect(Collectors.toMap(type -> type, type -> Versions.from(change, application, @@ -102,8 +105,8 @@ class JobControllerApiHandlerHelper { Cursor lastVersionsObject = responseObject.setObject("lastVersions"); if (application.latestVersion().isPresent()) { - lastPlatformToSlime(lastVersionsObject.setObject("platform"), controller, application, instance, change, steps); - lastApplicationToSlime(lastVersionsObject.setObject("application"), application, instance, change, steps, controller); + lastPlatformToSlime(lastVersionsObject.setObject("platform"), controller, application, instance, status, change, steps); + lastApplicationToSlime(lastVersionsObject.setObject("application"), application, instance, status, change, steps, controller); } Cursor deployingObject = responseObject.setObject("deploying"); @@ -125,6 +128,7 @@ class JobControllerApiHandlerHelper { pendingProduction, running, type, + Optional.ofNullable(status.get(type)), deployment); }); }); @@ -135,6 +139,7 @@ class JobControllerApiHandlerHelper { controller, application, instance, + status, type, steps, pendingProduction, @@ -158,7 +163,7 @@ class JobControllerApiHandlerHelper { return new SlimeJsonResponse(slime); } - private static void lastPlatformToSlime(Cursor lastPlatformObject, Controller controller, Application application, Instance instance, Change change, DeploymentSteps steps) { + private static void lastPlatformToSlime(Cursor lastPlatformObject, Controller controller, Application application, Instance instance, Map status, Change change, DeploymentSteps steps) { VespaVersion lastVespa = controller.versionStatus().version(controller.systemVersion()); VespaVersion.Confidence targetConfidence = Map.of(defaultPolicy, normal, conservative, high) @@ -171,7 +176,7 @@ class JobControllerApiHandlerHelper { Version lastPlatform = lastVespa.versionNumber(); lastPlatformObject.setString("platform", lastPlatform.toString()); lastPlatformObject.setLong("at", lastVespa.committedAt().toEpochMilli()); - long completed = steps.productionJobs().stream().filter(type -> controller.applications().deploymentTrigger().isComplete(Change.of(lastPlatform), change, instance, type)).count(); + long completed = steps.productionJobs().stream().filter(type -> controller.applications().deploymentTrigger().isComplete(Change.of(lastPlatform), change, instance, type, status.get(type))).count(); if (Optional.of(lastPlatform).equals(change.platform())) lastPlatformObject.setString("deploying", completed + " of " + steps.productionJobs().size() + " complete"); else if (completed == steps.productionJobs().size()) @@ -191,12 +196,12 @@ class JobControllerApiHandlerHelper { : "Waiting for " + application.change() + " to complete"); } - private static void lastApplicationToSlime(Cursor lastApplicationObject, Application application, Instance instance, Change change, DeploymentSteps steps, Controller controller) { + private static void lastApplicationToSlime(Cursor lastApplicationObject, Application application, Instance instance, Map status, Change change, DeploymentSteps steps, Controller controller) { long completed; ApplicationVersion lastApplication = application.latestVersion().get(); applicationVersionToSlime(lastApplicationObject.setObject("application"), lastApplication); lastApplicationObject.setLong("at", lastApplication.buildTime().get().toEpochMilli()); - completed = steps.productionJobs().stream().filter(type -> controller.applications().deploymentTrigger().isComplete(Change.of(lastApplication), change, instance, type)).count(); + completed = steps.productionJobs().stream().filter(type -> controller.applications().deploymentTrigger().isComplete(Change.of(lastApplication), change, instance, type, status.get(type))).count(); if (Optional.of(lastApplication).equals(change.application())) lastApplicationObject.setString("deploying", completed + " of " + steps.productionJobs().size() + " complete"); else if (completed == steps.productionJobs().size()) @@ -215,14 +220,14 @@ class JobControllerApiHandlerHelper { private static void deploymentToSlime(Cursor deploymentObject, Instance instance, Change change, Map pendingProduction, Map running, - JobType type, Deployment deployment) { + JobType type, Optional jobStatus, Deployment deployment) { deploymentObject.setLong("at", deployment.at().toEpochMilli()); deploymentObject.setString("platform", deployment.version().toString()); applicationVersionToSlime(deploymentObject.setObject("application"), deployment.applicationVersion()); - deploymentObject.setBool("verified", instance.deploymentJobs().statusOf(type) - .flatMap(JobStatus::lastSuccess) - .filter(run -> run.platform().equals(deployment.version()) - && run.application().equals(deployment.applicationVersion())) + deploymentObject.setBool("verified", jobStatus.flatMap(job -> job.lastSuccess()) + .map(Run::versions) + .filter(run -> run.targetPlatform().equals(deployment.version()) + && run.targetApplication().equals(deployment.applicationVersion())) .isPresent()); if (running.containsKey(type)) deploymentObject.setString("status", running.get(type).steps().get(deployReal) == unfinished ? "deploying" : "verifying"); @@ -230,17 +235,17 @@ class JobControllerApiHandlerHelper { deploymentObject.setString("status", pendingProduction.containsKey(type) ? "pending" : "completed"); } - private static void jobTypeToSlime(Cursor jobObject, Controller controller, Application application, Instance instance, JobType type, DeploymentSteps steps, + private static void jobTypeToSlime(Cursor jobObject, Controller controller, Application application, Instance instance, Map status, JobType type, DeploymentSteps steps, Map pendingProduction, Map running, URI baseUriForJob) { - instance.deploymentJobs().statusOf(type).ifPresent(status -> status.pausedUntil().ifPresent(until -> + instance.deploymentJobs().statusOf(type).ifPresent(jobStatus -> jobStatus.pausedUntil().ifPresent(until -> jobObject.setLong("pausedUntil", until))); int runs = 0; Cursor runArray = jobObject.setArray("runs"); if (type.isTest()) { Deque> pending = new ArrayDeque<>(); pendingProduction.entrySet().stream() - .filter(typeVersions -> ! controller.applications().deploymentTrigger().testedIn(instance, type, typeVersions.getValue())) - .filter(typeVersions -> ! controller.applications().deploymentTrigger().alreadyTriggered(instance, typeVersions.getValue())) + .filter(typeVersions -> ! controller.applications().deploymentTrigger().testedIn(type, status.get(type), typeVersions.getValue())) + .filter(typeVersions -> ! controller.applications().deploymentTrigger().alreadyTriggered(status, typeVersions.getValue())) .collect(groupingBy(Map.Entry::getValue, LinkedHashMap::new, Collectors.mapping(Map.Entry::getKey, toList()))) @@ -254,7 +259,7 @@ class JobControllerApiHandlerHelper { Cursor runObject = runArray.addObject(); runObject.setString("status", "pending"); versionsToSlime(runObject, versions); - if ( ! controller.applications().deploymentTrigger().triggerAt(controller.clock().instant(), type, versions, instance, application.deploymentSpec())) + if ( ! controller.applications().deploymentTrigger().triggerAt(controller.clock().instant(), type, status.get(type), versions, instance, application.deploymentSpec())) runObject.setObject("tasks").setString("cooldown", "failed"); else runObject.setObject("tasks").setString("capacity", "running"); @@ -270,18 +275,18 @@ class JobControllerApiHandlerHelper { runObject.setString("status", "pending"); versionsToSlime(runObject, pendingProduction.get(type)); Cursor pendingObject = runObject.setObject("tasks"); - if (instance.deploymentJobs().statusOf(type).map(status -> status.pausedUntil().isPresent()).orElse(false)) + if (instance.deploymentJobs().statusOf(type).map(jobStatus -> jobStatus.pausedUntil().isPresent()).orElse(false)) pendingObject.setString("paused", "pending"); - else if ( ! controller.applications().deploymentTrigger().triggerAt(controller.clock().instant(), type, versions, instance, application.deploymentSpec())) + else if ( ! controller.applications().deploymentTrigger().triggerAt(controller.clock().instant(), type, status.get(type), versions, instance, application.deploymentSpec())) pendingObject.setString("cooldown", "failed"); else { int pending = 0; - if ( ! controller.applications().deploymentTrigger().alreadyTriggered(instance, versions)) { - if ( ! controller.applications().deploymentTrigger().testedIn(instance, systemTest, versions)) { + if ( ! controller.applications().deploymentTrigger().alreadyTriggered(status, versions)) { + if ( ! controller.applications().deploymentTrigger().testedIn(systemTest, status.get(systemTest), versions)) { pending++; pendingObject.setString(shortNameOf(systemTest, controller.system()), statusOf(controller, instance.id(), systemTest, versions)); } - if ( ! controller.applications().deploymentTrigger().testedIn(instance, stagingTest, versions)) { + if ( ! controller.applications().deploymentTrigger().testedIn(stagingTest, status.get(stagingTest), versions)) { pending++; pendingObject.setString(shortNameOf(stagingTest, controller.system()), statusOf(controller, instance.id(), stagingTest, versions)); } -- cgit v1.2.3 From fa2ae0775dea47fcbbc4c17e52fa7298d7ae16b7 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 7 Nov 2019 15:12:47 +0100 Subject: All applications are now deployed on internal pipeline --- .../integration/deployment/ArtifactRepository.java | 1 + .../yahoo/vespa/hosted/controller/Application.java | 10 +-- .../hosted/controller/ApplicationController.java | 35 +++------- .../vespa/hosted/controller/LockedApplication.java | 46 ++++++------- .../controller/deployment/DeploymentTrigger.java | 20 ++---- .../controller/deployment/JobController.java | 43 ++---------- .../persistence/ApplicationSerializer.java | 4 +- .../hosted/controller/persistence/CuratorDb.java | 9 ++- .../restapi/application/ApplicationApiHandler.java | 5 -- .../application/JobControllerApiHandlerHelper.java | 23 +++---- .../controller/deployment/DeploymentContext.java | 1 - .../controller/maintenance/JobRunnerTest.java | 2 +- .../persistence/ApplicationSerializerTest.java | 2 - .../restapi/application/ApplicationApiTest.java | 6 -- .../responses/instance-with-routing-policy.json | 1 - ...stance-without-change-multiple-deployments.json | 1 - .../restapi/application/responses/instance.json | 1 - .../application/responses/instance1-recursive.json | 1 - .../responses/jobs-direct-deployment.json | 76 +--------------------- 19 files changed, 59 insertions(+), 228 deletions(-) (limited to 'controller-server') diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java index 890059b86b2..2beb19536b6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java @@ -12,6 +12,7 @@ import com.yahoo.config.provision.zone.ZoneId; */ public interface ArtifactRepository { + // TODO unused, remove /** Returns the tenant application package of the given version. */ byte[] getApplicationPackage(ApplicationId application, String applicationVersion); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index 0a8b5ca8c3d..681c1b4283a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -45,7 +45,6 @@ public class Application { private final ValidationOverrides validationOverrides; private final Optional latestVersion; private final OptionalLong projectId; - private final boolean internal; private final Change change; private final Change outstandingChange; private final Optional deploymentIssueId; @@ -60,14 +59,14 @@ public class Application { public Application(TenantAndApplicationId id, Instant now) { this(id, now, DeploymentSpec.empty, ValidationOverrides.empty, Change.empty(), Change.empty(), Optional.empty(), Optional.empty(), Optional.empty(), OptionalInt.empty(), - new ApplicationMetrics(0, 0), Set.of(), OptionalLong.empty(), false, Optional.empty(), List.of()); + new ApplicationMetrics(0, 0), Set.of(), OptionalLong.empty(), Optional.empty(), List.of()); } // DO NOT USE! For serialization purposes, only. public Application(TenantAndApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, ValidationOverrides validationOverrides, Change change, Change outstandingChange, Optional deploymentIssueId, Optional ownershipIssueId, Optional owner, OptionalInt majorVersion, ApplicationMetrics metrics, Set deployKeys, OptionalLong projectId, - boolean internal, Optional latestVersion, Collection instances) { + Optional latestVersion, Collection instances) { this.id = Objects.requireNonNull(id, "id cannot be null"); this.createdAt = Objects.requireNonNull(createdAt, "instant of creation cannot be null"); this.deploymentSpec = Objects.requireNonNull(deploymentSpec, "deploymentSpec cannot be null"); @@ -81,7 +80,6 @@ public class Application { this.metrics = Objects.requireNonNull(metrics, "metrics cannot be null"); this.deployKeys = Objects.requireNonNull(deployKeys, "deployKeys cannot be null"); this.projectId = Objects.requireNonNull(projectId, "projectId cannot be null"); - this.internal = internal; this.latestVersion = requireNotUnknown(latestVersion); this.instances = ImmutableSortedMap.copyOf(instances.stream().collect(Collectors.toMap(Instance::name, Function.identity()))); } @@ -102,10 +100,6 @@ public class Application { /** Returns the last submitted version of this application. */ public Optional latestVersion() { return latestVersion; } - /** Returns whether this application is run on the internal deployment pipeline. */ - // TODO jonmv: Remove, as will be always true. - public boolean internal() { return internal; } - /** * Returns the last deployed validation overrides of this application, * or the empty validation overrides if it has never been deployed diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 881e793b0b3..90c3fabf2d6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -207,6 +207,11 @@ public class ApplicationController { return curator.readApplications(); } + /** Returns the ID of all known applications. */ + public List idList() { + return curator.readApplicationIds(); + } + /** Returns a snapshot of all applications of a tenant */ public List asList(TenantName tenant) { return curator.readApplications(tenant); @@ -385,7 +390,7 @@ public class ApplicationController { applicationVersion = preferOldestVersion ? triggered.sourceApplication().orElse(triggered.application()) : triggered.application(); - applicationPackage = getApplicationPackage(instanceId, application.get().internal(), applicationVersion); + applicationPackage = getApplicationPackage(instanceId, applicationVersion); applicationPackage = withTesterCertificate(applicationPackage, instanceId, jobType); validateRun(application.get(), instance, zone, platformVersion, applicationVersion); } @@ -397,13 +402,6 @@ public class ApplicationController { applicationCertificate = Optional.empty(); } - // TODO jonmv: REMOVE! This is now irrelevant for non-CD-test deployments and non-unit tests. - if ( ! preferOldestVersion - && ! application.get().internal() - && ! zone.environment().isManuallyDeployed()) { - application = storeWithUpdatedConfig(application, applicationPackage); - } - endpoints = registerEndpointsInDns(applicationPackage.deploymentSpec(), application.get().require(instanceId.instance()), zone); } // Release application lock while doing the deployment, which is a lengthy task. @@ -434,25 +432,8 @@ public class ApplicationController { } /** Fetches the requested application package from the artifact store(s). */ - public ApplicationPackage getApplicationPackage(ApplicationId id, boolean internal, ApplicationVersion version) { - try { - return internal - ? new ApplicationPackage(applicationStore.get(id.tenant(), id.application(), version)) - : new ApplicationPackage(artifactRepository.getApplicationPackage(id, version.id())); - } - catch (RuntimeException e) { // If application has switched deployment pipeline, artifacts stored prior to the switch are in the other artifact store. - try { - log.info("Fetching application package for " + id + " from alternate repository; it is now deployed " - + (internal ? "internally" : "externally") + "\nException was: " + Exceptions.toMessageString(e)); - return internal - ? new ApplicationPackage(artifactRepository.getApplicationPackage(id, version.id())) - : new ApplicationPackage(applicationStore.get(id.tenant(), id.application(), version)); - } - catch (RuntimeException s) { // If this fails, too, the first failure is most likely the relevant one. - e.addSuppressed(s); - throw e; - } - } + public ApplicationPackage getApplicationPackage(ApplicationId id, ApplicationVersion version) { + return new ApplicationPackage(applicationStore.get(id.tenant(), id.application(), version)); } /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 46d1d436521..fa81a990c70 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -45,7 +45,6 @@ public class LockedApplication { private final ApplicationMetrics metrics; private final Set deployKeys; private final OptionalLong projectId; - private final boolean internal; private final Optional latestVersion; private final Map instances; @@ -60,14 +59,14 @@ public class LockedApplication { application.deploymentSpec(), application.validationOverrides(), application.change(), application.outstandingChange(), application.deploymentIssueId(), application.ownershipIssueId(), application.owner(), application.majorVersion(), application.metrics(), application.deployKeys(), - application.projectId(), application.internal(), application.latestVersion(), application.instances()); + application.projectId(), application.latestVersion(), application.instances()); } private LockedApplication(Lock lock, TenantAndApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, ValidationOverrides validationOverrides, Change change, Change outstandingChange, Optional deploymentIssueId, Optional ownershipIssueId, Optional owner, OptionalInt majorVersion, ApplicationMetrics metrics, Set deployKeys, - OptionalLong projectId, boolean internal, Optional latestVersion, + OptionalLong projectId, Optional latestVersion, Map instances) { this.lock = lock; this.id = id; @@ -83,7 +82,6 @@ public class LockedApplication { this.metrics = metrics; this.deployKeys = deployKeys; this.projectId = projectId; - this.internal = internal; this.latestVersion = latestVersion; this.instances = Map.copyOf(instances); } @@ -92,7 +90,7 @@ public class LockedApplication { public Application get() { return new Application(id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances.values()); + projectId, latestVersion, instances.values()); } public LockedApplication withNewInstance(InstanceName instance) { @@ -100,7 +98,7 @@ public class LockedApplication { instances.put(instance, new Instance(id.instance(instance))); return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication with(InstanceName instance, UnaryOperator modification) { @@ -108,7 +106,7 @@ public class LockedApplication { instances.put(instance, modification.apply(instances.get(instance))); return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication without(InstanceName instance) { @@ -116,67 +114,61 @@ public class LockedApplication { instances.remove(instance); return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withNewSubmission(ApplicationVersion latestVersion) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, Optional.of(latestVersion), instances); - } - - public LockedApplication withBuiltInternally(boolean builtInternally) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, - deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, builtInternally, latestVersion, instances); + projectId, Optional.of(latestVersion), instances); } public LockedApplication withProjectId(OptionalLong projectId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withDeploymentIssueId(IssueId issueId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, Optional.ofNullable(issueId), ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication with(DeploymentSpec deploymentSpec) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication with(ValidationOverrides validationOverrides) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withChange(Change change) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withOutstandingChange(Change outstandingChange) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withOwnershipIssueId(IssueId issueId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, Optional.of(issueId), owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withOwner(User owner) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, Optional.of(owner), majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } /** Set a major version for this, or set to null to remove any major version override */ @@ -184,13 +176,13 @@ public class LockedApplication { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion == null ? OptionalInt.empty() : OptionalInt.of(majorVersion), - metrics, deployKeys, projectId, internal, latestVersion, instances); + metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication with(ApplicationMetrics metrics) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withDeployKey(PublicKey pemDeployKey) { @@ -198,7 +190,7 @@ public class LockedApplication { keys.add(pemDeployKey); return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, keys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } public LockedApplication withoutDeployKey(PublicKey pemDeployKey) { @@ -206,7 +198,7 @@ public class LockedApplication { keys.remove(pemDeployKey); return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, keys, - projectId, internal, latestVersion, instances); + projectId, latestVersion, instances); } @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 45bf8a43f07..8b38ed13b28 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 @@ -78,13 +78,11 @@ public class DeploymentTrigger { private final Controller controller; private final Clock clock; - private final BuildService buildService; private final JobController jobs; public DeploymentTrigger(Controller controller, BuildService buildService, Clock clock) { this.controller = Objects.requireNonNull(controller, "controller cannot be null"); this.clock = Objects.requireNonNull(clock, "clock cannot be null"); - this.buildService = Objects.requireNonNull(buildService, "buildService cannot be null"); this.jobs = controller.jobController(); } @@ -103,10 +101,9 @@ public class DeploymentTrigger { if (acceptNewApplicationVersion(application.get())) { application = application.withChange(application.get().change().with(version)) .withOutstandingChange(Change.empty()); - if (application.get().internal()) - for (Run run : jobs.active(id)) - if ( ! run.id().type().environment().isManuallyDeployed()) - jobs.abort(run.id()); + for (Run run : jobs.active(id)) + if ( ! run.id().type().environment().isManuallyDeployed()) + jobs.abort(run.id()); } else application = application.withOutstandingChange(Change.of(version)); @@ -197,13 +194,10 @@ public class DeploymentTrigger { log.log(LogLevel.DEBUG, String.format("Triggering %s: %s", job, job.triggering)); try { applications().lockApplicationOrThrow(TenantAndApplicationId.from(job.applicationId()), application -> { - if (application.get().internal()) - jobs.start(job.applicationId(), job.jobType, new Versions(job.triggering.platform(), - job.triggering.application(), - job.triggering.sourcePlatform(), - job.triggering.sourceApplication())); - else - buildService.trigger(job); + jobs.start(job.applicationId(), job.jobType, new Versions(job.triggering.platform(), + job.triggering.application(), + job.triggering.sourcePlatform(), + job.triggering.sourceApplication())); applications().store(application.with(job.applicationId().instance(), instance -> instance.withJobTriggering(job.jobType, job.triggering))); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 84f14109c4b..e22b70050ab 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -59,6 +59,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.Step.copyVespaLogs; import static com.yahoo.vespa.hosted.controller.deployment.Step.deactivateTester; import static com.yahoo.vespa.hosted.controller.deployment.Step.endTests; import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toUnmodifiableList; /** * A singleton owned by the controller, which contains the state and methods for controlling deployment jobs. @@ -198,7 +199,6 @@ public class JobController { /** Returns a list of all applications which have registered. */ public List applications() { return copyOf(controller.applications().asList().stream() - .filter(Application::internal) .map(Application::id) .iterator()); } @@ -206,7 +206,6 @@ public class JobController { /** Returns a list of all instances of applications which have registered. */ public List instances() { return copyOf(controller.applications().asList().stream() - .filter(Application::internal) .flatMap(application -> application.instances().values().stream()) .map(Instance::id) .iterator()); @@ -272,9 +271,9 @@ public class JobController { /** Returns a list of all active runs. */ public List active() { - return copyOf(applications().stream() - .flatMap(id -> active(id).stream()) - .iterator()); + return controller.applications().idList().stream() + .flatMap(id -> active(id).stream()) + .collect(toUnmodifiableList()); } /** Returns a list of all active runs for the given instance. */ @@ -347,9 +346,6 @@ public class JobController { ApplicationPackage applicationPackage, byte[] testPackageBytes) { AtomicReference version = new AtomicReference<>(); controller.applications().lockApplicationOrThrow(id, application -> { - if ( ! application.get().internal()) - application = registered(application); - long run = 1 + application.get().latestVersion() .map(latestVersion -> latestVersion.buildNumber().getAsLong()) .orElse(0L); @@ -377,31 +373,12 @@ public class JobController { return version.get(); } - /** Registers the given application, copying necessary application packages, and returns the modified version. */ - private LockedApplication registered(LockedApplication application) { - for (Instance instance : application.get().instances().values()) { - // TODO jvenstad: Remove when everyone has migrated off SDv3 pipelines. Real soon now! - // Copy all current packages to the new application store - instance.productionDeployments().values().stream() - .map(Deployment::applicationVersion) - .distinct() - .forEach(appVersion -> { - byte[] content = controller.applications().artifacts().getApplicationPackage(instance.id(), appVersion.id()); - controller.applications().applicationStore().put(instance.id().tenant(), instance.id().application(), appVersion, content); - }); - } - return application.withBuiltInternally(true); - } - /** Orders a run of the given type, or throws an IllegalStateException if that job type is already running. */ public void start(ApplicationId id, JobType type, Versions versions) { if ( ! type.environment().isManuallyDeployed() && versions.targetApplication().isUnknown()) throw new IllegalArgumentException("Target application must be a valid reference."); controller.applications().lockApplicationIfPresent(TenantAndApplicationId.from(id), application -> { - if ( ! application.get().internal()) - throw new IllegalArgumentException(id + " is not built here!"); - locked(id, type, __ -> { Optional last = last(id, type); if (last.flatMap(run -> active(run.id())).isPresent()) @@ -423,9 +400,6 @@ public class JobController { controller.applications().createApplication(TenantAndApplicationId.from(id), Optional.empty()); controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { - if ( ! application.get().internal()) - application = registered(application); - if ( ! application.get().instances().containsKey(id.instance())) application = application.withNewInstance(id.instance()); @@ -460,15 +434,6 @@ public class JobController { } } - /** Unregisters the given application and makes all associated data eligible for garbage collection. */ - public void unregister(TenantAndApplicationId id) { - controller.applications().lockApplicationIfPresent(id, application -> { - controller.applications().store(application.withBuiltInternally(false)); - for (InstanceName instance : application.get().instances().keySet()) - jobs(id.instance(instance)).forEach(type -> last(id.instance(instance), type).ifPresent(last -> abort(last.id()))); - }); - } - /** Deletes run data and tester deployments for applications which are unknown, or no longer built internally. */ public void collectGarbage() { Set applicationsToBuild = new HashSet<>(instances()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 75f489d3345..487413442b7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -179,7 +179,6 @@ public class ApplicationSerializer { application.projectId().ifPresent(projectId -> root.setLong(projectIdField, projectId)); application.deploymentIssueId().ifPresent(jiraIssueId -> root.setString(deploymentIssueField, jiraIssueId.value())); application.ownershipIssueId().ifPresent(issueId -> root.setString(ownershipIssueIdField, issueId.value())); - root.setBool(builtInternallyField, application.internal()); toSlime(application.change(), root, deployingField); toSlime(application.outstandingChange(), root, outstandingChangeField); application.owner().ifPresent(owner -> root.setString(ownerField, owner.username())); @@ -369,11 +368,10 @@ public class ApplicationSerializer { List instances = instancesFromSlime(id, deploymentSpec, root.field(instancesField)); OptionalLong projectId = Serializers.optionalLong(root.field(projectIdField)); Optional latestVersion = latestVersionFromSlime(root.field(latestVersionField)); - boolean builtInternally = root.field(builtInternallyField).asBool(); return new Application(id, createdAt, deploymentSpec, validationOverrides, deploying, outstandingChange, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, - deployKeys, projectId, builtInternally, latestVersion, instances); + deployKeys, projectId, latestVersion, instances); } private Optional latestVersionFromSlime(Inspector latestVersionObject) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 037fb4da987..637da842d2f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -56,6 +56,7 @@ import java.util.stream.Stream; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.collectingAndThen; +import static java.util.stream.Collectors.toUnmodifiableList; /** * Curator backed database for storing the persistence state of controllers. This maps controller specific operations @@ -352,16 +353,18 @@ public class CuratorDb { } private List readApplications(Predicate applicationFilter) { - return readApplicationIds().filter(applicationFilter) + return readApplicationIds().stream() + .filter(applicationFilter) .sorted() .map(this::readApplication) .flatMap(Optional::stream) .collect(Collectors.toUnmodifiableList()); } - private Stream readApplicationIds() { + public List readApplicationIds() { return curator.getChildren(applicationRoot).stream() - .map(TenantAndApplicationId::fromSerialized); + .map(TenantAndApplicationId::fromSerialized) + .collect(toUnmodifiableList()); } public void removeApplication(TenantAndApplicationId id) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 5cb7d7a6065..c8f5720327a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -290,11 +290,9 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), "all"); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/{choice}")) return cancelDeploy(path.get("tenant"), path.get("application"), path.get("choice")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/key")) return removeDeployKey(path.get("tenant"), path.get("application"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/submit")) return JobControllerApiHandlerHelper.unregisterResponse(controller.jobController(), path.get("tenant"), path.get("application")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return deleteInstance(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), "all"); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/{choice}")) return cancelDeploy(path.get("tenant"), path.get("application"), path.get("choice")); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/submit")) return JobControllerApiHandlerHelper.unregisterResponse(controller.jobController(), path.get("tenant"), path.get("application")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return JobControllerApiHandlerHelper.abortJobResponse(controller.jobController(), appIdFromPath(path), jobTypeFromPath(path)); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deactivate(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), true, request); @@ -833,7 +831,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { .steps(application.deploymentSpec().requireInstance(instance.name())) .sortedJobs(instance.deploymentJobs().jobStatus().values()); - object.setBool("deployedInternally", application.internal()); Cursor deploymentsArray = object.setArray("deploymentJobs"); for (JobStatus job : jobStatus) { Cursor jobObject = deploymentsArray.addObject(); @@ -1479,7 +1476,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { applicationVersion = Optional.of(ApplicationVersion.from(toSourceRevision(sourceRevision), buildNumber.asLong())); applicationPackage = Optional.of(controller.applications().getApplicationPackage(applicationId, - application.get().internal(), applicationVersion.get())); } @@ -1503,7 +1499,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { applicationVersion = Optional.of(version); vespaVersion = Optional.of(deployment.get().version()); applicationPackage = Optional.of(controller.applications().getApplicationPackage(applicationId, - application.get().internal(), applicationVersion.get())); } 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 cb4c1e71c43..899eb546bca 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 @@ -41,10 +41,12 @@ import java.util.ArrayDeque; import java.util.Comparator; import java.util.Deque; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy.conservative; @@ -128,7 +130,7 @@ class JobControllerApiHandlerHelper { pendingProduction, running, type, - Optional.ofNullable(status.get(type)), + status.get(type), deployment); }); }); @@ -220,15 +222,15 @@ class JobControllerApiHandlerHelper { private static void deploymentToSlime(Cursor deploymentObject, Instance instance, Change change, Map pendingProduction, Map running, - JobType type, Optional jobStatus, Deployment deployment) { + JobType type, JobStatus jobStatus, Deployment deployment) { deploymentObject.setLong("at", deployment.at().toEpochMilli()); deploymentObject.setString("platform", deployment.version().toString()); applicationVersionToSlime(deploymentObject.setObject("application"), deployment.applicationVersion()); - deploymentObject.setBool("verified", jobStatus.flatMap(job -> job.lastSuccess()) + deploymentObject.setBool("verified", jobStatus.lastSuccess() .map(Run::versions) - .filter(run -> run.targetPlatform().equals(deployment.version()) - && run.targetApplication().equals(deployment.applicationVersion())) - .isPresent()); + .filter(run -> run.targetPlatform().equals(deployment.version()) + && run.targetApplication().equals(deployment.applicationVersion())) + .isPresent()); if (running.containsKey(type)) deploymentObject.setString("status", running.get(type).steps().get(deployReal) == unfinished ? "deploying" : "verifying"); else if (change.hasTargets()) @@ -467,15 +469,6 @@ class JobControllerApiHandlerHelper { return new SlimeJsonResponse(slime); } - /** Unregisters the application from the internal deployment pipeline. */ - static HttpResponse unregisterResponse(JobController jobs, String tenantName, String applicationName) { - TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); - jobs.unregister(id); - Slime slime = new Slime(); - slime.setObject().setString("message", "Unregistered '" + id + "' from internal deployment pipeline."); - return new SlimeJsonResponse(slime); - } - private static String nameOf(RunStatus status) { switch (status) { case running: return "running"; 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 d0eb86c3ba1..d181ab8d38f 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 @@ -357,7 +357,6 @@ public class DeploymentContext { /** Deploy default application package, start a run for that change and return its ID */ public RunId newRun(JobType type) { - assertFalse(application().internal()); // Use this only once per test. submit(); readyJobsTrigger.maintain(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java index d3752fcaff4..f7d451ab931 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java @@ -190,7 +190,7 @@ public class JobRunnerTest { // Start a third run, then unregister and wait for data to be deleted. jobs.start(id, systemTest, versions); - jobs.unregister(appId); + tester.applications().deleteInstance(id); runner.maintain(); assertFalse(jobs.last(id, systemTest).isPresent()); assertTrue(jobs.runs(id, systemTest).isEmpty()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 9ac73604da5..7536272d6d9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -151,7 +151,6 @@ public class ApplicationSerializerTest { new ApplicationMetrics(0.5, 0.9), Set.of(publicKey, otherPublicKey), projectId, - true, Optional.of(applicationVersion1), instances); @@ -165,7 +164,6 @@ public class ApplicationSerializerTest { assertEquals(original.validationOverrides().xmlForm(), serialized.validationOverrides().xmlForm()); assertEquals(original.projectId(), serialized.projectId()); - assertEquals(original.internal(), serialized.internal()); assertEquals(original.deploymentIssueId(), serialized.deploymentIssueId()); assertEquals(0, serialized.require(id3.instance()).deployments().size()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 5deedaa2fb1..b5b7c1d0ad0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -696,12 +696,6 @@ public class ApplicationApiTest extends ControllerContainerTest { .userIdentity(USER_ID), "{\"message\":\"Aborting run 2 of staging-test for tenant1.application1.instance1\"}"); - // TODO jonmv: Remove, as this is the only option now. - // DELETE submission to unsubscribe from continuous deployment. - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/submit", DELETE) - .userIdentity(HOSTED_VESPA_OPERATOR), - "{\"message\":\"Unregistered 'tenant1.application1' from internal deployment pipeline.\"}"); - // PUT (create) the authenticated user byte[] data = new byte[0]; tester.assertResponse(request("/application/v4/user?user=new_user&domain=by", PUT) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json index bb81125e54d..0f759eaed55 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json @@ -9,7 +9,6 @@ "gitCommit": "commit1" }, "projectId": 1000, - "deployedInternally": true, "deploymentJobs": [ { "type": "system-test", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-without-change-multiple-deployments.json index 75493a30155..e8adfd579d5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-without-change-multiple-deployments.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-without-change-multiple-deployments.json @@ -9,7 +9,6 @@ "gitCommit": "commit1" }, "projectId": 1000, - "deployedInternally": true, "deploymentJobs": [ { "type": "system-test", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json index 061931336ed..3664af7116c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json @@ -20,7 +20,6 @@ } } }, - "deployedInternally": true, "deploymentJobs": [ { "type": "system-test", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json index 2dacacedf51..ddce3921518 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json @@ -20,7 +20,6 @@ } } }, - "deployedInternally": true, "deploymentJobs": [ { "type": "system-test", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs-direct-deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs-direct-deployment.json index 5535e286dcd..85a3245c308 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs-direct-deployment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs-direct-deployment.json @@ -1,79 +1,7 @@ { "devJobs": {}, - "deployments": [ - { - "us-west-1": { - "at": 0, - "application": { - "hash": "unknown" - }, - "verified": false, - "platform": "6.1" - } - } - ], + "deployments": [], "lastVersions": {}, "deploying": {}, - "jobs": { - "staging-test": { - "runs": [ - { - "reason": "Testing for productionUsWest1", - "wantedPlatform": "6.1", - "currentPlatform": "6.1", - "wantedApplication": { - "hash": "unknown" - }, - "currentApplication": { - "hash": "unknown" - }, - "tasks": { - "capacity": "running" - }, - "status": "pending" - } - ], - "url": "https://some.url:43/root/staging-test" - }, - "system-test": { - "runs": [ - { - "reason": "Testing for productionUsWest1", - "wantedPlatform": "6.1", - "currentPlatform": "6.1", - "wantedApplication": { - "hash": "unknown" - }, - "currentApplication": { - "hash": "unknown" - }, - "tasks": { - "capacity": "running" - }, - "status": "pending" - } - ], - "url": "https://some.url:43/root/system-test" - }, - "us-west-1": { - "runs": [ - { - "wantedPlatform": "6.1", - "currentPlatform": "6.1", - "wantedApplication": { - "hash": "unknown" - }, - "currentApplication": { - "hash": "unknown" - }, - "tasks": { - "staging-test": "pending", - "system-test": "pending" - }, - "status": "pending" - } - ], - "url": "https://some.url:43/root/production-us-west-1" - } - } + "jobs": {} } -- cgit v1.2.3 From 8b133ea876d8673ea016e28bb05f847271cdcc7d Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 7 Nov 2019 16:45:21 +0100 Subject: Remove BuildService --- .../controller/api/integration/BuildService.java | 84 ---------------------- .../api/integration/ServiceRegistry.java | 3 - .../api/integration/deployment/RunId.java | 2 +- .../api/integration/stubs/MockBuildService.java | 47 ------------ .../hosted/controller/ApplicationController.java | 2 +- .../controller/application/DeploymentJobs.java | 3 - .../controller/deployment/DeploymentTrigger.java | 22 +++--- .../controller/deployment/JobController.java | 8 +-- .../hosted/controller/deployment/JobStatus.java | 2 +- .../vespa/hosted/controller/ControllerTester.java | 2 - .../integration/ServiceRegistryMock.java | 11 --- 11 files changed, 12 insertions(+), 174 deletions(-) delete mode 100644 controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java delete mode 100644 controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockBuildService.java (limited to 'controller-server') diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java deleted file mode 100644 index 0f5b7f154e1..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/BuildService.java +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration; - -import com.yahoo.config.provision.ApplicationId; - -import java.util.Objects; - -/** - * @author jonmv - */ -// TODO jonmv: Remove this. -public interface BuildService { - - /** - * Enqueues a job defined by buildJob in an external build system. - * - * Implementations should throw an exception if the triggering fails. - */ - void trigger(BuildJob buildJob); - - /** - * Returns the state of the given job in the build service. - */ - JobState stateOf(BuildJob buildJob); - - enum JobState { - - /** Job is not running, and may be triggered. */ - idle, - - /** Job is already running, and will be queued if triggered now. */ - running, - - /** Job is running and queued and will automatically be started again after it finishes its current run. */ - queued, - - /** Job is disabled, i.e., it can not be triggered. */ - disabled - - } - - - class BuildJob { - - private final ApplicationId applicationId; - private final long projectId; - private final String jobName; - - protected BuildJob(ApplicationId applicationId, long projectId, String jobName) { - this.applicationId = applicationId; - this.projectId = projectId; - this.jobName = jobName; - } - - public static BuildJob of(ApplicationId applicationId, long projectId, String jobName) { - return new BuildJob(applicationId, projectId, jobName); - } - - public ApplicationId applicationId() { return applicationId; } - public long projectId() { return projectId; } - public String jobName() { return jobName; } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof BuildJob)) return false; - BuildJob job = (BuildJob) o; - return Objects.equals(applicationId, job.applicationId) && - Objects.equals(jobName, job.jobName); - } - - @Override - public int hashCode() { - return Objects.hash(applicationId, jobName); - } - - @Override - public String toString() { - return jobName + " for " + applicationId + " with project " + projectId; - } - - } - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java index 3d04c239798..4eb4f669225 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/ServiceRegistry.java @@ -73,9 +73,6 @@ public interface ServiceRegistry { RunDataStore runDataStore(); - // TODO: No longer used. Remove this once untangled from test code - BuildService buildService(); - TenantCost tenantCost(); ZoneRegistry zoneRegistry(); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java index 506c0482bca..ebce77f7e40 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java @@ -6,7 +6,7 @@ import com.yahoo.config.provision.ApplicationId; import java.util.Objects; /** - * Immutable ID of a job run by a {@link com.yahoo.vespa.hosted.controller.api.integration.BuildService}. + * Immutable ID of a deployment job. * * @author jonmv */ diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockBuildService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockBuildService.java deleted file mode 100644 index 2a8b06888b0..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockBuildService.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration.stubs; - -import com.yahoo.component.AbstractComponent; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.hosted.controller.api.integration.BuildService; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.JobState.idle; -import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.JobState.running; - -/** - * @author jonmv - */ -public class MockBuildService extends AbstractComponent implements BuildService { - - private final List jobs = Collections.synchronizedList(new ArrayList<>()); - - @Override - public void trigger(BuildJob buildJob) { - jobs.add(buildJob); - } - - @Override - public JobState stateOf(BuildJob buildJob) { - return jobs.contains(buildJob) ? running : idle; - } - - /** List all running jobs. */ - public List jobs() { - return new ArrayList<>(jobs); - } - - /** Clears all running jobs. */ - public void clear() { - jobs.clear(); - } - - /** Removes the given job for the given project and returns whether it was found. */ - public boolean remove(BuildJob buildJob) { - return jobs.remove(buildJob); - } - -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 90c3fabf2d6..d28df826957 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -147,7 +147,7 @@ public class ApplicationController { routingPolicies = new RoutingPolicies(controller); rotationRepository = new RotationRepository(rotationsConfig, this, curator); - deploymentTrigger = new DeploymentTrigger(controller, controller.serviceRegistry().buildService(), clock); + deploymentTrigger = new DeploymentTrigger(controller, clock); provisionApplicationCertificate = Flags.PROVISION_APPLICATION_CERTIFICATE.bindTo(controller.flagSource()); applicationPackageValidator = new ApplicationPackageValidator(controller); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index fb238e0a652..e126128ce2e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -3,10 +3,8 @@ package com.yahoo.vespa.hosted.controller.application; import com.google.common.collect.ImmutableMap; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.hosted.controller.api.integration.BuildService; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import java.util.Collection; import java.util.LinkedHashMap; @@ -116,7 +114,6 @@ public class DeploymentJobs { public boolean success() { return ! jobError.isPresent(); } public Optional version() { return version; } public Optional jobError() { return jobError; } - public BuildService.BuildJob buildJob() { return BuildService.BuildJob.of(applicationId, projectId, jobType.jobName()); } } 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 8b38ed13b28..d0f34857b06 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 @@ -12,7 +12,7 @@ import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.integration.BuildService; +import com.yahoo.vespa.hosted.controller.api.identifiers.InstanceId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; @@ -29,7 +29,6 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; @@ -42,7 +41,6 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; import static java.util.Collections.emptyList; @@ -80,7 +78,7 @@ public class DeploymentTrigger { private final Clock clock; private final JobController jobs; - public DeploymentTrigger(Controller controller, BuildService buildService, Clock clock) { + public DeploymentTrigger(Controller controller, Clock clock) { this.controller = Objects.requireNonNull(controller, "controller cannot be null"); this.clock = Objects.requireNonNull(clock, "clock cannot be null"); this.jobs = controller.jobController(); @@ -152,11 +150,6 @@ public class DeploymentTrigger { }); } - /** Returns a map of jobs that are scheduled to be run, grouped by the job type */ - public Map> jobsToRun() { - return computeReadyJobs().stream().collect(groupingBy(Job::jobType)); - } - /** * Finds and triggers jobs that can and should run but are currently not, and returns the number of triggered jobs. * @@ -437,7 +430,7 @@ public class DeploymentTrigger { private List runningProductionJobs(Map status) { return status.values().parallelStream() .filter(job -> job.isRunning()) - .map(job -> job.job().type()) + .map(job -> job.id().type()) .filter(JobType::isProduction) .collect(toList()); } @@ -497,7 +490,7 @@ public class DeploymentTrigger { public boolean alreadyTriggered(Map status, Versions versions) { return status.values().stream() - .filter(job -> job.job().type().isProduction()) + .filter(job -> job.id().type().isProduction()) .anyMatch(job -> job.lastTriggered() .map(Run::versions) .filter(versions::targetsMatch) @@ -575,9 +568,9 @@ public class DeploymentTrigger { // ---------- Data containers ---------- - // TODO jonmv: Replace with a JobSpec class not based on BuildJob. - private static class Job extends BuildJob { + private static class Job { + private final ApplicationId instanceId; private final JobType jobType; private final JobRun triggering; private final Instant availableSince; @@ -586,7 +579,7 @@ public class DeploymentTrigger { private Job(Instance instance, JobRun triggering, JobType jobType, Instant availableSince, boolean isRetry, boolean isApplicationUpgrade) { - super(instance.id(), 0L, jobType.jobName()); + this.instanceId = instance.id(); this.jobType = jobType; this.triggering = triggering; this.availableSince = availableSince; @@ -594,6 +587,7 @@ public class DeploymentTrigger { this.isApplicationUpgrade = isApplicationUpgrade; } + ApplicationId applicationId() { return instanceId; } JobType jobType() { return jobType; } Instant availableSince() { return availableSince; } // TODO jvenstad: This is 95% broken now. Change.at() can restore it. boolean isRetry() { return isRetry; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index e22b70050ab..331327117b5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -1,17 +1,14 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.deployment; -import com.google.common.collect.ImmutableMap; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; -import com.yahoo.vespa.hosted.controller.LockedApplication; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NotFoundException; @@ -34,10 +31,8 @@ import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; 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.List; import java.util.Map; @@ -45,7 +40,6 @@ import java.util.NavigableMap; import java.util.Optional; import java.util.Set; import java.util.SortedMap; -import java.util.TreeMap; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -296,7 +290,7 @@ public class JobController { return new DeploymentSteps(spec.requireInstance(id.instance()), controller::system) .jobs().stream() .map(type -> jobStatus(new JobId(id, type))) - .collect(Collectors.toUnmodifiableMap(status -> status.job().type(), + .collect(Collectors.toUnmodifiableMap(status -> status.id().type(), status -> status)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java index a338a766727..52d60aca388 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobStatus.java @@ -30,7 +30,7 @@ public class JobStatus { this.firstFailing = firstFailing(runs); } - public JobId job() { + public JobId id() { return id; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index c96d1648041..ff6a5d3795f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -19,10 +19,8 @@ import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; -import com.yahoo.vespa.hosted.controller.api.integration.BuildService; import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactoryMock; import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzDbMock; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java index 83d9d4d1ad0..eced161cebc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java @@ -23,7 +23,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerato import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGeneratorMock; import com.yahoo.vespa.hosted.controller.api.integration.stubs.DummyOwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.stubs.LoggingDeploymentIssues; -import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMeteringClient; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore; @@ -57,7 +56,6 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg private final MockTesterCloud mockTesterCloud = new MockTesterCloud(); private final ApplicationStoreMock applicationStoreMock = new ApplicationStoreMock(); private final MockRunDataStore mockRunDataStore = new MockRunDataStore(); - private final MockBuildService mockBuildService = new MockBuildService(); private final MockTenantCost mockTenantCost = new MockTenantCost(); public ServiceRegistryMock(SystemName system) { @@ -169,11 +167,6 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg return mockRunDataStore; } - @Override - public MockBuildService buildService() { - return mockBuildService; - } - @Override public MemoryNameService nameService() { return memoryNameService; @@ -211,10 +204,6 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg return artifactRepositoryMock; } - public MockBuildService buildServiceMock() { - return mockBuildService; - } - public ApplicationCertificateMock applicationCertificateMock() { return applicationCertificateMock; } -- cgit v1.2.3 From 24a1e9481ae732a67c8fe385293d07b8cf9b1096 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 8 Nov 2019 08:43:17 +0100 Subject: Add AbstractFilteringList as a super for such lists --- .../controller/application/ApplicationList.java | 96 ++++++-------- .../controller/application/EndpointList.java | 25 ++-- .../controller/deployment/DeploymentTrigger.java | 8 -- .../hosted/controller/deployment/JobList.java | 140 +++++++++++++++++++++ .../yahoo/collections/AbstractFilteringList.java | 80 ++++++++++++ .../collections/AbstractFilteringListTest.java | 69 ++++++++++ 6 files changed, 337 insertions(+), 81 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobList.java create mode 100644 vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java create mode 100644 vespajlib/src/test/java/com/yahoo/collections/AbstractFilteringListTest.java (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index 33d03801efc..8978af78574 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -1,7 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.application; -import com.google.common.collect.ImmutableList; +import com.yahoo.collections.AbstractFilteringList; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; @@ -16,7 +16,6 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Optional; -import java.util.function.Predicate; import java.util.stream.Collectors; /** @@ -24,55 +23,48 @@ import java.util.stream.Collectors; * * @author jonmv */ -public class ApplicationList { +public class ApplicationList extends AbstractFilteringList { - private final List list; - - private ApplicationList(List applications) { - this.list = applications; + private ApplicationList(Collection applications, boolean negate) { + super(applications, negate, ApplicationList::new); } // ----------------------------------- Factories - public static ApplicationList from(Collection applications) { - return new ApplicationList(List.copyOf(applications)); + public static ApplicationList from(Collection applications) { + return new ApplicationList(applications, false); } public static ApplicationList from(Collection ids, ApplicationController applications) { - return new ApplicationList(ids.stream() - .map(TenantAndApplicationId::from) - .distinct() - .map(applications::requireApplication) - .collect(Collectors.toUnmodifiableList())); + return from(ids.stream() + .map(TenantAndApplicationId::from) + .distinct() + .map(applications::requireApplication) + .collect(Collectors.toUnmodifiableList())); } // ----------------------------------- Accessors - /** Returns the applications in this as an immutable list */ - public List asList() { return list; } - /** Returns the ids of the applications in this as an immutable list */ - public List idList() { return list.stream().map(Application::id).collect(Collectors.toUnmodifiableList()); } - - public boolean isEmpty() { return list.isEmpty(); } - - public int size() { return list.size(); } + public List idList() { + return mapToList(Application::id); + } // ----------------------------------- Filters /** Returns the subset of applications which are upgrading (to any version), not considering block windows. */ public ApplicationList upgrading() { - return filteredOn(application -> application.change().platform().isPresent()); + return matching(application -> application.change().platform().isPresent()); } /** Returns the subset of applications which are currently upgrading to the given version */ public ApplicationList upgradingTo(Version version) { - return filteredOn(application -> isUpgradingTo(version, application)); + return matching(application -> isUpgradingTo(version, application)); } /** Returns the subset of applications which are not pinned to a certain Vespa version. */ public ApplicationList unpinned() { - return filteredOn(application -> ! application.change().isPinned()); + return matching(application -> ! application.change().isPinned()); } /** Returns the subset of applications which are currently not upgrading to the given version */ @@ -81,7 +73,7 @@ public class ApplicationList { } public ApplicationList notFailingUpgrade() { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .allMatch(instance -> JobList.from(instance) .failing() .not().failingApplicationChange() @@ -90,7 +82,7 @@ public class ApplicationList { /** Returns the subset of applications which are currently not upgrading to any of the given versions */ public ApplicationList notUpgradingTo(Collection versions) { - return filteredOn(application -> versions.stream().noneMatch(version -> isUpgradingTo(version, application))); + return matching(application -> versions.stream().noneMatch(version -> isUpgradingTo(version, application))); } /** @@ -104,91 +96,91 @@ public class ApplicationList { /** Returns the subset of applications which have changes left to deploy; blocked, or deploying */ public ApplicationList withChanges() { - return filteredOn(application -> application.change().hasTargets() || application.outstandingChange().hasTargets()); + return matching(application -> application.change().hasTargets() || application.outstandingChange().hasTargets()); } /** Returns the subset of applications which are currently not deploying a change */ public ApplicationList notDeploying() { - return filteredOn(application -> ! application.change().hasTargets()); + return matching(application -> ! application.change().hasTargets()); } /** Returns the subset of applications which currently does not have any failing jobs */ public ApplicationList notFailing() { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .noneMatch(instance -> instance.deploymentJobs().hasFailures())); } /** Returns the subset of applications which currently have failing jobs */ public ApplicationList failing() { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .anyMatch(instance -> instance.deploymentJobs().hasFailures())); } /** Returns the subset of applications which have been failing an upgrade to the given version since the given instant */ public ApplicationList failingUpgradeToVersionSince(Version version, Instant threshold) { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .anyMatch(instance -> failingUpgradeToVersionSince(instance, version, threshold))); } /** Returns the subset of applications which have been failing an application change since the given instant */ public ApplicationList failingApplicationChangeSince(Instant threshold) { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .anyMatch(instance -> failingApplicationChangeSince(instance, threshold))); } /** Returns the subset of applications which currently does not have any failing jobs on the given version */ public ApplicationList notFailingOn(Version version) { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .noneMatch(instance -> failingOn(version, instance))); } /** Returns the subset of applications which have at least one production deployment */ public ApplicationList withProductionDeployment() { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .anyMatch(instance -> instance.productionDeployments().size() > 0)); } /** Returns the subset of applications which started failing on the given version */ public ApplicationList startedFailingOn(Version version) { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .anyMatch(instance -> ! JobList.from(instance).firstFailing().on(version).isEmpty())); } /** Returns the subset of applications which has the given upgrade policy */ // TODO jonmv: Make this instance based when instances are orchestrated, and deployments reported per instance. public ApplicationList with(UpgradePolicy policy) { - return filteredOn(application -> application.deploymentSpec().instances().stream() + return matching(application -> application.deploymentSpec().instances().stream() .anyMatch(instance -> instance.upgradePolicy() == policy)); } /** Returns the subset of applications which does not have the given upgrade policy */ // TODO jonmv: Make this instance based when instances are orchestrated, and deployments reported per instance. public ApplicationList without(UpgradePolicy policy) { - return filteredOn(application -> application.deploymentSpec().instances().stream() + return matching(application -> application.deploymentSpec().instances().stream() .allMatch(instance -> instance.upgradePolicy() != policy)); } /** Returns the subset of applications which have at least one deployment on a lower version than the given one */ public ApplicationList onLowerVersionThan(Version version) { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .flatMap(instance -> instance.productionDeployments().values().stream()) .anyMatch(deployment -> deployment.version().isBefore(version))); } /** Returns the subset of applications which have a project ID */ public ApplicationList withProjectId() { - return filteredOn(application -> application.projectId().isPresent()); + return matching(application -> application.projectId().isPresent()); } /** Returns the subset of applications that are allowed to upgrade at the given time */ public ApplicationList canUpgradeAt(Instant instant) { - return filteredOn(application -> application.deploymentSpec().instances().stream() + return matching(application -> application.deploymentSpec().instances().stream() .allMatch(instance -> instance.canUpgradeAt(instant))); } /** Returns the subset of applications that have at least one assigned rotation */ public ApplicationList hasRotation() { - return filteredOn(application -> application.instances().values().stream() + return matching(application -> application.instances().values().stream() .anyMatch(instance -> ! instance.rotations().isEmpty())); } @@ -199,20 +191,14 @@ public class ApplicationList { * @param defaultMajorVersion the default major version to assume for applications not specifying one */ public ApplicationList allowMajorVersion(int targetMajorVersion, int defaultMajorVersion) { - return filteredOn(application -> targetMajorVersion <= application.deploymentSpec().majorVersion() + return matching(application -> targetMajorVersion <= application.deploymentSpec().majorVersion() .orElse(application.majorVersion() .orElse(defaultMajorVersion))); } /** Returns the subset of application which have submitted a non-empty deployment spec. */ public ApplicationList withDeploymentSpec() { - return filteredOn(application -> ! DeploymentSpec.empty.equals(application.deploymentSpec())); - } - - /** Returns the first n application in this (or all, if there are less than n). */ - public ApplicationList first(int n) { - if (list.size() < n) return this; - return new ApplicationList(list.subList(0, n)); + return matching(application -> ! DeploymentSpec.empty.equals(application.deploymentSpec())); } // ----------------------------------- Sorting @@ -223,10 +209,8 @@ public class ApplicationList { * Applications without any deployments are ordered first. */ public ApplicationList byIncreasingDeployedVersion() { - return new ApplicationList(list.stream() - .sorted(Comparator.comparing(application -> application.oldestDeployedPlatform() - .orElse(Version.emptyVersion))) - .collect(Collectors.toUnmodifiableList())); + return sortedBy(Comparator.comparing(application -> application.oldestDeployedPlatform() + .orElse(Version.emptyVersion))); } // ----------------------------------- Internal helpers @@ -257,8 +241,4 @@ public class ApplicationList { .isEmpty(); } - private ApplicationList filteredOn(Predicate condition) { - return new ApplicationList(list.stream().filter(condition).collect(Collectors.toUnmodifiableList())); - } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java index c4613db27d1..e12bb5cda7f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/EndpointList.java @@ -1,10 +1,12 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.application; +import com.yahoo.collections.AbstractFilteringList; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.application.Endpoint.Port; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.function.Predicate; @@ -17,41 +19,34 @@ import java.util.stream.Stream; * * @author mpolden */ -public class EndpointList { +public class EndpointList extends AbstractFilteringList { public static final EndpointList EMPTY = new EndpointList(List.of()); - private final List endpoints; - - private EndpointList(List endpoints) { + private EndpointList(Collection endpoints, boolean negate) { + super(endpoints, negate, EndpointList::new); if (endpoints.stream().distinct().count() != endpoints.size()) { throw new IllegalArgumentException("Expected all endpoints to be distinct, got " + endpoints); } - this.endpoints = List.copyOf(endpoints); } - public List asList() { - return endpoints; + private EndpointList(Collection endpoints) { + this(endpoints, false); } /** Returns the main endpoint, if any */ public Optional main() { - return endpoints.stream().filter(Predicate.not(Endpoint::legacy)).findFirst(); + return asList().stream().filter(Predicate.not(Endpoint::legacy)).findFirst(); } /** Returns the subset of endpoints are either legacy or not */ public EndpointList legacy(boolean legacy) { - return of(endpoints.stream().filter(endpoint -> endpoint.legacy() == legacy)); + return matching(endpoint -> endpoint.legacy() == legacy); } /** Returns the subset of endpoints with given scope */ public EndpointList scope(Endpoint.Scope scope) { - return of(endpoints.stream().filter(endpoint -> endpoint.scope() == scope)); - } - - /** Returns the union of this and given endpoints */ - public EndpointList and(EndpointList endpoints) { - return of(Stream.concat(asList().stream(), endpoints.asList().stream())); + return matching(endpoint -> endpoint.scope() == scope); } public static EndpointList of(Stream endpoints) { 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 d0f34857b06..ae7ff81c001 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 @@ -63,14 +63,6 @@ import static java.util.stream.Collectors.toList; */ public class DeploymentTrigger { - /* - * Instance orchestration TODO jonmv. - * Store new production application packages under non-instance path - * Read production packages from non-instance path, with fallback - * Deprecate and redirect some instance qualified paths in application/v4 - * Orchestrate deployment across instances. - */ - public static final Duration maxPause = Duration.ofDays(3); private final static Logger log = Logger.getLogger(DeploymentTrigger.class.getName()); 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 new file mode 100644 index 00000000000..973b718a4c6 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobList.java @@ -0,0 +1,140 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.deployment; + +import com.yahoo.collections.AbstractFilteringList; +import com.yahoo.component.Version; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; + +import java.time.Instant; +import java.util.Collection; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; + +/** + * A list of deployment jobs that can be filtered in various ways. + * + * @author jonmv + */ +public class JobList extends AbstractFilteringList { + + private JobList(Collection jobs, boolean negate) { + super(jobs, negate, JobList::new); + } + + // ----------------------------------- Factories + + public static JobList from(Collection jobs) { + return new JobList(jobs, false); + } + + // ----------------------------------- Basic filters + + /** Returns the subset of jobs which are currently upgrading */ + public JobList upgrading() { + return matching(job -> job.isRunning() + && job.lastSuccess().isPresent() + && job.lastSuccess().get().versions().targetPlatform().isBefore(job.lastTriggered().get().versions().targetPlatform())); + } + + /** Returns the subset of jobs which are currently failing */ + public JobList failing() { + return matching(job -> ! job.isSuccess()); + } + + /** Returns the subset of jobs which must be failing due to an application change */ + public JobList failingApplicationChange() { + return matching(JobList::failingApplicationChange); + } + + /** Returns the subset of jobs which are failing with the given run status */ + public JobList withStatus(RunStatus status) { + return matching(job -> job.lastStatus().map(status::equals).orElse(false)); + } + + /** Returns the subset of jobs of the given type -- most useful when negated */ + public JobList type(JobType type) { + return matching(job -> job.id().type() == type); + } + + /** Returns the subset of jobs of which are production jobs */ + public JobList production() { + return matching(job -> job.id().type().isProduction()); + } + + // ----------------------------------- JobRun filtering + + /** Returns the list in a state where the next filter is for the lastTriggered run type */ + public RunFilter lastTriggered() { + return new RunFilter(JobStatus::lastTriggered); + } + + /** Returns the list in a state where the next filter is for the lastCompleted run type */ + public RunFilter lastCompleted() { + return new RunFilter(JobStatus::lastCompleted); + } + + /** Returns the list in a state where the next filter is for the lastSuccess run type */ + public RunFilter lastSuccess() { + return new RunFilter(JobStatus::lastSuccess); + } + + /** Returns the list in a state where the next filter is for the firstFailing run type */ + public RunFilter firstFailing() { + return new RunFilter(JobStatus::firstFailing); + } + + + /** Allows sub-filters for runs of the given kind */ + public class RunFilter { + + private final Function> which; + + private RunFilter(Function> which) { + this.which = which; + } + + /** Returns the subset of jobs where the run of the given type exists */ + public JobList present() { + return matching(run -> true); + } + + /** Returns the subset of jobs where the run of the given type occurred before the given instant */ + public JobList startedBefore(Instant threshold) { + return matching(run -> run.start().isBefore(threshold)); + } + + /** Returns the subset of jobs where the run of the given type occurred after the given instant */ + public JobList startedAfter(Instant threshold) { + return matching(run -> run.start().isAfter(threshold)); + } + + /** Returns the subset of jobs where the run of the given type was on the given version */ + public JobList on(ApplicationVersion version) { + return matching(run -> run.versions().targetApplication().equals(version)); + } + + /** Returns the subset of jobs where the run of the given type was on the given version */ + public JobList on(Version version) { + return matching(run -> run.versions().targetPlatform().equals(version)); + } + + /** Transforms the JobRun condition to a JobStatus condition, by considering only the JobRun mapped by which, and executes */ + private JobList matching(Predicate condition) { + return JobList.this.matching(job -> which.apply(job).filter(condition).isPresent()); + } + + } + + // ----------------------------------- Internal helpers + + private static boolean failingApplicationChange(JobStatus job) { + if (job.isSuccess()) return false; + if (job.lastSuccess().isEmpty()) return true; // An application which never succeeded is surely bad. + if ( ! job.firstFailing().get().versions().targetPlatform().equals(job.lastSuccess().get().versions().targetPlatform())) return false; // Version change may be to blame. + return ! job.firstFailing().get().versions().targetApplication().equals(job.lastSuccess().get().versions().targetApplication()); // Return whether there is an application change. + } + +} + diff --git a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java new file mode 100644 index 00000000000..efcdeeaebfc --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java @@ -0,0 +1,80 @@ +package com.yahoo.collections; + +import java.util.Collection; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Stream; + +import static java.util.stream.Collectors.reducing; +import static java.util.stream.Collectors.toUnmodifiableList; + +/** + * Abstract, immutable list for subclassing with concrete types, and domain specific filters. + * + * @author jonmv + */ +public abstract class AbstractFilteringList> { + + private final List items; + private final boolean negate; + private final BiFunction, Boolean, ListType> constructor; + + // Subclass constructor: + // private SomeFilteringList(Collection items, boolean negate) { + // super(items, negate, SomeFilteringList::new); + // } + protected AbstractFilteringList(Collection items, boolean negate, BiFunction, Boolean, ListType> constructor) { + this.items = List.copyOf(items); + this.negate = negate; + this.constructor = constructor; + } + + /** Negates the next filter operation. All other operations which return a new list reset this modifier. */ + public final ListType not() { + return constructor.apply(items, ! negate); + } + + /** Returns a new list which is the result of filtering with the -- possibly negated -- condition. */ + public final ListType matching(Predicate condition) { + return constructor.apply(items.stream().filter(negate ? condition.negate() : condition).collect(toUnmodifiableList()), false); + } + + /** Returns the first n items in this list, or everything except those if negated. */ + public ListType first(int n) { + n = Math.min(n, items.size()); + return constructor.apply(items.subList(negate ? n : 0, negate ? items.size() : n), false); + } + + /** Returns the subset of items in this which are (not) present in the other list. */ + public ListType in(ListType others) { + return matching(new HashSet<>(others.asList())::contains); + } + + /** 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); + } + + /** Returns the items in this as an immutable list. */ + public final List asList() { return items; } + + /** Returns the items in this as an immutable list after mapping with the given function. */ + public final List mapToList(Function mapper) { + return items.stream().map(mapper).collect(toUnmodifiableList()); + } + + /** Returns the items sorted by the given comparator. */ + public final ListType sortedBy(Comparator comparator) { + return constructor.apply(items.stream().sorted(comparator).collect(toUnmodifiableList()), false); + } + + public final boolean isEmpty() { return items.isEmpty(); } + + public final int size() { return items.size(); } + +} diff --git a/vespajlib/src/test/java/com/yahoo/collections/AbstractFilteringListTest.java b/vespajlib/src/test/java/com/yahoo/collections/AbstractFilteringListTest.java new file mode 100644 index 00000000000..3f9342c5f45 --- /dev/null +++ b/vespajlib/src/test/java/com/yahoo/collections/AbstractFilteringListTest.java @@ -0,0 +1,69 @@ +package com.yahoo.collections; + +import org.junit.Test; + +import java.util.Comparator; +import java.util.List; +import java.util.Locale; + +import static org.junit.Assert.assertEquals; + +/** + * @author jonmv + */ +public class AbstractFilteringListTest { + + @Test + public void testOperations() { + MyList list = MyList.of("ABC", "abc", "cba", "bbb", "ABC"); + + assertEquals(List.of("ABC", "abc", "cba", "bbb", "ABC"), + list.first(100).asList()); + + assertEquals(List.of("ABC", "abc"), + list.first(2).asList()); + + assertEquals(List.of("cba", "bbb", "ABC"), + list.not().first(2).asList()); + + assertEquals(List.of("ABC", "ABC", "abc", "bbb"), + list.sortedBy(Comparator.naturalOrder()).first(4).asList()); + + assertEquals(List.of("abc", "cba", "bbb"), + list.allLowercase().asList()); + + assertEquals(List.of("ABC", "ABC"), + list.not().allLowercase().asList()); + + assertEquals(List.of("abc", "cba", "bbb"), + list.not().not().allLowercase().asList()); + + assertEquals(List.of(3, 3, 3, 3, 3), + list.mapToList(String::length)); + + assertEquals(List.of("ABC", "ABC"), + list.in(MyList.of("ABC", "CBA")).asList()); + + assertEquals(List.of("abc", "cba", "bbb"), + list.not().in(MyList.of("ABC", "CBA")).asList()); + + assertEquals(List.of("ABC", "abc", "cba", "bbb", "ABC", "aaa"), + list.and(MyList.of("aaa")).asList()); + } + + private static class MyList extends AbstractFilteringList { + + private MyList(List strings, boolean negate) { + super(strings, negate, MyList::new); + } + + private static MyList of(String... strings) { + return new MyList(List.of(strings), false); + } + + private MyList allLowercase() { + return matching(string -> string.toLowerCase(Locale.ROOT).equals(string)); + } + } + +} -- cgit v1.2.3 From 3e43ffd02d19a0af114f9d341b34ab6a621607dc Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 8 Nov 2019 09:07:40 +0100 Subject: Use not() for negated conditions in ApplicationList --- .../controller/application/ApplicationList.java | 60 ++++++---------------- .../hosted/controller/maintenance/Upgrader.java | 18 +++---- .../hosted/controller/versions/VespaVersion.java | 8 ++- 3 files changed, 28 insertions(+), 58 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index 8978af78574..3fe8e9f52c3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.Instance; import java.time.Instant; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Optional; @@ -59,7 +58,12 @@ public class ApplicationList extends AbstractFilteringList isUpgradingTo(version, application)); + return upgradingTo(List.of(version)); + } + + /** Returns the subset of applications which are currently upgrading to the given version */ + public ApplicationList upgradingTo(Collection versions) { + return matching(application -> versions.stream().anyMatch(version -> isUpgradingTo(version, application))); } /** Returns the subset of applications which are not pinned to a certain Vespa version. */ @@ -67,47 +71,22 @@ public class ApplicationList extends AbstractFilteringList ! application.change().isPinned()); } - /** Returns the subset of applications which are currently not upgrading to the given version */ - public ApplicationList notUpgradingTo(Version version) { - return notUpgradingTo(Collections.singletonList(version)); - } - - public ApplicationList notFailingUpgrade() { - return matching(application -> application.instances().values().stream() + public ApplicationList failingUpgrade() { + return matching(application -> ! application.instances().values().stream() .allMatch(instance -> JobList.from(instance) .failing() .not().failingApplicationChange() .isEmpty())); } - /** Returns the subset of applications which are currently not upgrading to any of the given versions */ - public ApplicationList notUpgradingTo(Collection versions) { - return matching(application -> versions.stream().noneMatch(version -> isUpgradingTo(version, application))); - } - - /** - * Returns the subset of applications which are currently not upgrading to the given version, - * or returns all if no version is specified - */ - public ApplicationList notUpgradingTo(Optional version) { - if (version.isEmpty()) return this; - return notUpgradingTo(version.get()); - } - /** Returns the subset of applications which have changes left to deploy; blocked, or deploying */ public ApplicationList withChanges() { return matching(application -> application.change().hasTargets() || application.outstandingChange().hasTargets()); } - /** Returns the subset of applications which are currently not deploying a change */ - public ApplicationList notDeploying() { - return matching(application -> ! application.change().hasTargets()); - } - - /** Returns the subset of applications which currently does not have any failing jobs */ - public ApplicationList notFailing() { - return matching(application -> application.instances().values().stream() - .noneMatch(instance -> instance.deploymentJobs().hasFailures())); + /** Returns the subset of applications which are currently deploying a change */ + public ApplicationList deploying() { + return matching(application -> application.change().hasTargets()); } /** Returns the subset of applications which currently have failing jobs */ @@ -119,19 +98,19 @@ public class ApplicationList extends AbstractFilteringList application.instances().values().stream() - .anyMatch(instance -> failingUpgradeToVersionSince(instance, version, threshold))); + .anyMatch(instance -> failingUpgradeToVersionSince(instance, version, threshold))); } /** Returns the subset of applications which have been failing an application change since the given instant */ public ApplicationList failingApplicationChangeSince(Instant threshold) { return matching(application -> application.instances().values().stream() - .anyMatch(instance -> failingApplicationChangeSince(instance, threshold))); + .anyMatch(instance -> failingApplicationChangeSince(instance, threshold))); } - /** Returns the subset of applications which currently does not have any failing jobs on the given version */ - public ApplicationList notFailingOn(Version version) { + /** Returns the subset of applications which currently have failing jobs on the given version */ + public ApplicationList failingOn(Version version) { return matching(application -> application.instances().values().stream() - .noneMatch(instance -> failingOn(version, instance))); + .anyMatch(instance -> failingOn(version, instance))); } /** Returns the subset of applications which have at least one production deployment */ @@ -153,13 +132,6 @@ public class ApplicationList extends AbstractFilteringList instance.upgradePolicy() == policy)); } - /** Returns the subset of applications which does not have the given upgrade policy */ - // TODO jonmv: Make this instance based when instances are orchestrated, and deployments reported per instance. - public ApplicationList without(UpgradePolicy policy) { - return matching(application -> application.deploymentSpec().instances().stream() - .allMatch(instance -> instance.upgradePolicy() != policy)); - } - /** Returns the subset of applications which have at least one deployment on a lower version than the given one */ public ApplicationList onLowerVersionThan(Version version) { return matching(application -> application.instances().values().stream() 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 08fa3abcd9f..28e276c1497 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 @@ -49,29 +49,29 @@ public class Upgrader extends Maintainer { @Override public void maintain() { // Determine target versions for each upgrade policy - Optional canaryTarget = controller().versionStatus().systemVersion().map(VespaVersion::versionNumber); + Version canaryTarget = controller().systemVersion(); Collection defaultTargets = targetVersions(Confidence.normal); Collection conservativeTargets = targetVersions(Confidence.high); // Cancel upgrades to broken targets (let other ongoing upgrades complete to avoid starvation) for (VespaVersion version : controller().versionStatus().versions()) { if (version.confidence() == Confidence.broken) - cancelUpgradesOf(applications().without(UpgradePolicy.canary).upgradingTo(version.versionNumber()), + cancelUpgradesOf(applications().not().with(UpgradePolicy.canary).upgradingTo(version.versionNumber()), version.versionNumber() + " is broken"); } // Canaries should always try the canary target - cancelUpgradesOf(applications().with(UpgradePolicy.canary).upgrading().notUpgradingTo(canaryTarget), + cancelUpgradesOf(applications().with(UpgradePolicy.canary).upgrading().not().upgradingTo(canaryTarget), "Outdated target version for Canaries"); // Cancel *failed* upgrades to earlier versions, as the new version may fix it String reason = "Failing on outdated version"; - cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().notUpgradingTo(defaultTargets), reason); - cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().notUpgradingTo(conservativeTargets), reason); + cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().not().upgradingTo(defaultTargets), reason); + cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().not().upgradingTo(conservativeTargets), reason); // Schedule the right upgrades ApplicationList applications = applications(); - canaryTarget.ifPresent(target -> upgrade(applications.with(UpgradePolicy.canary), target)); + upgrade(applications.with(UpgradePolicy.canary), canaryTarget); defaultTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.defaultPolicy), target)); conservativeTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.conservative), target)); } @@ -98,13 +98,13 @@ public class Upgrader extends Maintainer { applications = applications.withProductionDeployment(); applications = applications.onLowerVersionThan(version); applications = applications.allowMajorVersion(version.getMajor(), targetMajorVersion().orElse(version.getMajor())); - applications = applications.notDeploying(); // wait with applications deploying an application change or already upgrading - applications = applications.notFailingOn(version); // try to upgrade only if it hasn't failed on this version + applications = applications.not().deploying(); // wait with applications deploying an application change or already upgrading + applications = applications.not().failingOn(version); // try to upgrade only if it hasn't failed on this version applications = applications.canUpgradeAt(controller().clock().instant()); // wait with applications that are currently blocking upgrades applications = applications.byIncreasingDeployedVersion(); // start with lowest versions for (Application application : applications.with(UpgradePolicy.canary).asList()) controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version)); - for (Application application : applications.without(UpgradePolicy.canary).first(numberOfApplicationsToUpgrade()).asList()) + for (Application application : applications.not().with(UpgradePolicy.canary).first(numberOfApplicationsToUpgrade()).asList()) controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java index d0cf5aac2d9..296639245a3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java @@ -45,9 +45,7 @@ public class VespaVersion implements Comparable { public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) { // 'production on this': All deployment jobs upgrading to this version have completed without failure - ApplicationList productionOnThis = ApplicationList.from(statistics.production(), controller.applications()) - .notUpgradingTo(statistics.version()) - .notFailingUpgrade(); + ApplicationList productionOnThis = ApplicationList.from(statistics.production(), controller.applications()).not().upgradingTo(statistics.version()).not().failingUpgrade(); ApplicationList failingOnThis = ApplicationList.from(statistics.failing(), controller.applications()); ApplicationList all = ApplicationList.from(controller.applications().asList()) .withProductionDeployment(); @@ -162,8 +160,8 @@ public class VespaVersion implements Comparable { private static boolean nonCanaryApplicationsBroken(Version version, ApplicationList failingOnThis, ApplicationList productionOnThis) { - ApplicationList failingNonCanaries = failingOnThis.without(UpgradePolicy.canary).startedFailingOn(version); - ApplicationList productionNonCanaries = productionOnThis.without(UpgradePolicy.canary); + ApplicationList failingNonCanaries = failingOnThis.not().with(UpgradePolicy.canary).startedFailingOn(version); + ApplicationList productionNonCanaries = productionOnThis.not().with(UpgradePolicy.canary); if (productionNonCanaries.size() + failingNonCanaries.size() == 0) return false; -- cgit v1.2.3 From bb576c41791e31d25e00b49c2b1e871b5d3d09c3 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 8 Nov 2019 10:31:23 +0100 Subject: Wrap Application and its JobStatuses in DeploymentStatus --- .../controller/deployment/DeploymentStatus.java | 49 ++++++++++++++++++++++ .../controller/deployment/DeploymentTrigger.java | 6 +-- .../controller/deployment/JobController.java | 16 ++++--- .../hosted/controller/deployment/JobList.java | 7 ++-- .../hosted/controller/deployment/Versions.java | 5 --- .../application/JobControllerApiHandlerHelper.java | 6 +-- 6 files changed, 67 insertions(+), 22 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java (limited to 'controller-server') 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 new file mode 100644 index 00000000000..1582bc144f4 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -0,0 +1,49 @@ +package com.yahoo.vespa.hosted.controller.deployment; + +import com.yahoo.config.provision.InstanceName; +import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; + +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; + +/** + * Status of the deployment jobs of an {@link Application}. + * + * @author jonmv + */ +public class DeploymentStatus { + + private final Application application; + private final Map jobs; + + public DeploymentStatus(Application application, Map jobs) { + this.application = Objects.requireNonNull(application); + this.jobs = Map.copyOf(jobs); + } + + public Application application() { + return application; + } + + public Map jobs() { + return jobs; + } + + public boolean hasFailures() { + return ! JobList.from(jobs.values()) + .failing() + .not().withStatus(RunStatus.outOfCapacity) + .isEmpty(); + } + + public Map instanceJobs(InstanceName instance) { + return jobs.entrySet().stream() + .filter(entry -> entry.getKey().application().equals(application.id().instance(instance))) + .collect(Collectors.toUnmodifiableMap(entry -> entry.getKey().type(), + entry -> entry.getValue())); + } + +} 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 ae7ff81c001..f1b93c7b3b2 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 @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.identifiers.InstanceId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; @@ -205,7 +204,7 @@ public class DeploymentTrigger { Versions versions = Versions.from(application.change(), application, deploymentFor(instance, jobType), controller.systemVersion()); String reason = "Job triggered manually by " + user; - var jobStatus = jobs.jobStatus(applicationId, application.deploymentSpec()); + var jobStatus = jobs.deploymentStatus(application).instanceJobs(instance.name()); return (jobType.isProduction() && ! isTested(jobStatus, versions) ? testJobs(application.deploymentSpec(), application.change(), instance, jobStatus, versions, reason, clock.instant(), __ -> true).stream() : Stream.of(deploymentJob(instance, versions, application.change(), jobType, jobStatus.get(jobType), reason, clock.instant()))) @@ -299,8 +298,9 @@ public class DeploymentTrigger { Collection instances = application.deploymentSpec().instances().stream() .flatMap(instance -> application.get(instance.name()).stream()) .collect(Collectors.toUnmodifiableList()); + DeploymentStatus deploymentStatus = this.jobs.deploymentStatus(application); for (Instance instance : instances) { - var jobStatus = this.jobs.jobStatus(instance.id(), application.deploymentSpec()); + var jobStatus = deploymentStatus.instanceJobs(instance.name()); Change change = application.change(); Optional completedAt = max(Optional.ofNullable(jobStatus.get(systemTest)) .flatMap(job -> job.lastSuccess().map(run -> run.end().get())), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 331327117b5..e6c59b464a6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; @@ -54,6 +55,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.Step.deactivateTester import static com.yahoo.vespa.hosted.controller.deployment.Step.endTests; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toUnmodifiableList; +import static java.util.stream.Collectors.toUnmodifiableMap; /** * A singleton owned by the controller, which contains the state and methods for controlling deployment jobs. @@ -286,12 +288,14 @@ public class JobController { } /** Returns the job status of all declared jobs for the given instance id, indexed by job type. */ - public Map jobStatus(ApplicationId id, DeploymentSpec spec) { - return new DeploymentSteps(spec.requireInstance(id.instance()), controller::system) - .jobs().stream() - .map(type -> jobStatus(new JobId(id, type))) - .collect(Collectors.toUnmodifiableMap(status -> status.id().type(), - status -> status)); + public DeploymentStatus deploymentStatus(Application application) { + return new DeploymentStatus(application, + application.deploymentSpec().instances().stream() + .flatMap(spec -> new DeploymentSteps(spec, controller::system) + .jobs().stream() + .map(type -> jobStatus(new JobId(application.id().instance(spec.name()), type)))) + .collect(toUnmodifiableMap(status -> status.id(), + status -> status))); } /** Changes the status of the given step, for the given run, provided it is still active. */ 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 973b718a4c6..1ef83153bef 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 @@ -8,6 +8,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import java.time.Instant; import java.util.Collection; +import java.util.List; import java.util.Optional; import java.util.function.Function; import java.util.function.Predicate; @@ -33,7 +34,7 @@ public class JobList extends AbstractFilteringList { /** Returns the subset of jobs which are currently upgrading */ public JobList upgrading() { - return matching(job -> job.isRunning() + return matching(job -> job.isRunning() && job.lastSuccess().isPresent() && job.lastSuccess().get().versions().targetPlatform().isBefore(job.lastTriggered().get().versions().targetPlatform())); } @@ -54,8 +55,8 @@ public class JobList extends AbstractFilteringList { } /** Returns the subset of jobs of the given type -- most useful when negated */ - public JobList type(JobType type) { - return matching(job -> job.id().type() == type); + public JobList type(JobType... types) { + return matching(job -> List.of(types).contains(job.id().type())); } /** Returns the subset of jobs of which are production jobs */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index 5d4a380411d..b2b217d0814 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -80,11 +80,6 @@ public class Versions { targetApplication.equals(versions.targetApplication()); } - public boolean targetsMatch(JobStatus.JobRun jobRun) { - return targetPlatform.equals(jobRun.platform()) && - targetApplication.equals(jobRun.application()); - } - @Override public boolean equals(Object o) { if (this == o) return true; 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 899eb546bca..9a9a9798c6d 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 @@ -16,7 +16,6 @@ import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.NotExistsException; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; -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.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; @@ -40,13 +39,10 @@ import java.net.URI; import java.util.ArrayDeque; import java.util.Comparator; import java.util.Deque; -import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.stream.Collectors; import static com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy.conservative; @@ -81,7 +77,7 @@ class JobControllerApiHandlerHelper { Instance instance = application.require(id.instance()); Change change = application.change(); DeploymentSteps steps = new DeploymentSteps(application.deploymentSpec().requireInstance(id.instance()), controller::system); - Map status = controller.jobController().jobStatus(id, application.deploymentSpec()); + Map status = controller.jobController().deploymentStatus(application).instanceJobs(id.instance()); // The logic for pending runs imitates DeploymentTrigger logic; not good, but the trigger wiring must be re-written to reuse :S Map pendingProduction = -- cgit v1.2.3 From bb87cf716748abffda3959389d1d82a3e0048976 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 8 Nov 2019 11:54:03 +0100 Subject: Write "true" for field which is being removed --- .../yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java | 1 + 1 file changed, 1 insertion(+) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 487413442b7..79296476aaa 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -179,6 +179,7 @@ public class ApplicationSerializer { application.projectId().ifPresent(projectId -> root.setLong(projectIdField, projectId)); application.deploymentIssueId().ifPresent(jiraIssueId -> root.setString(deploymentIssueField, jiraIssueId.value())); application.ownershipIssueId().ifPresent(issueId -> root.setString(ownershipIssueIdField, issueId.value())); + root.setBool(builtInternallyField, true); // TODO jonmv: remove when the change with this comment has deployed. toSlime(application.change(), root, deployingField); toSlime(application.outstandingChange(), root, outstandingChangeField); application.owner().ifPresent(owner -> root.setString(ownerField, owner.username())); -- cgit v1.2.3