Make deactivate idempotent

3 files changed, 35 insertions, 16 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 1b3821a12bf..1edaef6edb8 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
@@ -22,6 +22,10 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname;
 import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
+import com.yahoo.vespa.hosted.controller.api.integration.athens.NToken;
+import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClient;
+import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClientFactory;
+import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerClient;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException;
@@ -31,10 +35,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordId;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator;
-import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClient;
-import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClientFactory;
-import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsException;
-import com.yahoo.vespa.hosted.controller.api.integration.athens.NToken;
 import com.yahoo.vespa.hosted.controller.api.rotation.Rotation;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.ApplicationRevision;
@@ -490,20 +490,30 @@ public class ApplicationController {
         }
     }
 
+    /** Deactivate application in the given zone */
+    public Application deactivate(Application application, Zone zone) {
+        return deactivate(application, zone, Optional.empty(), false);
+    }
+
+    /** Deactivate a known deployment of the given application */
     public Application deactivate(Application application, Deployment deployment, boolean requireThatDeploymentHasExpired) {
+        return deactivate(application, deployment.zone(), Optional.of(deployment), requireThatDeploymentHasExpired);
+    }
+
+    private Application deactivate(Application application, Zone zone, Optional<Deployment> deployment,
+                                   boolean requireThatDeploymentHasExpired) {
         try (Lock lock = lock(application.id())) {
-            // TODO: ignore no application errors for config server client, only return such errors from sherpa client.
-            if (requireThatDeploymentHasExpired && ! DeploymentExpirer.hasExpired(controller.zoneRegistry(), deployment,
-                                                                                  clock.instant()))
+            if (deployment.isPresent() && requireThatDeploymentHasExpired && ! DeploymentExpirer.hasExpired(
+                    controller.zoneRegistry(), deployment.get(), clock.instant())) {
                 return application;
+            }
 
             try { 
-                configserverClient.deactivate(new DeploymentId(application.id(), deployment.zone())); 
-            } 
-            catch (NoInstanceException e) {
+                configserverClient.deactivate(new DeploymentId(application.id(), zone));
+            }  catch (NoInstanceException ignored) {
                 // ok; already gone
             }
-            application = application.withoutDeploymentIn(deployment.zone());
+            application = application.withoutDeploymentIn(zone);
             store(application, lock);
             return application;
         }
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 d701f3d57a0..6d1edc22b93 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
@@ -796,13 +796,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
     private HttpResponse deactivate(String tenantName, String applicationName, String instanceName, String environment, String region) {
         Application application = controller.applications().require(ApplicationId.from(tenantName, applicationName, instanceName));
-        
+
         Zone zone = new Zone(Environment.from(environment), RegionName.from(region));
         Deployment deployment = application.deployments().get(zone);
-        if (deployment == null)
-            return ErrorResponse.notFoundError("Could not deactivate: " + application + " is not deployed in " + zone);
-
-        controller.applications().deactivate(application, deployment, false);
+        if (deployment == null) {
+            // Attempt to deactivate application even if the deployment is not known by the controller
+            controller.applications().deactivate(application, zone);
+        } else {
+            controller.applications().deactivate(application, deployment, false);
+        }
 
         // 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/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 97e1bac35c8..7d1700270ea 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -194,6 +194,13 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       "",
                                       Request.Method.DELETE),
                               "Deactivated tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default");
+
+        // DELETE (deactivate) a deployment is idempotent
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default",
+                                      "",
+                                      Request.Method.DELETE),
+                              "Deactivated tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default");
+
         // DELETE an application
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", "", Request.Method.DELETE),
                               "");