Disallow removal of instances specified in deployment.xml

2 files changed, 18 insertions, 6 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 dd99adcaffb..07be894eaef 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
@@ -718,6 +718,7 @@ public class ApplicationController {
         if (instances.size() > 1)
             throw new IllegalArgumentException("Could not delete application; more than one instance present: " + instances);
 
+        lockApplicationOrThrow(id, application -> store(application.with(DeploymentSpec.empty)));
         for (ApplicationId instance : instances)
             deleteInstance(instance);
 
@@ -744,6 +745,9 @@ public class ApplicationController {
                 throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments in: " +
                                                    application.get().require(instanceId.instance()).deployments().keySet().stream().map(ZoneId::toString)
                                                               .sorted().collect(Collectors.joining(", ")));
+            if (   ! application.get().deploymentSpec().equals(DeploymentSpec.empty)
+                &&   application.get().deploymentSpec().instanceNames().contains(instanceId.instance()))
+                throw new IllegalArgumentException("Can not delete '" + instanceId + "', which is specified in 'deployment.xml'; remove it there instead");
 
             Instance instance = application.get().require(instanceId.instance());
             instance.rotations().forEach(assignedRotation -> {
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 9f869b0904b..6e778fd08c1 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
@@ -6,6 +6,7 @@ import ai.vespa.hosted.api.Signatures;
 import com.yahoo.application.container.handler.Request;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.ValidationId;
+import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.AthenzService;
@@ -718,11 +719,14 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .userIdentity(USER_ID),
                               "");
 
+        // POST an application package with an empty deployment spec, to allow removal of production instances.
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/submit", POST)
+                                      .screwdriverIdentity(SCREWDRIVER_ID)
+                                      .data(createApplicationSubmissionData(new ApplicationPackageBuilder()
+                                                                                    .allow(ValidationId.deploymentRemoval)
+                                                                                    .build(), 1000)),
+                              "{\"message\":\"Application package version: 1.0.5-commit1, source revision of repository 'repository1', branch 'master' with commit 'commit1', by a@b, built against 6.1 at 1970-01-01T00:00:01Z\"}");
         // DELETE all instances under an application to delete the application
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/default", DELETE)
-                                      .userIdentity(USER_ID)
-                                      .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
-                              "{\"message\":\"Deleted instance tenant1.application1.default\"}");
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/my-user", DELETE)
                                       .userIdentity(USER_ID)
                                       .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
@@ -735,8 +739,12 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .userIdentity(USER_ID)
                                       .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
                               "{\"message\":\"Deleted instance tenant1.application1.instance2\"}");
-
-        // DELETE a tenant
+        // DELETE the application which now only has one instance
+        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", DELETE)
+                                      .userIdentity(USER_ID)
+                                      .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
+                              "{\"message\":\"Deleted application tenant1.application1\"}");
+        // DELETE an empty tenant
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE).userIdentity(USER_ID)
                                       .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
                               new File("tenant-without-applications.json"));