From a6920f6ae9c4292fac233c41ab21d0d9fcc950e9 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 28 Feb 2022 16:20:52 +0100 Subject: Store non-canonical reasons for runs, and avoid auto-aborting these from dep.orch. --- .../controller/deployment/DeploymentTrigger.java | 30 +++++++++++------- .../controller/deployment/JobController.java | 16 ++++------ .../vespa/hosted/controller/deployment/Run.java | 35 ++++++++++++-------- .../controller/maintenance/DeploymentUpgrader.java | 2 +- .../maintenance/EndpointCertificateMaintainer.java | 2 +- .../maintenance/RetriggerMaintainer.java | 3 +- .../controller/persistence/RunSerializer.java | 5 ++- .../restapi/application/ApplicationApiHandler.java | 6 ++-- .../restapi/controller/ControllerApiHandler.java | 2 +- .../deployment/DeploymentTriggerTest.java | 6 ++-- .../maintenance/DeploymentExpirerTest.java | 2 +- .../controller/maintenance/JobRunnerTest.java | 37 ++++++++++++---------- .../maintenance/RetriggerMaintainerTest.java | 4 +-- .../controller/persistence/RunSerializerTest.java | 4 ++- .../persistence/testdata/run-status.json | 3 +- .../restapi/deployment/BadgeApiTest.java | 2 +- 16 files changed, 90 insertions(+), 69 deletions(-) (limited to 'controller-server/src') 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 5a99f6b1e50..52f87f6cff6 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 @@ -167,18 +167,24 @@ public class DeploymentTrigger { return triggeredJobs; } + + /** Attempts to trigger the given job. */ + private void trigger(Job job) { + trigger(job, null); + } + /** Attempts to trigger the given job. */ - public void trigger(Job job) { + private void trigger(Job job, String reason) { log.log(Level.FINE, () -> "Triggering " + job); applications().lockApplicationOrThrow(TenantAndApplicationId.from(job.applicationId()), application -> { - jobs.start(job.applicationId(), job.jobType, job.versions); + jobs.start(job.applicationId(), job.jobType, job.versions, false, Optional.ofNullable(reason)); applications().store(application.with(job.applicationId().instance(), instance -> instance.withJobPause(job.jobType, OptionalLong.empty()))); }); } /** Force triggering of a job for given instance, with same versions as last run. */ - public JobId reTrigger(ApplicationId applicationId, JobType jobType) { + public JobId reTrigger(ApplicationId applicationId, JobType jobType, String reason) { Application application = applications().requireApplication(TenantAndApplicationId.from(applicationId)); Instance instance = application.require(applicationId.instance()); JobId job = new JobId(instance.id(), jobType); @@ -186,17 +192,17 @@ public class DeploymentTrigger { Versions versions = jobStatus.lastTriggered() .orElseThrow(() -> new IllegalArgumentException(job + " has never been triggered")) .versions(); - trigger(deploymentJob(instance, versions, jobType, jobStatus, clock.instant())); + trigger(deploymentJob(instance, versions, jobType, jobStatus, clock.instant()), reason); return job; } /** Force triggering of a job for given instance. */ - public List forceTrigger(ApplicationId applicationId, JobType jobType, String user, boolean requireTests) { + public List forceTrigger(ApplicationId applicationId, JobType jobType, String reason, boolean requireTests) { Application application = applications().requireApplication(TenantAndApplicationId.from(applicationId)); Instance instance = application.require(applicationId.instance()); JobId job = new JobId(instance.id(), jobType); if (job.type().environment().isManuallyDeployed()) - return forceTriggerManualJob(job); + return forceTriggerManualJob(job, reason); DeploymentStatus status = jobs.deploymentStatus(application); Versions versions = Versions.from(instance.change(), application, status.deploymentFor(job), controller.readSystemVersion()); @@ -210,23 +216,23 @@ public class DeploymentTrigger { .collect(toMap(Map.Entry::getKey, Map.Entry::getValue)); jobs.forEach((jobId, versionsList) -> { - trigger(deploymentJob(instance, versionsList.get(0).versions(), jobId.type(), status.jobs().get(jobId).get(), clock.instant())); + trigger(deploymentJob(instance, versionsList.get(0).versions(), jobId.type(), status.jobs().get(jobId).get(), clock.instant()), reason); }); return List.copyOf(jobs.keySet()); } - private List forceTriggerManualJob(JobId job) { + private List forceTriggerManualJob(JobId job, String reason) { Run last = jobs.last(job).orElseThrow(() -> new IllegalArgumentException(job + " has never been run")); Versions target = new Versions(controller.readSystemVersion(), last.versions().targetApplication(), Optional.of(last.versions().targetPlatform()), Optional.of(last.versions().targetApplication())); - jobs.start(job.application(), job.type(), target, true); + jobs.start(job.application(), job.type(), target, true, Optional.of(reason)); return List.of(job); } /** Retrigger job. If the job is already running, it will be canceled, and retrigger enqueued. */ - public Optional reTriggerOrAddToQueue(DeploymentId deployment) { + public Optional reTriggerOrAddToQueue(DeploymentId deployment, String reason) { JobType jobType = JobType.from(controller.system(), deployment.zoneId()) .orElseThrow(() -> new IllegalArgumentException(Text.format("No job to trigger for (system/zone): %s/%s", controller.system().value(), deployment.zoneId().value()))); Optional existingRun = controller.jobController().active(deployment.applicationId()).stream() @@ -250,7 +256,7 @@ public class DeploymentTrigger { controller.jobController().abort(run.id(), "force re-triggered"); return Optional.empty(); } else { - return Optional.of(reTrigger(deployment.applicationId(), jobType)); + return Optional.of(reTrigger(deployment.applicationId(), jobType, reason)); } } @@ -358,7 +364,7 @@ public class DeploymentTrigger { private void abortIfOutdated(DeploymentStatus status, Map> jobs, JobId job) { status.jobs().get(job) .flatMap(JobStatus::lastTriggered) - .filter(last -> ! last.hasEnded()) + .filter(last -> ! last.hasEnded() && last.reason().isEmpty()) .ifPresent(last -> { if (jobs.get(job).stream().noneMatch(versions -> versions.versions().targetsMatch(last.versions()) && versions.versions().sourcesMatchIfPresent(last.versions()))) { 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 88b47194936..70640278f4c 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 @@ -495,24 +495,19 @@ public class JobController { } /** 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) { - start(id, type, versions, false); + public void start(ApplicationId id, JobType type, Versions versions, boolean isRedeployment, Optional reason) { + start(id, type, versions, isRedeployment, JobProfile.of(type), reason); } /** 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, boolean isRedeployment) { - start(id, type, versions, isRedeployment, JobProfile.of(type)); - } - - /** 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, boolean isRedeployment, JobProfile profile) { + public void start(ApplicationId id, JobType type, Versions versions, boolean isRedeployment, JobProfile profile, Optional reason) { locked(id, type, __ -> { Optional last = last(id, type); if (last.flatMap(run -> active(run.id())).isPresent()) throw new IllegalArgumentException("Cannot start " + type + " for " + id + "; it is already running!"); RunId newId = new RunId(id, type, last.map(run -> run.id().number()).orElse(0L) + 1); - curator.writeLastRun(Run.initial(newId, versions, isRedeployment, controller.clock().instant(), profile)); + curator.writeLastRun(Run.initial(newId, versions, isRedeployment, controller.clock().instant(), profile, reason)); metric.jobStarted(newId.job()); }); } @@ -557,7 +552,8 @@ public class JobController { lastRun.map(run -> run.versions().targetPlatform()), lastRun.map(run -> run.versions().targetApplication())), false, - dryRun ? JobProfile.developmentDryRun : JobProfile.development); + dryRun ? JobProfile.developmentDryRun : JobProfile.development, + Optional.empty()); }); locked(id, type, __ -> { 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 c12448ab269..e73d3f52e1f 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 @@ -40,11 +40,12 @@ public class Run { private final Optional convergenceSummary; private final Optional testerCertificate; private final boolean dryRun; + private final Optional reason; // For deserialisation only -- do not use! public Run(RunId id, Map steps, Versions versions, boolean isRedeployment, Instant start, Optional end, Optional sleepUntil, RunStatus status, long lastTestRecord, Instant lastVespaLogTimestamp, Optional noNodesDownSince, - Optional convergenceSummary, Optional testerCertificate, boolean dryRun) { + Optional convergenceSummary, Optional testerCertificate, boolean dryRun, Optional reason) { this.id = id; this.steps = Collections.unmodifiableMap(new EnumMap<>(steps)); this.versions = versions; @@ -59,13 +60,14 @@ public class Run { this.convergenceSummary = convergenceSummary; this.testerCertificate = testerCertificate; this.dryRun = dryRun; + this.reason = reason; } - public static Run initial(RunId id, Versions versions, boolean isRedeployment, Instant now, JobProfile profile) { + public static Run initial(RunId id, Versions versions, boolean isRedeployment, Instant now, JobProfile profile, Optional triggeredBy) { EnumMap steps = new EnumMap<>(Step.class); profile.steps().forEach(step -> steps.put(step, StepInfo.initial(step))); return new Run(id, steps, requireNonNull(versions), isRedeployment, requireNonNull(now), Optional.empty(), Optional.empty(), running, - -1, Instant.EPOCH, Optional.empty(), Optional.empty(), Optional.empty(), profile == JobProfile.developmentDryRun); + -1, Instant.EPOCH, Optional.empty(), Optional.empty(), Optional.empty(), profile == JobProfile.developmentDryRun, triggeredBy); } /** Returns a new Run with the status of the given completed step set accordingly. */ @@ -79,7 +81,7 @@ public class Run { EnumMap steps = new EnumMap<>(this.steps); steps.put(step.get(), stepInfo.with(Step.Status.of(status))); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, this.status == running ? status : this.status, - lastTestRecord, lastVespaLogTimestamp, noNodesDownSince, convergenceSummary, testerCertificate, dryRun); + lastTestRecord, lastVespaLogTimestamp, noNodesDownSince, convergenceSummary, testerCertificate, dryRun, reason); } /** Returns a new Run with a new start time*/ @@ -94,19 +96,19 @@ public class Run { steps.put(step.get(), stepInfo.with(startTime)); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, status, lastTestRecord, lastVespaLogTimestamp, - noNodesDownSince, convergenceSummary, testerCertificate, dryRun); + noNodesDownSince, convergenceSummary, testerCertificate, dryRun, reason); } public Run finished(Instant now) { requireActive(); return new Run(id, steps, versions, isRedeployment, start, Optional.of(now), sleepUntil, status == running ? success : status, - lastTestRecord, lastVespaLogTimestamp, noNodesDownSince, convergenceSummary, Optional.empty(), dryRun); + lastTestRecord, lastVespaLogTimestamp, noNodesDownSince, convergenceSummary, Optional.empty(), dryRun, reason); } public Run aborted() { requireActive(); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, aborted, lastTestRecord, lastVespaLogTimestamp, - noNodesDownSince, convergenceSummary, testerCertificate, dryRun); + noNodesDownSince, convergenceSummary, testerCertificate, dryRun, reason); } public Run reset() { @@ -114,43 +116,43 @@ public class Run { Map reset = new EnumMap<>(steps); reset.replaceAll((step, __) -> StepInfo.initial(step)); return new Run(id, reset, versions, isRedeployment, start, end, sleepUntil, running, -1, lastVespaLogTimestamp, - Optional.empty(), Optional.empty(), testerCertificate, dryRun); + Optional.empty(), Optional.empty(), testerCertificate, dryRun, reason); } public Run with(long lastTestRecord) { requireActive(); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, status, lastTestRecord, lastVespaLogTimestamp, - noNodesDownSince, convergenceSummary, testerCertificate, dryRun); + noNodesDownSince, convergenceSummary, testerCertificate, dryRun, reason); } public Run with(Instant lastVespaLogTimestamp) { requireActive(); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, status, lastTestRecord, lastVespaLogTimestamp, - noNodesDownSince, convergenceSummary, testerCertificate, dryRun); + noNodesDownSince, convergenceSummary, testerCertificate, dryRun, reason); } public Run noNodesDownSince(Instant noNodesDownSince) { requireActive(); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, status, lastTestRecord, lastVespaLogTimestamp, - Optional.ofNullable(noNodesDownSince), convergenceSummary, testerCertificate, dryRun); + Optional.ofNullable(noNodesDownSince), convergenceSummary, testerCertificate, dryRun, reason); } public Run withSummary(ConvergenceSummary convergenceSummary) { requireActive(); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, status, lastTestRecord, lastVespaLogTimestamp, - noNodesDownSince, Optional.ofNullable(convergenceSummary), testerCertificate, dryRun); + noNodesDownSince, Optional.ofNullable(convergenceSummary), testerCertificate, dryRun, reason); } public Run with(X509Certificate testerCertificate) { requireActive(); return new Run(id, steps, versions, isRedeployment, start, end, sleepUntil, status, lastTestRecord, lastVespaLogTimestamp, - noNodesDownSince, convergenceSummary, Optional.of(testerCertificate), dryRun); + noNodesDownSince, convergenceSummary, Optional.of(testerCertificate), dryRun, reason); } public Run sleepingUntil(Instant instant) { requireActive(); return new Run(id, steps, versions, isRedeployment, start, end, Optional.of(instant), status, lastTestRecord, lastVespaLogTimestamp, - noNodesDownSince, convergenceSummary, testerCertificate, dryRun); + noNodesDownSince, convergenceSummary, testerCertificate, dryRun, reason); } /** Returns the id of this run. */ @@ -254,6 +256,11 @@ public class Run { /** Whether this is a dry run deployment. */ public boolean isDryRun() { return dryRun; } + /** The specific reason for triggering this run, if any. This should be empty for jobs triggered bvy deployment orchestration. */ + public Optional reason() { + return reason; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java index 73ee8527492..bf3b9b90f66 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java @@ -49,7 +49,7 @@ public class DeploymentUpgrader extends ControllerMaintainer { if ( ! isLikelyNightFor(job)) continue; log.log(Level.FINE, "Upgrading deployment of " + instance.id() + " in " + deployment.zone()); - controller().jobController().start(instance.id(), JobType.from(controller().system(), deployment.zone()).get(), target, true); + controller().jobController().start(instance.id(), JobType.from(controller().system(), deployment.zone()).get(), target, true, Optional.of("automated upgrade")); } catch (Exception e) { failures.incrementAndGet(); log.log(Level.WARNING, "Failed upgrading " + deployment + " of " + instance + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index cc00572e760..13b91c0cee3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -107,7 +107,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { .ifPresent(instance -> instance.productionDeployments().forEach((zone, deployment) -> { if (deployment.at().isBefore(refreshTime)) { JobType job = JobType.from(controller().system(), zone).orElseThrow(); - deploymentTrigger.reTrigger(applicationId, job); + deploymentTrigger.reTrigger(applicationId, job, "re-triggered by EndpointCertificateMaintainer"); log.info("Re-triggering deployment job " + job.jobName() + " for instance " + applicationId.serializedForm() + " to roll out refreshed endpoint certificate"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java index 73d28a1eeae..4c72f6747d5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java @@ -31,7 +31,8 @@ public class RetriggerMaintainer extends ControllerMaintainer { retriggerEntries.stream() .filter(this::needsTrigger) .filter(entry -> readyToTrigger(entry.jobId())) - .forEach(entry -> controller().applications().deploymentTrigger().reTrigger(entry.jobId().application(), entry.jobId().type())); + .forEach(entry -> controller().applications().deploymentTrigger().reTrigger(entry.jobId().application(), entry.jobId().type(), + "re-triggered by RetriggerMaintainer")); // Remove all jobs that has succeeded with the required job run and persist the list List remaining = retriggerEntries.stream() 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 9a301f50b1a..c3d81b8dcd5 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 @@ -105,6 +105,7 @@ class RunSerializer { private static final String convergenceSummaryField = "convergenceSummaryV2"; private static final String testerCertificateField = "testerCertificate"; private static final String isDryRunField = "isDryRun"; + private static final String reasonField = "reason"; Run runFromSlime(Slime slime) { return runFromSlime(slime.get()); @@ -150,7 +151,8 @@ class RunSerializer { Optional.of(runObject.field(testerCertificateField)) .filter(Inspector::valid) .map(certificate -> X509CertificateUtils.fromPem(certificate.asString())), - runObject.field(isDryRunField).valid() && runObject.field(isDryRunField).asBool()); + runObject.field(isDryRunField).valid() && runObject.field(isDryRunField).asBool(), + SlimeUtils.optionalString(runObject.field(reasonField))); } private Versions versionsFromSlime(Inspector versionsObject) { @@ -257,6 +259,7 @@ class RunSerializer { versionsObject.setObject(sourceField)); }); runObject.setBool(isDryRunField, run.isDryRun()); + run.reason().ifPresent(reason -> runObject.setString(reasonField, reason)); } private void toSlime(Version platformVersion, ApplicationVersion applicationVersion, Cursor versionsObject) { 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 576e5fe1c8a..9b0bbaa5082 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 @@ -1126,7 +1126,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { DeploymentId deployment = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); Principal principal = requireUserPrincipal(request); SupportAccess disallowed = controller.supportAccess().disallow(deployment, principal.getName()); - controller.applications().deploymentTrigger().reTriggerOrAddToQueue(deployment); + controller.applications().deploymentTrigger().reTriggerOrAddToQueue(deployment, "re-triggered to disallow support access, by " + request.getJDiscRequest().getUserPrincipal().getName()); return new SlimeJsonResponse(SupportAccessSerializer.serializeCurrentState(disallowed, controller.clock().instant())); } @@ -1161,9 +1161,9 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { boolean reTrigger = requestObject.field("reTrigger").asBool(); String triggered = reTrigger ? controller.applications().deploymentTrigger() - .reTrigger(id, type).type().jobName() + .reTrigger(id, type, "re-triggered by " + request.getJDiscRequest().getUserPrincipal().getName()).type().jobName() : controller.applications().deploymentTrigger() - .forceTrigger(id, type, request.getJDiscRequest().getUserPrincipal().getName(), requireTests) + .forceTrigger(id, type, "triggered by " + request.getJDiscRequest().getUserPrincipal().getName(), requireTests) .stream().map(job -> job.type().jobName()).collect(joining(", ")); return new MessageResponse(triggered.isEmpty() ? "Job " + type.jobName() + " for " + id + " not triggered" : "Triggered " + triggered + " for " + id); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java index 951a22c5ee7..346e61c907c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java @@ -124,7 +124,7 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { SupportAccess supportAccess = controller.supportAccess().registerGrant(deployment, principal.getName(), certificate); // Trigger deployment to include operator cert - Optional jobId = controller.applications().deploymentTrigger().reTriggerOrAddToQueue(deployment); + Optional jobId = controller.applications().deploymentTrigger().reTriggerOrAddToQueue(deployment, "re-triggered to grant access, by " + request.getJDiscRequest().getUserPrincipal().getName()); return new MessageResponse( jobId.map(id -> Text.format("Operator %s granted access and job %s triggered", principal.getName(), id.type().jobName())) .orElseGet(() -> Text.format("Operator %s granted access and job trigger queued", principal.getName()))); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 805d727d355..6035cd274bf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -1988,9 +1988,9 @@ public class DeploymentTriggerTest { app.submit(); tester.triggerJobs(); - tester.deploymentTrigger().reTrigger(app.instanceId(), productionUsEast3); - tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3"))); - tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3"))); + tester.deploymentTrigger().reTrigger(app.instanceId(), productionUsEast3, null); + tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3")), null); + tester.deploymentTrigger().reTriggerOrAddToQueue(app.deploymentIdIn(ZoneId.from("prod", "us-east-3")), null); List retriggerEntries = tester.controller().curator().readRetriggerEntries(); Assert.assertEquals(1, retriggerEntries.size()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java index d55b5cea4ee..5f443759048 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java @@ -72,7 +72,7 @@ public class DeploymentExpirerTest { // Dev application expires when enough time has passed since most recent attempt // Redeployments done by DeploymentUpgrader do not affect this tester.clock().advance(Duration.ofDays(12).plus(Duration.ofSeconds(1))); - tester.jobs().start(devApp.instanceId(), JobType.devUsEast1, lastRun.versions(), true); + tester.jobs().start(devApp.instanceId(), JobType.devUsEast1, lastRun.versions(), true, Optional.of("upgrade")); expirer.maintain(); assertEquals(0, permanentDeployments(devApp.instance())); assertEquals(1, permanentDeployments(prodApp.instance())); 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 a339cde2b89..71a6fcb1d84 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 @@ -5,6 +5,7 @@ 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.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.pkg.ApplicationPackage; @@ -95,13 +96,13 @@ public class JobRunnerTest { ApplicationId id = appId.defaultInstance(); jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]); - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); try { - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); fail("Job is already running, so this should not be allowed!"); } catch (IllegalArgumentException ignored) { } - jobs.start(id, stagingTest, versions); + start(jobs, id, stagingTest); assertTrue(jobs.last(id, systemTest).get().stepStatuses().values().stream().allMatch(unfinished::equals)); assertFalse(jobs.last(id, systemTest).get().hasEnded()); @@ -127,7 +128,7 @@ public class JobRunnerTest { jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]); Supplier run = () -> jobs.last(id, systemTest).get(); - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); RunId first = run.get().id(); Map steps = run.get().stepStatuses(); @@ -188,7 +189,7 @@ public class JobRunnerTest { assertSame(aborted, run.get().status()); // A new run is attempted. - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); assertEquals(first.number() + 1, run.get().id().number()); // Run fails on tester deployment -- remaining run-always steps succeed, and the run finishes. @@ -206,7 +207,7 @@ public class JobRunnerTest { assertEquals(2, jobs.runs(id, systemTest).size()); // Start a third run, then unregister and wait for data to be deleted. - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); tester.applications().deleteInstance(id); runner.maintain(); assertFalse(jobs.last(id, systemTest).isPresent()); @@ -234,7 +235,7 @@ public class JobRunnerTest { jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]); RunId runId = new RunId(id, systemTest, 1); - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); runner.maintain(); barrier.await(); try { @@ -272,14 +273,14 @@ public class JobRunnerTest { assertFalse(jobs.lastSuccess(jobId).isPresent()); for (int i = 0; i < jobs.historyLength(); i++) { - jobs.start(instanceId, systemTest, versions); + start(jobs, instanceId, systemTest); runner.run(); } assertEquals(64, jobs.runs(jobId).size()); assertTrue(jobs.details(new RunId(instanceId, systemTest, 1)).isPresent()); - jobs.start(instanceId, systemTest, versions); + start(jobs, instanceId, systemTest); runner.run(); assertEquals(64, jobs.runs(jobId).size()); @@ -291,7 +292,7 @@ public class JobRunnerTest { // Make all but the oldest of the 54 jobs a failure. for (int i = 0; i < jobs.historyLength() - 1; i++) { - jobs.start(instanceId, systemTest, versions); + start(jobs, instanceId, systemTest); failureRunner.run(); } assertEquals(64, jobs.runs(jobId).size()); @@ -300,7 +301,7 @@ public class JobRunnerTest { assertEquals(66, jobs.firstFailing(jobId).get().id().number()); // Oldest success is kept even though it would normally overflow. - jobs.start(instanceId, systemTest, versions); + start(jobs, instanceId, systemTest); failureRunner.run(); assertEquals(65, jobs.runs(jobId).size()); assertEquals(65, jobs.runs(jobId).keySet().iterator().next().number()); @@ -308,7 +309,7 @@ public class JobRunnerTest { assertEquals(66, jobs.firstFailing(jobId).get().id().number()); // First failure after the last success is also kept. - jobs.start(instanceId, systemTest, versions); + start(jobs, instanceId, systemTest); failureRunner.run(); assertEquals(66, jobs.runs(jobId).size()); assertEquals(65, jobs.runs(jobId).keySet().iterator().next().number()); @@ -317,7 +318,7 @@ public class JobRunnerTest { assertEquals(66, jobs.firstFailing(jobId).get().id().number()); // No other jobs are kept with repeated failures. - jobs.start(instanceId, systemTest, versions); + start(jobs, instanceId, systemTest); failureRunner.run(); assertEquals(66, jobs.runs(jobId).size()); assertEquals(65, jobs.runs(jobId).keySet().iterator().next().number()); @@ -327,7 +328,7 @@ public class JobRunnerTest { assertEquals(66, jobs.firstFailing(jobId).get().id().number()); // history length returns to 256 when a new success is recorded. - jobs.start(instanceId, systemTest, versions); + start(jobs, instanceId, systemTest); runner.run(); assertEquals(64, jobs.runs(jobId).size()); assertEquals(69, jobs.runs(jobId).keySet().iterator().next().number()); @@ -365,7 +366,7 @@ public class JobRunnerTest { ApplicationId id = appId.defaultInstance(); jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]); - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); tester.clock().advance(JobRunner.jobTimeout.plus(Duration.ofSeconds(1))); runner.run(); assertSame(aborted, jobs.last(id, systemTest).get().status()); @@ -388,7 +389,7 @@ public class JobRunnerTest { for (RunStatus status : RunStatus.values()) { if (status == success || status == reset) continue; // Status not used for steps. outcomes.put(deployTester, status); - jobs.start(id, systemTest, versions); + start(jobs, id, systemTest); runner.run(); jobs.finish(jobs.last(id, systemTest).get().id()); } @@ -410,6 +411,10 @@ public class JobRunnerTest { assertEquals(1, metric.getMetric(context::equals, JobMetrics.testFailure).get().intValue()); } + private void start(JobController jobs, ApplicationId id, JobType type) { + jobs.start(id, type, versions, false, Optional.empty()); + } + public static ExecutorService inThreadExecutor() { return new AbstractExecutorService() { final AtomicBoolean shutDown = new AtomicBoolean(false); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java index 0c1b3b39b65..ccb2b6ebb74 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainerTest.java @@ -38,10 +38,10 @@ public class RetriggerMaintainerTest { devApp.completeRollout(); // Trigger a run (to simulate a running job) - tester.deploymentTrigger().reTrigger(applicationId, JobType.devUsEast1); + tester.deploymentTrigger().reTrigger(applicationId, JobType.devUsEast1, null); // Add a job to the queue - tester.deploymentTrigger().reTriggerOrAddToQueue(devApp.deploymentIdIn(ZoneId.from("dev", "us-east-1"))); + tester.deploymentTrigger().reTriggerOrAddToQueue(devApp.deploymentIdIn(ZoneId.from("dev", "us-east-1")), null); // Should be 1 entry in the queue: List retriggerEntries = tester.controller().curator().readRetriggerEntries(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java index f4f52b20325..237d54db20c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java @@ -99,6 +99,7 @@ public class RunSerializerTest { assertEquals(applicationVersion.compileVersion(), run.versions().targetApplication().compileVersion()); assertEquals("f00bad", run.versions().targetApplication().commit().get()); assertEquals("https://github.com/user/repo/tree/f00bad", run.versions().targetApplication().sourceUrl().get()); + assertEquals("because", run.reason().get()); assertEquals(new Version(1, 2, 2), run.versions().sourcePlatform().get()); assertEquals(ApplicationVersion.from(new SourceRevision("git@github.com:user/repo.git", "master", @@ -153,8 +154,9 @@ public class RunSerializerTest { assertEquals(run.versions(), phoenix.versions()); assertEquals(run.steps(), phoenix.steps()); assertEquals(run.isDryRun(), phoenix.isDryRun()); + assertEquals(run.reason(), phoenix.reason()); - Run initial = Run.initial(id, run.versions(), run.isRedeployment(), run.start(), JobProfile.production); + Run initial = Run.initial(id, run.versions(), run.isRedeployment(), run.start(), JobProfile.production, Optional.empty()); assertEquals(initial, serializer.runFromSlime(serializer.toSlime(initial))); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json index 54cde2bacef..85881fbfdbc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json @@ -53,6 +53,7 @@ "build": 122, "deployedDirectly": false } - } + }, + "reason": "because" } ] \ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java index b830a4ce512..cf6453235d3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java @@ -49,7 +49,7 @@ public class BadgeApiTest extends ControllerContainerTest { for (int i = 0; i < 31; i++) application.failDeployment(JobType.productionUsWest1); application.triggerJobs(); - tester.controller().applications().deploymentTrigger().reTrigger(application.instanceId(), JobType.testEuWest1); + tester.controller().applications().deploymentTrigger().reTrigger(application.instanceId(), JobType.testEuWest1, "reason"); tester.assertResponse(authenticatedRequest("http://localhost:8080/badge/v1/tenant/application/default"), Files.readString(Paths.get(responseFiles + "overview.svg")), 200); -- cgit v1.2.3