Merge pull request #8678 from vespa-engine/hmusum/change-deactivate-to-return-not-found-exception

Deactivate should return NotFoundException

7 files changed, 44 insertions, 21 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
index 9a75764befe..07eaf046eeb 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
@@ -23,6 +23,7 @@ import java.util.Set;
 public interface ConfigServer {
 
     interface PreparedApplication {
+        // TODO: Remove the two methods below
         void activate();
         List<Log> messages();
         PrepareResponse prepareResponse();
@@ -32,7 +33,7 @@ public interface ConfigServer {
 
     void restart(DeploymentId deployment, Optional<Hostname> hostname);
 
-    void deactivate(DeploymentId deployment) throws NoInstanceException;
+    void deactivate(DeploymentId deployment) throws NotFoundException;
 
     boolean isSuspended(DeploymentId deployment);
 
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NoInstanceException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NoInstanceException.java
deleted file mode 100644
index a415721407f..00000000000
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NoInstanceException.java
+++ /dev/null
@@ -1,11 +0,0 @@
-// 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.api.integration.configserver;
-
-/**
- * @author Tony Vaagenes
- */
-public class NoInstanceException extends Exception {
-    public NoInstanceException(String msg) {
-        super(msg);
-    }
-}
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NotFoundException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NotFoundException.java
new file mode 100644
index 00000000000..5f0e4f8e551
--- /dev/null
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NotFoundException.java
@@ -0,0 +1,11 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.api.integration.configserver;
+
+/**
+ * @author Tony Vaagenes
+ */
+public class NotFoundException extends Exception {
+    public NotFoundException(String msg) {
+        super(msg);
+    }
+}
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 d68b3652675..f51bccff5d1 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
@@ -26,8 +26,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFact
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.NotFoundException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationStore;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
@@ -644,7 +644,7 @@ public class ApplicationController {
         try {
             configServer.deactivate(new DeploymentId(application.get().id(), zone));
         }
-        catch (NoInstanceException ignored) {
+        catch (NotFoundException ignored) {
             // ok; already gone
         }
         return application.withoutDeploymentIn(zone);
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 6e3b6110efb..cf706c0a1a4 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
@@ -9,7 +9,7 @@ import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
 import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.NotFoundException;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
@@ -319,7 +319,7 @@ public class JobController {
         try {
             controller.configServer().deactivate(new DeploymentId(id.id(), type.zone(controller.system())));
         }
-        catch (NoInstanceException ignored) {
+        catch (NotFoundException ignored) {
             // Already gone -- great!
         }
     }
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 32068b006f0..63a9d0ce905 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
@@ -506,6 +506,24 @@ public class ControllerTest {
         assertFalse(tester.configServer().isSuspended(deployment2));
     }
 
+    // Application may already have been deleted, or deployment failed without response, test that deleting a
+    // second time will not fail
+    @Test
+    public void testDeletingApplicationThatHasAlreadyBeenDeleted() {
+        DeploymentTester tester = new DeploymentTester();
+        Application app = tester.createApplication("app2", "tenant1", 1, 12L);
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .environment(Environment.prod)
+                .region("us-east-3")
+                .region("us-west-1")
+                .build();
+
+        ZoneId zone = ZoneId.from("prod", "us-west-1");
+        tester.controller().applications().deploy(app.id(), zone, Optional.of(applicationPackage), DeployOptions.none());
+        tester.controller().applications().deactivate(app.id(), ZoneId.from(Environment.prod, RegionName.from("us-west-1")));
+        tester.controller().applications().deactivate(app.id(), ZoneId.from(Environment.prod, RegionName.from("us-west-1")));
+    }
+
     private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) {
         Version next = Version.fromString("6.2");
         tester.upgradeSystem(next);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 69f348c9174..bb8228db679 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -19,6 +19,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalanc
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Logs;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.NotFoundException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
@@ -205,10 +206,10 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
 
         return new PreparedApplication() {
 
-            @Override
+            // TODO: Remove when no longer part of interface
             public void activate() {}
 
-            @Override
+            // TODO: Remove when no longer part of interface
             public List<Log> messages() {
                 Log warning = new Log();
                 warning.level = "WARNING";
@@ -272,10 +273,13 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     }
 
     @Override
-    public void deactivate(DeploymentId deployment) {
-        applications.remove(deployment.applicationId());
+    public void deactivate(DeploymentId deployment) throws NotFoundException {
+        ApplicationId applicationId = deployment.applicationId();
         nodeRepository().removeByHostname(deployment.zoneId(),
-                                          nodeRepository().list(deployment.zoneId(), deployment.applicationId()));
+                                          nodeRepository().list(deployment.zoneId(), applicationId));
+        if ( ! applications.containsKey(applicationId))
+            throw new NotFoundException("No application with id " + applicationId + " exists, cannot deactivate");
+        applications.remove(applicationId);
         serviceStatus.remove(deployment);
     }