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",