diff options
author | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2018-04-12 17:23:28 +0200 |
---|---|---|
committer | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2018-04-13 12:08:10 +0200 |
commit | a4073d0e1f5078cdeebd2c1f23134da20ca62dec (patch) | |
tree | fb9b3d494b03216dd4d8fe5dbefb563940ff1861 | |
parent | 7c1ee8beccb08d4f0cd42b74d5a5bab8480e0f25 (diff) |
Remove projectId when project is gone or we have no access
5 files changed, 21 insertions, 15 deletions
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 b7e72987573..505775aef12 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 @@ -54,7 +54,7 @@ public class LockedApplication extends Application { builder.outstandingChange, builder.ownershipIssueId, builder.metrics, builder.rotation); } - public LockedApplication withProjectId(long projectId) { + public LockedApplication withProjectId(Optional<Long> projectId) { return new LockedApplication(new Builder(this).with(deploymentJobs().withProjectId(projectId))); } 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 04e51cdecb6..f31360451cc 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 @@ -74,8 +74,8 @@ public class DeploymentJobs { return new DeploymentJobs(projectId, status, issueId); } - public DeploymentJobs withProjectId(long projectId) { - return new DeploymentJobs(Optional.of(projectId), status, issueId); + public DeploymentJobs withProjectId(Optional<Long> projectId) { + return new DeploymentJobs(projectId, status, issueId); } public DeploymentJobs with(IssueId issueId) { 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 2d02df55fe1..55f16999cce 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 @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -106,7 +107,7 @@ public class DeploymentTrigger { ApplicationVersion applicationVersion = report.sourceRevision().map(sr -> ApplicationVersion.from(sr, report.buildNumber())) .orElse(ApplicationVersion.unknown); application = application.withJobCompletion(report, applicationVersion, clock.instant(), controller); - application = application.withProjectId(report.projectId()); + application = application.withProjectId(Optional.of(report.projectId())); if (report.jobType() == JobType.component && report.success()) { if (acceptNewApplicationVersion(application)) @@ -150,18 +151,22 @@ public class DeploymentTrigger { * Triggers the given job for the given application. */ public boolean trigger(Job job) { - log.info(String.format("Attempting to trigger %s for %s, deploying %s: %s", job.jobType, job.id, job.change, job.reason)); + log.log(LogLevel.INFO, String.format("Attempting to trigger %s for %s, deploying %s: %s", job.jobType, job.id, job.change, job.reason)); BuildService.BuildJob buildJob = new BuildService.BuildJob(job.projectId, job.jobType.jobName()); - if (buildService.trigger(buildJob)) { - applications().lockOrThrow(job.id, application -> applications().store(application.withJobTriggering( - job.jobType, new JobStatus.JobRun(-1, job.platformVersion, job.applicationVersion, job.reason, clock.instant())))); - return true; + try { + if (buildService.trigger(buildJob)) { + applications().lockOrThrow(job.id, application -> applications().store(application.withJobTriggering( + job.jobType, new JobStatus.JobRun(-1, job.platformVersion, job.applicationVersion, job.reason, clock.instant())))); + return true; + } + } + catch (NoSuchElementException | IllegalArgumentException e) { + applications().lockOrThrow(job.id, application -> applications().store(application.withProjectId(Optional.empty()))); + log.log(LogLevel.WARNING, "Removing projectId " + job.projectId + " from " + job.id + + " because of exception trying to trigger " + buildJob + ": " + e.getMessage()); } - // TODO jvenstad: On 404: set empty projectId (and send ticket?). Throw and catch NoSuchElementException. - // TODO jvensatd: On 401, 403 with crumb issues: refresh auth. Handle in client. - // TODO jvenstad: On 403 with missing collaborator status: send deployment issue ticket? Throw and catch IllegalArumentException. - log.log(LogLevel.WARNING, "Failed to trigger " + buildJob + " for " + job.id); + log.log(LogLevel.INFO, "Failed to trigger " + buildJob + " for " + job.id); return false; } 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 8793ce3a7e4..27b77cfc901 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 @@ -221,7 +221,7 @@ public final class ControllerTester { ApplicationId applicationId = ApplicationId.from(tenant.value(), applicationName, instanceName); controller().applications().createApplication(applicationId, Optional.of(TestIdentities.userNToken)); controller().applications().lockOrThrow(applicationId, lockedApplication -> - controller().applications().store(lockedApplication.withProjectId(projectId))); + controller().applications().store(lockedApplication.withProjectId(Optional.of(projectId)))); return controller().applications().require(applicationId); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java index 2911b9ab72f..42f291d5f96 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java @@ -10,6 +10,7 @@ import org.junit.Test; import java.io.File; import java.nio.charset.StandardCharsets; +import java.util.Optional; /** * @author bratseth @@ -38,7 +39,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { Application app = tester.createApplication(); tester.controller().applications().lockOrThrow(app.id(), application -> - tester.controller().applications().store(application.withProjectId(1))); + tester.controller().applications().store(application.withProjectId(Optional.of(1L)))); // Unknown application assertResponse(new Request("http://localhost:8080/screwdriver/v1/trigger/tenant/foo/application/bar", |