Expire deployment based on time of deployment attempt

2 files changed, 54 insertions, 26 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java
index 37de7369452..e5316788802 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java
@@ -1,13 +1,18 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
+import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.Instance;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
+import com.yahoo.vespa.hosted.controller.deployment.Run;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
+import java.util.Optional;
 import java.util.logging.Level;
 
 /**
@@ -28,7 +33,7 @@ public class DeploymentExpirer extends ControllerMaintainer {
         for (Application application : controller().applications().readable()) {
             for (Instance instance : application.instances().values())
                 for (Deployment deployment : instance.deployments().values()) {
-                    if (!isExpired(deployment)) continue;
+                    if (!isExpired(deployment, instance.id())) continue;
 
                     try {
                         log.log(Level.INFO, "Expiring deployment of " + instance.id() + " in " + deployment.zone());
@@ -45,10 +50,19 @@ public class DeploymentExpirer extends ControllerMaintainer {
     }
 
     /** Returns whether given deployment has expired according to its TTL */
-    private boolean isExpired(Deployment deployment) {
+    private boolean isExpired(Deployment deployment, ApplicationId instance) {
         if (deployment.zone().environment().isProduction()) return false; // Never expire production deployments
-        return controller().zoneRegistry().getDeploymentTimeToLive(deployment.zone())
-                           .map(timeToLive -> deployment.at().plus(timeToLive).isBefore(controller().clock().instant()))
+
+        Optional<Duration> ttl = controller().zoneRegistry().getDeploymentTimeToLive(deployment.zone());
+        if (ttl.isEmpty()) return false;
+
+        Optional<JobId> jobId = JobType.from(controller().system(), deployment.zone())
+                                       .map(type -> new JobId(instance, type));
+        if (jobId.isEmpty()) return false;
+
+        return controller().jobController().last(jobId.get())
+                           .flatMap(Run::end)
+                           .map(end -> end.plus(ttl.get()).isBefore(controller().clock().instant()))
                            .orElse(false);
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
index d718dc6b9cf..232521c9609 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
@@ -10,13 +10,15 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.deployment.Run;
+import com.yahoo.vespa.hosted.controller.deployment.RunStatus;
 import org.junit.Test;
 
 import java.time.Duration;
-import java.util.List;
-import java.util.stream.Collectors;
+import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
 
 /**
  * @author bratseth
@@ -27,12 +29,9 @@ public class DeploymentExpirerTest {
 
     @Test
     public void testDeploymentExpiry() {
-        tester.controllerTester().zoneRegistry().setDeploymentTimeToLive(
-                ZoneId.from(Environment.dev, RegionName.from("us-east-1")),
-                Duration.ofDays(14)
-        );
-        DeploymentExpirer expirer = new DeploymentExpirer(tester.controller(), Duration.ofDays(10)
-        );
+        ZoneId devZone = ZoneId.from(Environment.dev, RegionName.from("us-east-1"));
+        tester.controllerTester().zoneRegistry().setDeploymentTimeToLive(devZone, Duration.ofDays(14));
+        DeploymentExpirer expirer = new DeploymentExpirer(tester.controller(), Duration.ofDays(1));
         var devApp = tester.newDeploymentContext("tenant1", "app1", "default");
         var prodApp = tester.newDeploymentContext("tenant2", "app2", "default");
 
@@ -45,27 +44,42 @@ public class DeploymentExpirerTest {
 
         // Deploy prod
         prodApp.submit(appPackage).deploy();
-
-        assertEquals(1, permanentDeployments(devApp.instance()).size());
-        assertEquals(1, permanentDeployments(prodApp.instance()).size());
+        assertEquals(1, permanentDeployments(devApp.instance()));
+        assertEquals(1, permanentDeployments(prodApp.instance()));
 
         // Not expired at first
         expirer.maintain();
-        assertEquals(1, permanentDeployments(devApp.instance()).size());
-        assertEquals(1, permanentDeployments(prodApp.instance()).size());
+        assertEquals(1, permanentDeployments(devApp.instance()));
+        assertEquals(1, permanentDeployments(prodApp.instance()));
+
+        // Deploy dev unsuccessfully a few days before expiry
+        tester.clock().advance(Duration.ofDays(12));
+        tester.configServer().throwOnNextPrepare(new RuntimeException(getClass().getSimpleName()));
+        tester.jobs().deploy(devApp.instanceId(), JobType.devUsEast1, Optional.empty(), appPackage);
+        Run lastRun = tester.jobs().last(devApp.instanceId(), JobType.devUsEast1).get();
+        assertSame(RunStatus.error, lastRun.status());
+        Deployment deployment = tester.applications().requireInstance(devApp.instanceId())
+                                      .deployments().get(devZone);
+        assertEquals("Time of last run is after time of deployment", Duration.ofDays(12),
+                     Duration.between(deployment.at(), lastRun.end().get()));
+
+        // Dev application does not expire based on time of successful deployment
+        tester.clock().advance(Duration.ofDays(2));
+        expirer.maintain();
+        assertEquals(1, permanentDeployments(devApp.instance()));
+        assertEquals(1, permanentDeployments(prodApp.instance()));
 
-        // The dev application is removed
-        tester.clock().advance(Duration.ofDays(15));
+        // Dev application expires when enough time has passed since most recent attempt
+        tester.clock().advance(Duration.ofDays(12));
         expirer.maintain();
-        assertEquals(0, permanentDeployments(devApp.instance()).size());
-        assertEquals(1, permanentDeployments(prodApp.instance()).size());
+        assertEquals(0, permanentDeployments(devApp.instance()));
+        assertEquals(1, permanentDeployments(prodApp.instance()));
     }
 
-    private List<Deployment> permanentDeployments(Instance instance) {
-        return tester.controller().applications().getInstance(instance.id()).get().deployments().values().stream()
-                     .filter(deployment -> deployment.zone().environment() != Environment.test &&
-                                      deployment.zone().environment() != Environment.staging)
-                     .collect(Collectors.toList());
+    private long permanentDeployments(Instance instance) {
+        return tester.controller().applications().requireInstance(instance.id()).deployments().values().stream()
+                     .filter(deployment -> !deployment.zone().environment().isTest())
+                     .count();
     }
 
 }