Always attempt to deactivate deployment

2 files changed, 35 insertions, 22 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 727f2f58c90..1bcef125e8c 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
@@ -428,14 +428,6 @@ public class ApplicationController {
                                            (wantedMajor.isPresent() ? " for specified major: " + wantedMajor.getAsInt() : ""));
     }
 
-    private OptionalInt firstNonEmpty(OptionalInt... choices) {
-        for (OptionalInt choice : choices)
-            if (choice.isPresent())
-                return choice;
-
-        return OptionalInt.empty();
-    }
-
     /**
      * Creates a new application for an existing tenant.
      *
@@ -729,10 +721,12 @@ public class ApplicationController {
         // Remove the instance as well, if it is no longer referenced, and contains only production deployments that are removed now.
         boolean removeInstance =    ! deploymentSpec.instanceNames().contains(instance)
                                  &&   application.get().require(instance).deployments().size() == deploymentsToRemove.size();
-        for (ZoneId zone : deploymentsToRemove)
-            application = deactivate(application, instance, zone);
-        if (removeInstance)
+        for (ZoneId zone : deploymentsToRemove) {
+            application = deactivate(application.get().id().instance(instance), zone, Optional.of(application)).get();
+        }
+        if (removeInstance) {
             application = application.without(instance);
+        }
         return application;
     }
 
@@ -871,10 +865,13 @@ public class ApplicationController {
         configServer.setSuspension(deploymentId, suspend);
     }
 
-    /** Deactivate application in the given zone */
-    public void deactivate(ApplicationId id, ZoneId zone) {
-        lockApplicationOrThrow(TenantAndApplicationId.from(id),
-                               application -> store(deactivate(application, id.instance(), zone)));
+    /** Deactivate application in the given zone. Even if the application itself does not exist, deactivation of the deployment will still be attempted */
+    public void deactivate(ApplicationId instanceId, ZoneId zone) {
+        TenantAndApplicationId applicationId = TenantAndApplicationId.from(instanceId);
+        try (Mutex lock = lock(applicationId)) {
+            Optional<LockedApplication> application = getApplication(applicationId).map(app -> new LockedApplication(app, lock));
+            deactivate(instanceId, zone, application).ifPresent(this::store);
+        }
     }
 
     /**
@@ -882,18 +879,18 @@ public class ApplicationController {
      *
      * @return the application with the deployment in the given zone removed
      */
-    private LockedApplication deactivate(LockedApplication application, InstanceName instanceName, ZoneId zone) {
-        DeploymentId id = new DeploymentId(application.get().id().instance(instanceName), zone);
+    private Optional<LockedApplication> deactivate(ApplicationId instanceId, ZoneId zone, Optional<LockedApplication> application) {
+        DeploymentId id = new DeploymentId(instanceId, zone);
         try {
             configServer.deactivate(id);
         } finally {
-            controller.routing().of(id).configure(application.get().deploymentSpec());
-            if (zone.environment().isManuallyDeployed())
+            application.ifPresent(app -> controller.routing().of(id).configure(app.get().deploymentSpec()));
+            if (id.zoneId().environment().isManuallyDeployed())
                 applicationStore.putMetaTombstone(id, clock.instant());
-            if (!zone.environment().isTest())
+            if (!id.zoneId().environment().isTest())
                 controller.notificationsDb().removeNotifications(NotificationSource.from(id));
         }
-        return application.with(instanceName, instance -> instance.withoutDeploymentIn(zone));
+        return application.map(app -> app.with(instanceId.instance(), instance -> instance.withoutDeploymentIn(id.zoneId())));
     }
 
     public DeploymentTrigger deploymentTrigger() { return deploymentTrigger; }
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 978587d9c5c..8fdff787420 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
@@ -15,12 +15,15 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.Tags;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.RoutingMethod;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.path.Path;
 import com.yahoo.vespa.flags.PermanentFlags;
+import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeploymentData;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
+import com.yahoo.vespa.hosted.controller.api.integration.billing.Quota;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
@@ -38,7 +41,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
-import com.yahoo.vespa.hosted.controller.deployment.RunStatus;
 import com.yahoo.vespa.hosted.controller.deployment.Submission;
 import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
 import com.yahoo.vespa.hosted.controller.notification.Notification;
@@ -54,6 +56,7 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence;
 import com.yahoo.vespa.hosted.rotation.config.RotationsConfig;
 import org.junit.jupiter.api.Test;
 
+import java.io.InputStream;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Collection;
@@ -1399,4 +1402,17 @@ public class ControllerTest {
         }
     }
 
+    @Test
+    void testDeactivateDeploymentUnknownByController() {
+        DeploymentContext context = tester.newDeploymentContext();
+        DeploymentId deployment = context.deploymentIdIn(ZoneId.from("prod", "us-west-1"));
+        DeploymentData deploymentData = new DeploymentData(deployment.applicationId(), Tags.empty(), deployment.zoneId(), InputStream::nullInputStream, Version.fromString("6.1"),
+                                                           Set.of(), Optional::empty, Optional.empty(), Optional.empty(),
+                                                           Quota::unlimited, List.of(), List.of(), Optional::empty, false);
+        tester.configServer().deploy(deploymentData);
+        assertTrue(tester.configServer().application(deployment.applicationId(), deployment.zoneId()).isPresent());
+        tester.controller().applications().deactivate(deployment.applicationId(), deployment.zoneId());
+        assertFalse(tester.configServer().application(deployment.applicationId(), deployment.zoneId()).isPresent());
+    }
+
 }