Let only successful run expire due to age

@mpolden please review.

This fixes a problem where a NoSuchElementException was thrown when the
only run was a success, this expired, and a new run failed.
The solution is simply to throw away the success in this case, as we
have no logs for it anyway

2 files changed, 24 insertions, 3 deletions

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 d8d9431dc22..be189004f6d 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
@@ -73,8 +73,8 @@ import static java.util.stream.Collectors.toUnmodifiableMap;
  */
 public class JobController {
 
-    private static final int historyLength = 64;
-    private static final Duration maxHistoryAge = Duration.ofDays(60);
+    public static final int historyLength = 64;
+    public static final Duration maxHistoryAge = Duration.ofDays(60);
 
     private final Controller controller;
     private final CuratorDb curator;
@@ -356,7 +356,9 @@ public class JobController {
                      old = oldEntries.next()) {
 
                     // Make sure we keep the last success and the first failing
-                    if (successes == 1 && old.getValue().status() == RunStatus.success) {
+                    if (     successes == 1
+                        &&   old.getValue().status() == RunStatus.success
+                        && ! old.getValue().start().isBefore(controller.clock().instant().minus(maxHistoryAge))) {
                         oldEntries.next();
                         continue;
                     }
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 b3a6ef53d2b..ce4ae7af6b4 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
@@ -338,6 +338,25 @@ public class JobRunnerTest {
     }
 
     @Test
+    public void onlySuccessfulRunExpiresThenAnotherFails() {
+        DeploymentTester tester = new DeploymentTester();
+        JobController jobs = tester.controller().jobController();
+        var app = tester.newDeploymentContext().submit();
+        JobId jobId = new JobId(app.instanceId(), systemTest);
+        assertFalse(jobs.lastSuccess(jobId).isPresent());
+
+        app.runJob(systemTest);
+        assertTrue(jobs.lastSuccess(jobId).isPresent());
+        assertEquals(1, jobs.runs(jobId).size());
+
+        tester.clock().advance(JobController.maxHistoryAge.plusSeconds(1));
+        app.submit();
+        app.failDeployment(systemTest);
+        assertFalse(jobs.lastSuccess(jobId).isPresent());
+        assertEquals(1, jobs.runs(jobId).size());
+    }
+
+    @Test
     public void timeout() {
         DeploymentTester tester = new DeploymentTester();
         JobController jobs = tester.controller().jobController();