Use Application.latestVersion instead of last component job success

6 files changed, 65 insertions, 47 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
index 6ce785630bb..7842c3891a6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
@@ -82,7 +82,7 @@ public class Application {
         this.deployKeys = Objects.requireNonNull(deployKeys, "deployKeys cannot be null");
         this.projectId = Objects.requireNonNull(projectId, "projectId cannot be null");
         this.internal = internal;
-        this.latestVersion = Objects.requireNonNull(latestVersion, "latestVersion cannot be null");
+        this.latestVersion = requireNotUnknown(latestVersion);
         this.instances = ImmutableSortedMap.copyOf(instances.stream().collect(Collectors.toMap(Instance::name, Function.identity())));
     }
 
@@ -199,6 +199,15 @@ public class Application {
     /** Returns the set of deploy keys for this application. */
     public Set<PublicKey> deployKeys() { return deployKeys; }
 
+    private static Optional<ApplicationVersion> requireNotUnknown(Optional<ApplicationVersion> latestVersion) {
+        Objects.requireNonNull(latestVersion, "latestVersion cannot be null");
+        latestVersion.ifPresent(version -> {
+            if (version.isUnknown())
+                throw new IllegalArgumentException("latstVersion cannot be unknown");
+        });
+        return latestVersion;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
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 376048143d9..2fa0d6d4b91 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
@@ -98,6 +98,32 @@ public class DeploymentTrigger {
         return new DeploymentSteps(spec, controller::system);
     }
 
+    public void notifyOfSubmission(TenantAndApplicationId id, ApplicationVersion version, long projectId) {
+        if (applications().getApplication(id).isEmpty()) {
+            log.log(LogLevel.WARNING, "Ignoring submission from project '" + projectId +
+                                      "': Unknown application '" + id + "'");
+            return;
+        }
+
+        applications().lockApplicationOrThrow(id, application -> {
+            if (acceptNewApplicationVersion(application.get())) {
+                application = application.withChange(application.get().change().with(version))
+                                         .withOutstandingChange(Change.empty());
+                if (application.get().internal())
+                    for (Run run : jobs.active())
+                        if (   ! run.id().type().environment().isManuallyDeployed()
+                            && TenantAndApplicationId.from(run.id().application()).equals(id))
+                            jobs.abort(run.id());
+            }
+            else
+                application = application.withOutstandingChange(Change.of(version));
+
+            application = application.withProjectId(OptionalLong.of(projectId));
+            application = application.withNewSubmission(version);
+            applications().store(application.withChange(remainingChange(application.get())));
+        });
+    }
+
     /**
      * Records information when a job completes (successfully or not). This information is used when deciding what to
      * trigger next.
@@ -116,40 +142,22 @@ public class DeploymentTrigger {
         }
 
         applications().lockApplicationOrThrow(TenantAndApplicationId.from(report.applicationId()), application -> {
-            JobRun triggering;
-            // TODO jonmv: Remove this, and replace with a simple application version counter.
             if (report.jobType() == component) {
-                ApplicationVersion applicationVersion = report.version().get();
-                triggering = JobRun.triggering(applications().oldestInstalledPlatform(TenantAndApplicationId.from(report.applicationId())),
-                                               applicationVersion,
-                                               Optional.empty(), Optional.empty(), "Application commit", clock.instant());
-                if (report.success()) {
-                    if (acceptNewApplicationVersion(application.get())) {
-                        application = application.withChange(application.get().change().with(applicationVersion))
-                                                 .withOutstandingChange(Change.empty());
-                        if (application.get().internal())
-                            for (Run run : jobs.active())
-                                if (run.id().application().equals(report.applicationId()))
-                                    jobs.abort(run.id());
-                    }
-                    else
-                        application = application.withOutstandingChange(Change.of(applicationVersion));
-                }
-                application = application.withProjectId(OptionalLong.of(report.projectId()));
-            }
-            else {
-                Optional<JobStatus> status = application.get().require(report.applicationId().instance())
-                                                        .deploymentJobs().statusOf(report.jobType());
-                triggering = status.filter(job ->    job.lastTriggered().isPresent()
-                                                  && job.lastCompleted()
-                                                        .map(completion -> ! completion.at().isAfter(job.lastTriggered().get().at()))
-                                                        .orElse(true))
-                                   .orElseThrow(() -> new IllegalStateException("Notified of completion of " + report.jobType().jobName() + " for " +
-                                                                                report.applicationId() + ", but that has not been triggered; last was " +
-                                                                                status.flatMap(job -> job.lastTriggered().map(run -> run.at().toString()))
-                                                                                      .orElse("never")))
-                                   .lastTriggered().get();
+                notifyOfSubmission(application.get().id(), report.version().get(), report.projectId());
+                return;
             }
+            JobRun triggering;
+            Optional<JobStatus> status = application.get().require(report.applicationId().instance())
+                                                    .deploymentJobs().statusOf(report.jobType());
+            triggering = status.filter(job ->    job.lastTriggered().isPresent()
+                                              && job.lastCompleted()
+                                                    .map(completion -> ! completion.at().isAfter(job.lastTriggered().get().at()))
+                                                    .orElse(true))
+                               .orElseThrow(() -> new IllegalStateException("Notified of completion of " + report.jobType().jobName() + " for " +
+                                                                            report.applicationId() + ", but that has not been triggered; last was " +
+                                                                            status.flatMap(job -> job.lastTriggered().map(run -> run.at().toString()))
+                                                                                  .orElse("never")))
+                               .lastTriggered().get();
 
             application = application.with(report.applicationId().instance(),
                                            instance -> instance.withJobCompletion(report.jobType(),
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 54b5c339159..8c159644940 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
@@ -289,7 +289,9 @@ public class JobController {
             if ( ! application.get().internal())
                 application = registered(application);
 
-            long run = nextBuild(id);
+            long run = 1 + application.get().latestVersion()
+                                      .map(latestVersion -> latestVersion.buildNumber().getAsLong())
+                                      .orElse(0L);
             if (applicationPackage.compileVersion().isPresent() && applicationPackage.buildTime().isPresent())
                 version.set(ApplicationVersion.from(revision, run, authorEmail,
                                                     applicationPackage.compileVersion().get(),
@@ -487,15 +489,6 @@ public class JobController {
                      .collect(Collectors.toSet());
     }
 
-    // TODO jvenstad: Find a more appropriate way of doing this, at least when this is the only build service.
-    private long nextBuild(ApplicationId id) {
-        return 1 + controller.applications().requireInstance(id).deploymentJobs()
-                             .statusOf(JobType.component)
-                             .flatMap(JobStatus::lastCompleted)
-                             .map(JobStatus.JobRun::id)
-                             .orElse(0L);
-    }
-
     private void prunePackages(ApplicationId id) {
         controller.applications().lockApplicationIfPresent(TenantAndApplicationId.from(id), application -> {
             application.get().require(id.instance()).productionDeployments().values().stream()
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
index 49015f16cce..7a511c62388 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
@@ -102,7 +102,7 @@ class JobControllerApiHandlerHelper {
         Cursor responseObject = slime.setObject();
 
         Cursor lastVersionsObject = responseObject.setObject("lastVersions");
-        if (instance.deploymentJobs().statusOf(component).flatMap(JobStatus::lastSuccess).isPresent()) {
+        if (application.latestVersion().isPresent()) {
             lastPlatformToSlime(lastVersionsObject.setObject("platform"), controller, application, instance, change, steps);
             lastApplicationToSlime(lastVersionsObject.setObject("application"), application, instance, change, steps, controller);
         }
@@ -192,9 +192,9 @@ class JobControllerApiHandlerHelper {
 
     private static void lastApplicationToSlime(Cursor lastApplicationObject, Application application, Instance instance, Change change, DeploymentSteps steps, Controller controller) {
         long completed;
-        ApplicationVersion lastApplication = instance.deploymentJobs().statusOf(component).flatMap(JobStatus::lastSuccess).get().application();
+        ApplicationVersion lastApplication = application.latestVersion().get();
         applicationVersionToSlime(lastApplicationObject.setObject("application"), lastApplication);
-        lastApplicationObject.setLong("at", instance.deploymentJobs().statusOf(component).flatMap(JobStatus::lastSuccess).get().at().toEpochMilli());
+        lastApplicationObject.setLong("at", lastApplication.buildTime().get().toEpochMilli());
         completed = steps.productionJobs().stream().filter(type -> controller.applications().deploymentTrigger().isComplete(Change.of(lastApplication), change, instance, type)).count();
         if (Optional.of(lastApplication).equals(change.application()))
             lastApplicationObject.setString("deploying", completed + " of " + steps.productionJobs().size() + " complete");
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json
index 1c660726d61..8bb17733544 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json
@@ -360,6 +360,14 @@
               "version": "6.174.156",
               "upgrade": false,
               "reason": "Application commit",
+              "revision": {
+                "applicationBuildNumber": 123,
+                "sourceRevision": {
+                  "repositoryField": "git@git.host:user/repo.git",
+                  "branchField": "origin/master",
+                  "commitField": "49cd7bbb1ed9f4b922083cb042590b0885ffe22b"
+                }
+              },
               "at": 1511217733555
             }
           },
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json
index 9c9e1b5ec67..d1e9a004dd2 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview.json
@@ -15,7 +15,7 @@
           "gitCommit": "commit1"
         }
       },
-      "at": 2000,
+      "at": 1000,
       "deploying": "0 of 3 complete"
     }
   },