Sort deployments and jobs in application API

6 files changed, 369 insertions, 11 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
index 82e6ca919e1..48ebbc9f972 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
@@ -1,9 +1,13 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.yahoo.config.application.api.DeploymentSpec;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.Change;
+import com.yahoo.vespa.hosted.controller.application.Deployment;
+import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 
@@ -11,10 +15,15 @@ import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.logging.Logger;
+import java.util.stream.Collector;
 import java.util.stream.Collectors;
 
 import static java.util.stream.Collectors.collectingAndThen;
@@ -104,6 +113,27 @@ public class DeploymentOrder {
                 .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
     }
 
+    /** Returns job status sorted according to deployment spec */
+    public Map<JobType, JobStatus> sortBy(DeploymentSpec deploymentSpec, Map<JobType, JobStatus> jobStatus) {
+        List<DeploymentJobs.JobType> jobs = jobsFrom(deploymentSpec);
+        return jobStatus.entrySet().stream()
+                .sorted(Comparator.comparingInt(kv -> jobs.indexOf(kv.getKey())))
+                .collect(Collectors.collectingAndThen(toLinkedMap(Map.Entry::getKey, Map.Entry::getValue),
+                                                      Collections::unmodifiableMap));
+    }
+
+    /** Returns deployments sorted according to declared zones */
+    public Map<Zone, Deployment> sortBy(List<DeploymentSpec.DeclaredZone> zones, Map<Zone, Deployment> deployments) {
+        List<Zone> productionZones = zones.stream()
+                .filter(z -> z.environment() == Environment.prod && z.region().isPresent())
+                .map(z -> new Zone(z.environment(), z.region().get()))
+                .collect(Collectors.toList());
+        return deployments.entrySet().stream()
+                .sorted(Comparator.comparingInt(kv -> productionZones.indexOf(kv.getKey())))
+                .collect(Collectors.collectingAndThen(toLinkedMap(Map.Entry::getKey, Map.Entry::getValue),
+                                                      Collections::unmodifiableMap));
+    }
+
     /** Returns jobs for the given step */
     private List<JobType> jobsFrom(DeploymentSpec.Step step) {
         return step.zones().stream()
@@ -166,4 +196,13 @@ public class DeploymentOrder {
         return totalDelay;
     }
 
+    private static <T, K, U> Collector<T, ?, Map<K,U>> toLinkedMap(Function<? super T, ? extends K> keyMapper,
+                                                                   Function<? super T, ? extends U> valueMapper) {
+        return Collectors.toMap(keyMapper, valueMapper,
+                                (u, v) -> {
+                                    throw new IllegalStateException(String.format("Duplicate key %s", u));
+                                },
+                                LinkedHashMap::new);
+    }
+
 }
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 9b2158e161e..423d8c674df 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
@@ -308,4 +308,6 @@ public class DeploymentTrigger {
 
     public BuildSystem buildSystem() { return buildSystem; }
 
+    public DeploymentOrder deploymentOrder() { return order; }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 142b6d6018b..c2777a78f32 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -62,6 +62,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.ApplicationRevision;
 import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
+import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.application.SourceRevision;
 import com.yahoo.vespa.hosted.controller.common.NotFoundCheckedException;
@@ -97,6 +98,7 @@ import java.util.logging.Level;
  * on hosted Vespa.
  * 
  * @author bratseth
+ * @author mpolden
  */
 @SuppressWarnings("unused") // created by injection
 public class ApplicationApiHandler extends LoggingRequestHandler {
@@ -123,7 +125,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                 case PUT: return handlePUT(request);
                 case POST: return handlePOST(request);
                 case DELETE: return handleDELETE(request);
-                case OPTIONS: return handleOPTIONS(request);
+                case OPTIONS: return handleOPTIONS();
                 default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported");
             }
         }
@@ -152,7 +154,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (path.matches("/application/v4/tenant")) return tenants(request);
         if (path.matches("/application/v4/tenant-pipeline")) return tenantPipelines();
         if (path.matches("/application/v4/athensDomain")) return athensDomains(request);
-        if (path.matches("/application/v4/property")) return properties(request);
+        if (path.matches("/application/v4/property")) return properties();
         if (path.matches("/application/v4/cookiefreshness")) return cookieFreshness(request);
         if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application")) return applications(path.get("tenant"), request);
@@ -202,7 +204,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
     
-    private HttpResponse handleOPTIONS(HttpRequest request) {
+    private HttpResponse handleOPTIONS() {
         // We implement this to avoid redirect loops on OPTIONS requests from browsers, but do not really bother
         // spelling out the methods supported at each path, which we should
         EmptyJsonResponse response = new EmptyJsonResponse();
@@ -211,8 +213,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
     
     private HttpResponse root(HttpRequest request) {
-        return new ResourceResponse(request,
-                                    "user", "tenant", "tenant-pipeline", "athensDomain", "property", "cookiefreshness");
+        return new ResourceResponse(request, "user", "tenant", "tenant-pipeline", "athensDomain",
+                                    "property", "cookiefreshness");
     }
     
     private HttpResponse authenticatedUser(HttpRequest request) {
@@ -270,7 +272,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
 
-    private HttpResponse properties(HttpRequest request) {
+    private HttpResponse properties() {
         Slime slime = new Slime();
         Cursor response = slime.setObject();
         Cursor array = response.setArray("properties");
@@ -324,9 +326,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                 toSlime(((Change.ApplicationChange)application.deploying().get()).revision().get(), deployingObject.setObject("revision"));
         }
 
-        // Deployment jobs 
+        // Jobs sorted according to deployment spec
+        Map<DeploymentJobs.JobType, JobStatus> jobStatus = controller.applications().deploymentTrigger()
+                .deploymentOrder()
+                .sortBy(application.deploymentSpec(), application.deploymentJobs().jobStatus());
+
         Cursor deploymentsArray = response.setArray("deploymentJobs");
-        for (JobStatus job : application.deploymentJobs().jobStatus().values()) {
+        for (JobStatus job : jobStatus.values()) {
             Cursor jobObject = deploymentsArray.addObject();            
             jobObject.setString("type", job.type().id());
             jobObject.setBool("success", job.isSuccess());
@@ -348,9 +354,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         for (URI rotation : rotations)
             globalRotationsArray.addString(rotation.toString());
 
-        // Deployments
+        // Deployments sorted according to deployment spec
+        Map<Zone, Deployment> deployments = controller.applications().deploymentTrigger()
+                .deploymentOrder()
+                .sortBy(application.deploymentSpec().zones(), application.deployments());
         Cursor instancesArray = response.setArray("instances");
-        for (Deployment deployment : application.deployments().values()) {
+        for (Deployment deployment : deployments.values()) {
             Cursor deploymentObject = instancesArray.addObject();
             deploymentObject.setString("environment", deployment.zone().environment().value());
             deploymentObject.setString("region", deployment.zone().region().value());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 18073ef06f3..af05c7bbb58 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -309,6 +309,74 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       athensScrewdriverDomain, "screwdriveruser1"),
                               new File("deploy-result.json"));
     }
+
+    @Test
+    public void testSortsDeploymentsAndJobs() throws Exception {
+        // Setup
+        ContainerControllerTester controllerTester = new ContainerControllerTester(container, responseFiles);
+        ContainerTester tester = controllerTester.containerTester();
+        tester.updateSystemVersion();
+        addTenantAthensDomain(athensUserDomain, "mytenant");
+        addScrewdriverUserToDomain("screwdriveruser1", "domain1");
+
+        // Create tenant
+        tester.assertResponse(request("/application/v4/tenant/tenant1",
+                                      "{\"athensDomain\":\"domain1\", \"property\":\"property1\"}",
+                                      Request.Method.POST),
+                              new File("tenant-without-applications.json"));
+
+        // Create application
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1",
+                                      "",
+                                      Request.Method.POST),
+                              new File("application-reference.json"));
+
+        // Deploy
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .region("us-east-3")
+                .build();
+        ApplicationId id = ApplicationId.from("tenant1", "application1", "default");
+        long projectId = 1;
+        HttpEntity deployData = createApplicationDeployData(applicationPackage, Optional.of(projectId));
+
+        startAndTestChange(controllerTester, id, projectId, deployData);
+
+        // us-east-3
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-east-3/instance/default/deploy",
+                                      deployData,
+                                      Request.Method.POST,
+                                      athensScrewdriverDomain, "screwdriveruser1"),
+                              new File("deploy-result.json"));
+        controllerTester.notifyJobCompletion(id, projectId, true, DeploymentJobs.JobType.productionUsEast3);
+
+        // New zone is added before us-east-3
+        applicationPackage = new ApplicationPackageBuilder()
+                // These decides the ordering of deploymentJobs and instances in the response
+                .region("us-west-1")
+                .region("us-east-3")
+                .build();
+        deployData = createApplicationDeployData(applicationPackage, Optional.of(projectId));
+        startAndTestChange(controllerTester, id, projectId, deployData);
+
+        // us-west-1
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/deploy",
+                                      deployData,
+                                      Request.Method.POST,
+                                      athensScrewdriverDomain, "screwdriveruser1"),
+                              new File("deploy-result.json"));
+        controllerTester.notifyJobCompletion(id, projectId, true, DeploymentJobs.JobType.productionUsWest1);
+
+        // us-east-3
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-east-3/instance/default/deploy",
+                                      deployData,
+                                      Request.Method.POST,
+                                      athensScrewdriverDomain, "screwdriveruser1"),
+                              new File("deploy-result.json"));
+        controllerTester.notifyJobCompletion(id, projectId, true, DeploymentJobs.JobType.productionUsEast3);
+
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", "", Request.Method.GET),
+                              new File("application-without-change-multiple-deployments.json"));
+    }
     
     @Test
     public void testErrorResponses() throws Exception {
@@ -643,5 +711,41 @@ public class ApplicationApiTest extends ControllerContainerTest {
         mock.setApplicationCost(new ApplicationId.Builder().tenant(tenant).applicationName(application).instanceName(instance).build(),
                                 new ApplicationCost("prod.us-west-1", tenant, application + "." + instance, 37, 1.0f, 0.0f, clusterCosts));
     }
+
+    private void startAndTestChange(ContainerControllerTester controllerTester, ApplicationId application, long projectId,
+                                    HttpEntity deployData) throws IOException {
+        ContainerTester tester = controllerTester.containerTester();
+
+        // Trigger application change
+        controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.component);
+
+        // system-test
+        String testPath = String.format("/application/v4/tenant/%s/application/%s/environment/test/region/us-east-1/instance/default",
+                                        application.tenant().value(), application.application().value());
+        tester.assertResponse(request(testPath,
+                                      deployData,
+                                      Request.Method.POST,
+                                      athensScrewdriverDomain, "screwdriveruser1"),
+                              new File("deploy-result.json"));
+        tester.assertResponse(request(testPath,
+                                      "",
+                                      Request.Method.DELETE),
+                              "Deactivated " + testPath.replaceFirst("/application/v4/", ""));
+        controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.systemTest);
+
+        // staging
+        String stagingPath = String.format("/application/v4/tenant/%s/application/%s/environment/staging/region/us-east-3/instance/default",
+                                           application.tenant().value(), application.application().value());
+        tester.assertResponse(request(stagingPath,
+                                      deployData,
+                                      Request.Method.POST,
+                                      athensScrewdriverDomain, "screwdriveruser1"),
+                              new File("deploy-result.json"));
+        tester.assertResponse(request(stagingPath,
+                                      "",
+                                      Request.Method.DELETE),
+                              "Deactivated " + stagingPath.replaceFirst("/application/v4/", ""));
+        controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.stagingTest);
+    }
     
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
new file mode 100644
index 00000000000..a82bdaa454a
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
@@ -0,0 +1,204 @@
+{
+  "deploymentJobs": [
+    {
+      "type": "component",
+      "success": true,
+      "lastCompleted": {
+        "version": "(ignore)",
+        "at": "(ignore)"
+      },
+      "lastSuccess": {
+        "version": "(ignore)",
+        "at": "(ignore)"
+      }
+    },
+    {
+      "type": "system-test",
+      "success": true,
+      "lastTriggered": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastCompleted": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastSuccess": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      }
+    },
+    {
+      "type": "staging-test",
+      "success": true,
+      "lastTriggered": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastCompleted": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastSuccess": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      }
+    },
+    {
+      "type": "production-us-west-1",
+      "success": true,
+      "lastTriggered": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastCompleted": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastSuccess": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      }
+    },
+    {
+      "type": "production-us-east-3",
+      "success": true,
+      "lastTriggered": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastCompleted": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      },
+      "lastSuccess": {
+        "version": "(ignore)",
+        "revision": {
+          "hash": "(ignore)",
+          "source": {
+            "gitRepository": "repository1",
+            "gitBranch": "master",
+            "gitCommit": "commit1"
+          }
+        },
+        "at": "(ignore)"
+      }
+    }
+  ],
+  "compileVersion": "(ignore)",
+  "globalRotations": [
+    "http://fake-global-rotation-tenant1.application1"
+  ],
+  "instances": [
+    {
+      "environment": "prod",
+      "region": "us-west-1",
+      "instance": "default",
+      "bcpStatus": {
+        "rotationStatus": "IN"
+      },
+      "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default"
+    },
+    {
+      "environment": "prod",
+      "region": "us-east-3",
+      "instance": "default",
+      "bcpStatus": {
+        "rotationStatus": "UNKNOWN"
+      },
+      "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-east-3/instance/default"
+    }
+  ],
+  "metrics": {
+    "queryServiceQuality": 0.5,
+    "writeServiceQuality": 0.7
+  }
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json
index d1ae5253a00..06b48064b94 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json
@@ -1,6 +1,6 @@
 {
   "revisionId":"(ignore)",
-  "applicationZipSize":412,
+  "applicationZipSize":"(ignore)",
   "prepareMessages":[],
   "configChangeActions":{
     "restart":[],