Clean up deactivation code

10 files changed, 16 insertions, 29 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 6e1940931e7..5e775ea6cd3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -579,8 +579,8 @@ public class ApplicationController {
     }
 
     /** Deactivate application in the given zone */
-    public void deactivate(Application application, ZoneId zone) {
-        lockOrThrow(application.id(), lockedApplication -> store(deactivate(lockedApplication, zone)));
+    public void deactivate(ApplicationId application, ZoneId zone) {
+        lockOrThrow(application, lockedApplication -> store(deactivate(lockedApplication, zone)));
     }
 
     /**
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index a0e273dc4c8..7241d35eaf8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -346,23 +346,13 @@ public class InternalStepRunner implements StepRunner {
 
     private Status deactivateReal(RunId id, ByteArrayLogger logger) {
         logger.log("Deactivating deployment of " + id.application() + " in " + zone(id.type()) + " ...");
-        Status status = deactivate(id.application(), id.type());
-        if (status == succeeded)
-            controller.applications().lockOrThrow(id.application(), application ->
-                    controller.applications().store(application.withoutDeploymentIn(zone(id.type()))));
-        return status;
+        controller.applications().deactivate(id.application(), id.type().zone(controller.system()));
+        return succeeded;
     }
 
     private Status deactivateTester(RunId id, ByteArrayLogger logger) {
         logger.log("Deactivating tester of " + id.application() + " in " + zone(id.type()) + " ...");
-        return deactivate(testerOf(id.application()), id.type());
-    }
-
-    private Status deactivate(ApplicationId id, JobType type) {
-        try {
-            controller.configServer().deactivate(new DeploymentId(id, zone(type)));
-        }
-        catch (NoInstanceException e) { }
+        controller.jobController().deactivateTester(id.application(), id.type());
         return succeeded;
     }
 
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 4a14d116c1c..c966492259f 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
@@ -242,13 +242,12 @@ public class JobController {
                });
     }
 
-    // TODO jvenstad: Urgh, clean this up somehow?
     public void deactivateTester(ApplicationId id, JobType type) {
         try {
             controller.configServer().deactivate(new DeploymentId(testerOf(id), type.zone(controller.system())));
         }
         catch (NoInstanceException ignored) {
-            // ok; already gone
+            // Already gone -- great!
         }
     }
 
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 423f7f50aed..9133c8980ec 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
@@ -42,7 +42,7 @@ public class DeploymentExpirer extends Maintainer {
 
                 if (hasExpired(controller().zoneRegistry(), deployment, clock.instant())) {
                     try {
-                        controller().applications().deactivate(application, deployment.zone());
+                        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 " +
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 c079fb31e99..34d2257c5f4 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
@@ -820,7 +820,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         Application application = controller.applications().require(ApplicationId.from(tenantName, applicationName, instanceName));
 
         // Attempt to deactivate application even if the deployment is not known by the controller
-        controller.applications().deactivate(application, ZoneId.from(environment, region));
+        controller.applications().deactivate(application.id(), ZoneId.from(environment, region));
 
         // TODO: Change to return JSON
         return new StringResponse("Deactivated " + path(TenantResource.API_PATH, tenantName,
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 85c7ea0af1e..635e0c1fb26 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -305,8 +305,8 @@ public class ControllerTest {
                     .build();
             tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit();
             tester.deployAndNotify(app1, applicationPackage, true, systemTest);
-            tester.applications().deactivate(app1, ZoneId.from(Environment.test, RegionName.from("us-east-1")));
-            tester.applications().deactivate(app1, ZoneId.from(Environment.staging, RegionName.from("us-east-3")));
+            tester.applications().deactivate(app1.id(), ZoneId.from(Environment.test, RegionName.from("us-east-1")));
+            tester.applications().deactivate(app1.id(), ZoneId.from(Environment.staging, RegionName.from("us-east-3")));
             tester.applications().deleteApplication(app1.id(), Optional.of(new NToken("ntoken")));
             try (RotationLock lock = tester.applications().rotationRepository().lock()) {
                 assertTrue("Rotation is unassigned",
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index 75a6d3d72d9..44af8edb96e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -30,7 +30,6 @@ import java.time.Duration;
 import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
-import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -267,7 +266,7 @@ public class DeploymentTester {
         }
         // Deactivate test deployments after deploy. This replicates the behaviour of the tenant pipeline
         if (job.isTest()) {
-            controller().applications().deactivate(application, job.zone(controller().system()));
+            controller().applications().deactivate(application.id(), job.zone(controller().system()));
         }
         jobCompletion(job).application(application).success(success).submit();
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java
index 703d65c8f9d..555fdb338e8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java
@@ -73,8 +73,8 @@ public class ApplicationOwnershipConfirmerTest {
 
         // The user deletes all production deployments — see that the issue is forgotten.
         assertEquals("Confirmation issue for user is sitll open.", issueId, userApp.get().ownershipIssueId());
-        tester.controller().applications().deactivate(userApp.get(), userApp.get().productionDeployments().keySet().stream().findAny().get());
-        tester.controller().applications().deactivate(userApp.get(), userApp.get().productionDeployments().keySet().stream().findAny().get());
+        tester.controller().applications().deactivate(userApp.get().id(), userApp.get().productionDeployments().keySet().stream().findAny().get());
+        tester.controller().applications().deactivate(userApp.get().id(), userApp.get().productionDeployments().keySet().stream().findAny().get());
         assertTrue("No production deployments are listed for user.", userApp.get().productionDeployments().isEmpty());
         confirmer.maintain();
         confirmer.maintain();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
index 1bedb29ec97..b950e969300 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
@@ -86,8 +86,8 @@ public class DnsMaintainerTest {
         tester.jobCompletion(component).application(application).nextBuildNumber().uploadArtifact(applicationPackage).submit();
 
         tester.deployAndNotify(application, applicationPackage, true, systemTest);
-        tester.applications().deactivate(application, ZoneId.from(Environment.test, RegionName.from("us-east-1")));
-        tester.applications().deactivate(application, ZoneId.from(Environment.staging, RegionName.from("us-east-3")));
+        tester.applications().deactivate(application.id(), ZoneId.from(Environment.test, RegionName.from("us-east-1")));
+        tester.applications().deactivate(application.id(), ZoneId.from(Environment.staging, RegionName.from("us-east-3")));
         tester.applications().deleteApplication(application.id(), Optional.of(new NToken("ntoken")));
 
         // DnsMaintainer removes records
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
index 1ed2af1f7b9..c284b120e33 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
@@ -61,8 +61,7 @@ public class DeploymentApiTest extends ControllerContainerTest {
 
         // Deploy once so that job information is stored, then remove the deployment
         deployCompletely(applicationWithoutDeployment, applicationPackage, 3L, true);
-        tester.controller().applications().deactivate(applicationWithoutDeployment,
-                                                      ZoneId.from("prod", "corp-us-east-1"));
+        tester.controller().applications().deactivate(applicationWithoutDeployment.id(), ZoneId.from("prod", "corp-us-east-1"));
 
         // New version released
         version = Version.fromString("5.1");