Merge pull request #3540 from vespa-engine/mpolden/handle-missing-project-id

Ignore applications missing project ID in build system

4 files changed, 61 insertions, 6 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java
index 41adb4abe6a..0d9330ed8ea 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystem.java
@@ -12,6 +12,8 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Deque;
 import java.util.List;
+import java.util.Optional;
+import java.util.logging.Logger;
 
 /**
  * Stores a queue for each type of job, and offers jobs from each of these to a periodic
@@ -22,6 +24,8 @@ import java.util.List;
  */
 public class PolledBuildSystem implements BuildSystem {
 
+    private static final Logger log = Logger.getLogger(PolledBuildSystem.class.getName());
+
     private final Controller controller;
 
     private final CuratorDb curator;
@@ -77,7 +81,14 @@ public class PolledBuildSystem implements BuildSystem {
                 Deque<ApplicationId> queue = curator.readJobQueue(jobType);
                 for (ApplicationId a : queue) {
                     ApplicationId application = removeFromQueue ? queue.poll() : a;
-                    jobsToRun.add(new BuildJob(projectIdFor(application), jobType.id()));
+
+                    Optional<Long> projectId = projectId(application);
+                    if (projectId.isPresent()) {
+                        jobsToRun.add(new BuildJob(projectId.get(), jobType.id()));
+                    } else {
+                        log.warning("Not queuing " + jobType.id() + " for " + application.toShortString() +
+                                    " because project ID is missing");
+                    }
 
                     // Return only one job at a time for capacity constrained queues
                     if (removeFromQueue && isCapacityConstrained(jobType)) break;
@@ -89,8 +100,8 @@ public class PolledBuildSystem implements BuildSystem {
         }
     }
 
-    private Long projectIdFor(ApplicationId applicationId) {
-        return controller.applications().require(applicationId).deploymentJobs().projectId().get();
+    private Optional<Long> projectId(ApplicationId applicationId) {
+        return controller.applications().require(applicationId).deploymentJobs().projectId();
     }
 
     private static boolean isCapacityConstrained(JobType jobType) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java
index 2c048bfa3ce..43c1755af34 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployer.java
@@ -50,7 +50,7 @@ public class FailureRedeployer extends Maintainer {
     }
 
     private void retryStuckJobs(List<Application> applications) {
-        Instant maxAge = controller().clock().instant().minus(jobTimeout);
+        Instant startOfGracePeriod = controller().clock().instant().minus(jobTimeout);
         for (Application application : applications) {
             Optional<JobStatus> job = oldestRunningJob(application);
             if (!job.isPresent()) {
@@ -60,7 +60,7 @@ public class FailureRedeployer extends Maintainer {
             if (!job.get().type().zone(controller().system()).isPresent()) {
                 continue;
             }
-            if (job.get().lastTriggered().get().at().isBefore(maxAge)) {
+            if (job.get().lastTriggered().get().at().isBefore(startOfGracePeriod)) {
                 triggerFailing(application, "Job " + job.get().type().id() +
                         " has been running for more than " + jobTimeout);
             }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java
index fd7a3605766..f5953889f31 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java
@@ -299,7 +299,6 @@ public class FailureRedeployerTest {
     @Test
     public void ignoresPullRequestInstances() throws Exception {
         DeploymentTester tester = new DeploymentTester();
-        tester.controllerTester().getZoneRegistryMock().setSystem(SystemName.cd);
 
         // Current system version, matches version in test data
         Version version = Version.fromString("6.42.1");
@@ -322,4 +321,29 @@ public class FailureRedeployerTest {
         assertTrue("No jobs scheduled", tester.buildSystem().jobs().isEmpty());
     }
 
+    @Test
+    public void applicationWithoutProjectIdIsNotTriggered() throws Exception {
+        DeploymentTester tester = new DeploymentTester();
+
+        // Current system version, matches version in test data
+        Version version = Version.fromString("6.42.1");
+        tester.configServerClientMock().setDefaultConfigServerVersion(version);
+        tester.updateVersionStatus(version);
+        assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber());
+
+        // Load test data data
+        ApplicationSerializer serializer = new ApplicationSerializer();
+        byte[] json = Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json"));
+        Slime slime = SlimeUtils.jsonToSlime(json);
+        Application application = serializer.fromSlime(slime);
+
+        try (Lock lock = tester.controller().applications().lock(application.id())) {
+            tester.controller().applications().store(application, lock);
+        }
+
+        // Failure redeployer does not restart deployment
+        tester.failureRedeployer().maintain();
+        assertTrue("No jobs scheduled", tester.buildSystem().jobs().isEmpty());
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json
new file mode 100644
index 00000000000..912d1c2f4dc
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/application-without-project-id.json
@@ -0,0 +1,20 @@
+{
+  "id": "tenant1:app1:default",
+  "deploymentSpecField": "<deployment version='1.0'>\n  <test />\n  <staging />\n  <prod>\n     <region active=\"true\">cd-us-central-1</region>\n     <region active=\"true\">cd-us-central-2</region>\n  </prod>\n</deployment>\n",
+  "validationOverrides": "<deployment version='1.0'/>",
+  "deployments": [],
+  "deploymentJobs": {
+    "jobStatus": [
+      {
+        "jobType": "system-test",
+        "lastTriggered": {
+          "version": "6.42.1",
+          "upgrade": false,
+          "at": 1506330088050
+        }
+      }
+    ],
+    "selfTriggering": false
+  },
+  "outstandingChangeField": false
+}