Merge pull request #13122 from vespa-engine/jonmv/correctly-compute-pending-jobs

Jonmv/correctly compute pending jobs

4 files changed, 27 insertions, 46 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
index 0419b4038bc..da1bcbdab54 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
@@ -34,6 +34,7 @@ import static com.yahoo.config.provision.Environment.staging;
 import static com.yahoo.config.provision.Environment.test;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest;
+import static java.util.Comparator.comparing;
 import static java.util.Comparator.naturalOrder;
 import static java.util.Objects.requireNonNull;
 import static java.util.function.BinaryOperator.maxBy;
@@ -234,17 +235,16 @@ public class DeploymentStatus {
                     && testJobs.keySet().stream()
                                .noneMatch(test ->    test.type() == testType
                                                   && testJobs.get(test).contains(versions)))
-                    testJobs.merge(anyDeclaredTest(testType).orElse(new JobId(job.application(), testType)), List.of(versions), DeploymentStatus::union);
+                    testJobs.merge(firstDeclaredOrElseImplicitTest(testType), List.of(versions), DeploymentStatus::union);
             });
         }
         return ImmutableMap.copyOf(testJobs);
     }
 
-    private Optional<JobId> anyDeclaredTest(JobType testJob) {
+    private JobId firstDeclaredOrElseImplicitTest(JobType testJob) {
         return application.deploymentSpec().instanceNames().stream()
-                .map(application.id()::instance)
-                .flatMap(id -> declaredTest(id, testJob).stream())
-                .findFirst();
+                          .map(name -> new JobId(application.id().instance(name), testJob))
+                          .min(comparing(id -> !jobSteps.get(id).isDeclared())).orElseThrow();
     }
 
     /** JobId of any declared test of the given type, for the given instance. */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java
index 55fbe1cc231..2cdfbcda571 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java
@@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus;
 import com.yahoo.vespa.hosted.controller.deployment.Run;
+import com.yahoo.vespa.hosted.controller.deployment.Versions;
 import com.yahoo.vespa.hosted.controller.restapi.application.EmptyResponse;
 import com.yahoo.vespa.hosted.controller.versions.DeploymentStatistics;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
@@ -189,11 +190,15 @@ public class DeploymentApiHandler extends LoggingRequestHandler {
                           jobStatus.coolingDownUntil(status.application().require(instance.instance()).change())
                                    .ifPresent(until -> jobObject.setLong("coolingDownUntil", until.toEpochMilli()));
                           if (jobsToRun.containsKey(job)) {
-                              jobObject.setString("pending", jobsToRun.get(job).stream()
-                                                                      .allMatch(versions -> versions.sourcePlatform()
-                                                                                                    .map(versions.targetPlatform()::equals)
-                                                                                                    .orElse(true))
-                                                             ? "application" : "platform");
+                              List<Versions> versionsOnThisPlatform = jobsToRun.get(job).stream()
+                                      .filter(versions -> versions.targetPlatform().equals(statistics.version()))
+                                      .collect(Collectors.toList());
+                              if ( ! versionsOnThisPlatform.isEmpty())
+                                  jobObject.setString("pending", versionsOnThisPlatform.stream()
+                                                                                       .allMatch(versions -> versions.sourcePlatform()
+                                                                                                                     .map(statistics.version()::equals)
+                                                                                                                     .orElse(true))
+                                                                 ? "application" : "platform");
                           }
                       });
                       Cursor allRunsObject = instanceObject.setObject("allRuns");
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
index 6067322f2b6..dd2267be3a4 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
@@ -259,7 +259,7 @@ public class DeploymentContext {
 
     /** Trigger all outstanding jobs, if any */
     public DeploymentContext triggerJobs() {
-        while (tester.controller().applications().deploymentTrigger().triggerReadyJobs() > 0);
+        tester.triggerJobs();
         return this;
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json
index 4f544427c4d..d0d79903da5 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json
@@ -40,17 +40,14 @@
           "jobs": [
             {
               "name": "system-test",
-              "coolingDownUntil": "(ignore)",
-              "pending": "platform"
+              "coolingDownUntil": "(ignore)"
             },
             {
               "name": "staging-test",
-              "coolingDownUntil": "(ignore)",
-              "pending": "platform"
+              "coolingDownUntil": "(ignore)"
             },
             {
-              "name": "production-us-west-1",
-              "pending": "platform"
+              "name": "production-us-west-1"
             }
           ],
           "allRuns": {
@@ -82,16 +79,13 @@
           "upgradePolicy": "default",
           "jobs": [
             {
-              "name": "system-test",
-              "pending": "platform"
+              "name": "system-test"
             },
             {
-              "name": "staging-test",
-              "pending": "platform"
+              "name": "staging-test"
             },
             {
-              "name": "production-us-west-1",
-              "pending": "platform"
+              "name": "production-us-west-1"
             }
           ],
           "allRuns": {
@@ -279,11 +273,11 @@
             {
               "name": "system-test",
               "coolingDownUntil": "(ignore)",
-              "pending": "application"
+              "pending": "platform"
             },
             {
               "name": "staging-test",
-              "pending": "application"
+              "pending": "platform"
             },
             {
               "name": "production-us-west-1",
@@ -336,12 +330,10 @@
           "upgradePolicy": "default",
           "jobs": [
             {
-              "name": "system-test",
-              "pending": "platform"
+              "name": "system-test"
             },
             {
-              "name": "staging-test",
-              "pending": "platform"
+              "name": "staging-test"
             },
             {
               "name": "production-us-west-1",
@@ -355,20 +347,6 @@
                 "start": "(ignore)",
                 "status": "running"
               }
-            },
-            "system-test": {
-              "running": {
-                "number": 1,
-                "start": "(ignore)",
-                "status": "running"
-              }
-            },
-            "staging-test": {
-              "running": {
-                "number": 1,
-                "start": "(ignore)",
-                "status": "running"
-              }
             }
           },
           "upgradeRuns": {
@@ -378,9 +356,7 @@
                 "start": "(ignore)",
                 "status": "running"
               }
-            },
-            "system-test": {},
-            "staging-test": {}
+            }
           }
         }
       ]