Handle multiple notifications from last job

3 files changed, 32 insertions, 3 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
index dc1dcb2d5ed..59ccfc15bcc 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
@@ -128,10 +128,13 @@ public class DeploymentJobs {
     }
 
     /** Returns whether change has been deployed completely */
-    public boolean isDeployed(Change change) {
+    public boolean isDeployed(Optional<Change> change) {
+        if (!change.isPresent()) {
+            return true;
+        }
         return status.values().stream()
                 .filter(status -> status.type().isProduction())
-                .allMatch(status -> isSuccessful(change, status.type()));
+                .allMatch(status -> isSuccessful(change.get(), status.type()));
     }
 
     /** Returns whether job has completed successfully */
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 ad48ef5c9cc..e4fc00d532e 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
@@ -72,7 +72,7 @@ public class DeploymentTrigger {
                 else { // start a new change deployment
                     application = application.withDeploying(Optional.of(Change.ApplicationChange.unknown()));
                 }
-            } else if (order.isLast(report.jobType(), application) && report.success() && application.deploymentJobs().isDeployed(application.deploying().get())) {
+            } else if (order.isLast(report.jobType(), application) && report.success() && application.deploymentJobs().isDeployed(application.deploying())) {
                 application = application.withDeploying(Optional.empty());
             }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
index 7ed0ad843cc..0c0cc0485c8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
@@ -254,4 +254,30 @@ public class DeploymentTriggerTest {
         tester.deployAndNotify(application, newApplicationPackage, true, JobType.productionApNortheast1);
         assertTrue("All jobs consumed", buildSystem.jobs().isEmpty());
     }
+
+    @Test
+    public void testHandleMultipleNotificationsFromLastJob() {
+        DeploymentTester tester = new DeploymentTester();
+        BuildSystem buildSystem = tester.buildSystem();
+        TenantId tenant = tester.controllerTester().createTenant("tenant1", "domain1", 1L);
+        Application application = tester.controllerTester().createApplication(tenant, "app1", "default", 1L);
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .environment(Environment.prod)
+                .region("corp-us-east-1")
+                .build();
+
+        // Component job finishes
+        tester.notifyJobCompletion(JobType.component, application, true);
+
+        // Application is deployed to all test environments and declared zones
+        tester.deployAndNotify(application, applicationPackage, true, JobType.systemTest);
+        tester.deployAndNotify(application, applicationPackage, true, JobType.stagingTest);
+        tester.deployAndNotify(application, applicationPackage, true, JobType.productionCorpUsEast1);
+
+        // Extra notification for last job
+        tester.notifyJobCompletion(JobType.productionCorpUsEast1, application, true);
+        assertFalse("Change has been deployed",
+                    tester.applications().require(application.id()).deploying().isPresent());
+        assertTrue("All jobs consumed", buildSystem.jobs().isEmpty());
+    }
 }