Remove resources when deleting application with no active session

3 files changed, 31 insertions, 15 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index 6fac2f7f73a..b13d4542eba 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -507,7 +507,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     // ---------------- Application operations ----------------------------------------------------------------
 
     /**
-     * Deletes an application
+     * Deletes an application and associated resources
      *
      * @return true if the application was found and deleted, false if it was not present
      * @throws RuntimeException if deleting the application fails. This method is exception safe.
@@ -522,17 +522,21 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
                                                                                  .map(lock -> new ApplicationTransaction(lock, transaction));
         try (var applicationLock = tenantApplications.lock(applicationId)) {
             Optional<Long> activeSession = tenantApplications.activeSessionOf(applicationId);
-            if (activeSession.isEmpty()) return false;
-
-            try {
-                Session session = getRemoteSession(tenant, activeSession.get());
-                transaction.add(tenant.getSessionRepository().createSetStatusTransaction(session, Session.Status.DELETE));
-            } catch (NotFoundException e) {
-                log.log(Level.INFO, TenantRepository.logPre(applicationId) + "Active session exists, but has not been deleted properly. Trying to cleanup");
+            CompletionWaiter waiter;
+            if (activeSession.isPresent()) {
+                try {
+                    Session session = getRemoteSession(tenant, activeSession.get());
+                    transaction.add(tenant.getSessionRepository().createSetStatusTransaction(session, Session.Status.DELETE));
+                } catch (NotFoundException e) {
+                    log.log(Level.INFO, TenantRepository.logPre(applicationId) + "Active session exists, but has not been deleted properly. Trying to cleanup");
+                }
+                waiter = tenantApplications.createRemoveApplicationWaiter(applicationId);
+            } else {
+                // If there's no active session, we still want to clean up any resources created in a failing prepare
+                waiter = new NoopCompletionWaiter();
             }
 
             Curator curator = tenantRepository.getCurator();
-            CompletionWaiter waiter = tenantApplications.createRemoveApplicationWaiter(applicationId);
             transaction.add(new ContainerEndpointsCache(tenant.getPath(), curator).delete(applicationId)); // TODO: Not unit tested
             // Delete any application roles
             transaction.add(new ApplicationRolesStore(curator, tenant.getPath()).delete(applicationId));
@@ -553,7 +557,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
             // Wait for app being removed on other servers
             waiter.awaitCompletion(Duration.ofSeconds(30));
 
-            return true;
+            return activeSession.isPresent();
         } finally {
             applicationTransaction.ifPresent(ApplicationTransaction::close);
         }
@@ -1210,4 +1214,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
 
     }
 
+    private static class NoopCompletionWaiter implements CompletionWaiter {
+
+        @Override
+        public void awaitCompletion(Duration timeout) {}
+
+        @Override
+        public void notifyCompletion() {}
+
+    }
+
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
index 1ad0765a25f..17a0a2e3cab 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
@@ -333,14 +333,16 @@ public class ApplicationRepositoryTest {
             assertTrue(applicationRepository.delete(applicationId()));
             assertTrue(applicationRepository.getActiveSession(applicationId()).isEmpty());
             assertEquals(Optional.empty(), sessionRepository.getRemoteSession(sessionId).applicationSet());
-            assertTrue(provisioner.removed());
+            assertEquals(1, provisioner.removeCount());
             assertEquals(tenant().getName(), provisioner.lastApplicationId().tenant());
             assertEquals(applicationId(), provisioner.lastApplicationId());
             assertTrue(curator.exists(sessionNode));
             assertEquals(Session.Status.DELETE.name(), Utf8.toString(curator.getData(sessionNode.append("sessionState")).get()));
             assertTrue(sessionFile.exists());
 
+            // Deleting a non-existent application still attempts to remove resources
             assertFalse(applicationRepository.delete(applicationId()));
+            assertEquals(2, provisioner.removeCount());
         }
 
         {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java b/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java
index f08da1e65c2..0ba3a6d883c 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java
@@ -23,7 +23,7 @@ import java.util.List;
 public class MockProvisioner implements Provisioner {
 
     private boolean activated = false;
-    private boolean removed = false;
+    private int removeCount = 0;
     private boolean restarted = false;
     private ApplicationId lastApplicationId;
     private Collection<HostSpec> lastHosts;
@@ -70,7 +70,7 @@ public class MockProvisioner implements Provisioner {
         if (failureOnRemove)
             throw new IllegalStateException("Unable to remove " + transaction.application());
 
-        removed = true;
+        removeCount++;
         lastApplicationId = transaction.application();
     }
 
@@ -94,8 +94,8 @@ public class MockProvisioner implements Provisioner {
         return activated;
     }
 
-    public boolean removed() {
-        return removed;
+    public int removeCount() {
+        return removeCount;
     }
 
     public boolean restarted() {