Simplify DeploymentExpirer

2 files changed, 16 insertions, 35 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 9133c8980ec..cd3341ed3a6 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,16 +1,12 @@
 // 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.Environment;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.yolean.Exceptions;
 
-import java.time.Clock;
 import java.time.Duration;
-import java.time.Instant;
 import java.util.logging.Level;
 
 /**
@@ -21,41 +17,32 @@ import java.util.logging.Level;
  */
 public class DeploymentExpirer extends Maintainer {
 
-    private final Clock clock;
-
     public DeploymentExpirer(Controller controller, Duration interval, JobControl jobControl) {
-        this(controller, interval, Clock.systemUTC(), jobControl);
-    }
-
-    public DeploymentExpirer(Controller controller, Duration interval, Clock clock, JobControl jobControl) {
         super(controller, interval, jobControl);
-        this.clock = clock;
     }
 
     @Override
     protected void maintain() {
         for (Application application : controller().applications().asList()) {
             for (Deployment deployment : application.deployments().values()) {
-                if (deployment.zone().environment().equals(Environment.prod)) {
-                    continue;
-                }
-
-                if (hasExpired(controller().zoneRegistry(), deployment, clock.instant())) {
-                    try {
-                        controller().applications().deactivate(application.id(), deployment.zone());
-                    } catch (Exception e) {
-                        log.log(Level.WARNING, "Could not expire " + deployment + " of " + application +
-                                               ": " + Exceptions.toMessageString(e) + ". Retrying in " +
-                                               maintenanceInterval());
-                    }
+                if (!isExpired(deployment)) continue;
+
+                try {
+                    controller().applications().deactivate(application.id(), deployment.zone());
+                } catch (Exception e) {
+                    log.log(Level.WARNING, "Could not expire " + deployment + " of " + application +
+                                           ": " + Exceptions.toMessageString(e) + ". Retrying in " +
+                                           maintenanceInterval());
                 }
             }
         }
     }
 
-    private static boolean hasExpired(ZoneRegistry zoneRegistry, Deployment deployment, Instant now) {
-        return zoneRegistry.getDeploymentTimeToLive(deployment.zone())
-                           .map(timeToLive -> deployment.at().plus(timeToLive).isBefore(now))
+    /** Returns whether given deployment has expired according to its TTL */
+    private boolean isExpired(Deployment deployment) {
+        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()))
                            .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 f642f3210b7..730b2943431 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
@@ -3,14 +3,13 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
-import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.vespa.hosted.controller.Application;
 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.persistence.MockCuratorDb;
-import org.junit.Before;
 import org.junit.Test;
 
 import java.time.Duration;
@@ -24,12 +23,7 @@ import static org.junit.Assert.assertEquals;
  */
 public class DeploymentExpirerTest {
 
-    private DeploymentTester tester;
-
-    @Before
-    public void before() {
-        tester = new DeploymentTester();
-    }
+    private final DeploymentTester tester = new DeploymentTester();
 
     @Test
     public void testDeploymentExpiry() {
@@ -38,7 +32,7 @@ public class DeploymentExpirerTest {
                 Duration.ofDays(14)
         );
         DeploymentExpirer expirer = new DeploymentExpirer(tester.controller(), Duration.ofDays(10),
-                                                          tester.clock(), new JobControl(new MockCuratorDb()));
+                                                          new JobControl(new MockCuratorDb()));
         Application devApp = tester.createApplication("app1", "tenant1", 123L, 1L);
         Application prodApp = tester.createApplication("app2", "tenant2", 456L, 2L);