Do not delete unused tenants that have been created last week

* Add support for getting stat info for a zookeeper node
* For unused tenants, check when tenant was created and do not delete
  unless TTL (7 days) has been exceeded

5 files changed, 76 insertions, 16 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 f8e1bcec0c3..a80ecffd94a 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
@@ -438,14 +438,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
 
     // ---------------- Tenant operations ----------------------------------------------------------------
 
-    public Set<TenantName> removeUnusedTenants() {
-        Set<TenantName> tenantsToBeDeleted = tenantRepository.getAllTenantNames().stream()
+    public Set<TenantName> deleteUnusedTenants(Duration ttlForUnusedTenant, Instant now) {
+        return tenantRepository.getAllTenantNames().stream()
                 .filter(tenantName -> activeApplications(tenantName).isEmpty())
                 .filter(tenantName -> !tenantName.equals(TenantName.defaultName())) // Not allowed to remove 'default' tenant
                 .filter(tenantName -> !tenantName.equals(TenantRepository.HOSTED_VESPA_TENANT)) // Not allowed to remove 'hosted-vespa' tenant
+                .filter(tenantName -> tenantRepository.getTenant(tenantName).getCreatedTime().isBefore(now.minus(ttlForUnusedTenant)))
+                .peek(tenantRepository::deleteTenant)
                 .collect(Collectors.toSet());
-        tenantsToBeDeleted.forEach(tenantRepository::deleteTenant);
-        return tenantsToBeDeleted;
     }
 
     public void deleteTenant(TenantName tenantName) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java
index 36306dbdde8..ad3b1f9b1ea 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java
@@ -5,15 +5,24 @@ import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.curator.Curator;
 
 import java.time.Duration;
+import java.time.Instant;
 
 public class TenantsMaintainer extends Maintainer {
 
+    private final Duration ttlForUnusedTenant;
+
     TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval) {
+        this(applicationRepository, curator, interval, Duration.ofDays(7));
+    }
+
+    private TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, Duration ttlForUnusedTenant) {
         super(applicationRepository, curator, interval);
+        this.ttlForUnusedTenant = ttlForUnusedTenant;
     }
 
     @Override
+    // Delete unused tenants that were created more than ttlForUnusedTenant ago
     protected void maintain() {
-        applicationRepository.removeUnusedTenants();
+        applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, Instant.now());
     }
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java
index 3fe4c750922..3557d7bf9ab 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java
@@ -11,6 +11,11 @@ import com.yahoo.vespa.config.server.session.LocalSessionRepo;
 import com.yahoo.vespa.config.server.session.RemoteSessionRepo;
 import com.yahoo.vespa.config.server.session.SessionFactory;
 import com.yahoo.vespa.curator.Curator;
+import org.apache.zookeeper.Op;
+import org.apache.zookeeper.data.Stat;
+
+import java.time.Instant;
+import java.util.Optional;
 
 /**
  * Contains all tenant-level components for a single tenant, dealing with editing sessions and
@@ -124,6 +129,14 @@ public class Tenant implements TenantHandlerProvider {
         return curator;
     }
 
+    public Instant getCreatedTime() {
+        Optional<Stat> stat = curator.getStat(path);
+        if (stat.isPresent())
+            return Instant.ofEpochMilli(stat.get().getCtime());
+        else
+            return Instant.now();
+    }
+
     @Override
     public boolean equals(Object other) {
         if (!(other instanceof Tenant)) {
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 f7e76c2b3bb..12a4ed7a4c0 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
@@ -11,11 +11,11 @@ import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.Provisioner;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.io.IOUtils;
+import com.yahoo.test.ManualClock;
 import com.yahoo.text.Utf8;
 import com.yahoo.vespa.config.server.http.SessionHandlerTest;
 import com.yahoo.vespa.config.server.http.v2.PrepareResult;
 import com.yahoo.vespa.config.server.session.PrepareParams;
-import com.yahoo.vespa.config.server.tenant.Tenant;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
@@ -45,10 +45,11 @@ public class ApplicationRepositoryTest {
     private final static File testAppJdiscOnly = new File("src/test/apps/app-jdisc-only");
     private final static File testAppJdiscOnlyRestart = new File("src/test/apps/app-jdisc-only-restart");
 
-    private final static TenantName tenantName = TenantName.from("test");
+    private final static TenantName tenant1 = TenantName.from("test1");
+    private final static TenantName tenant2 = TenantName.from("test2");
+    private final static TenantName tenant3 = TenantName.from("test3");
     private final static Clock clock = Clock.systemUTC();
 
-    private Tenant tenant;
     private ApplicationRepository applicationRepository;
     private TenantRepository tenantRepository;
     private TimeoutBudget timeoutBudget;
@@ -62,8 +63,9 @@ public class ApplicationRepositoryTest {
         tenantRepository = new TenantRepository(new TestComponentRegistry.Builder()
                                                                 .curator(curator)
                                                                 .build());
-        tenantRepository.addTenant(tenantName);
-        tenant = tenantRepository.getTenant(tenantName);
+        tenantRepository.addTenant(tenant1);
+        tenantRepository.addTenant(tenant2);
+        tenantRepository.addTenant(tenant3);
         Provisioner provisioner = new SessionHandlerTest.MockProvisioner();
         applicationRepository = new ApplicationRepository(tenantRepository, provisioner, clock);
         timeoutBudget = new TimeoutBudget(clock, Duration.ofSeconds(60));
@@ -93,10 +95,25 @@ public class ApplicationRepositoryTest {
 
     @Test
     public void deleteUnusedTenants() {
+        // Set clock to epoch plus hour, as mock curator will always return epoch as creation time
+        Instant now = ManualClock.at("1970-01-01T01:00:00");
+
+        // 3 tenants exist, tenant1 and tenant2 has applications deployed:
         deployApp(testApp);
-        assertTrue(applicationRepository.removeUnusedTenants().isEmpty());
+        deployApp(testApp, new PrepareParams.Builder().applicationId(applicationId(tenant2)).build());
+
+        // Should not be deleted, not old enough
+        Duration ttlForUnusedTenant = Duration.ofHours(1);
+        assertTrue(applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, now).isEmpty());
+        // Should be deleted
+        ttlForUnusedTenant = Duration.ofMillis(1);
+        assertEquals(tenant3, applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, now).iterator().next());
+
+        // Delete app used by tenant1, tenant2 still has an application
         applicationRepository.remove(applicationId());
-        assertEquals(tenantName, applicationRepository.removeUnusedTenants().iterator().next());
+        Set<TenantName> tenantsDeleted = applicationRepository.deleteUnusedTenants(Duration.ofMillis(1), now);
+        assertTrue(tenantsDeleted.contains(tenant1));
+        assertFalse(tenantsDeleted.contains(tenant2));
     }
 
     @Test
@@ -125,8 +142,7 @@ public class ApplicationRepositoryTest {
         // Add file reference that is not in use, but should not be deleted (not older than 14 days)
         File filereferenceDir2 = createFilereferenceOnDisk(new File(fileReferencesDir, "baz"), Instant.now());
 
-        tenantRepository.addTenant(tenantName);
-        tenant = tenantRepository.getTenant(tenantName);
+        tenantRepository.addTenant(tenant1);
         Provisioner provisioner = new SessionHandlerTest.MockProvisioner();
         applicationRepository = new ApplicationRepository(tenantRepository, provisioner, clock);
         timeoutBudget = new TimeoutBudget(clock, Duration.ofSeconds(60));
@@ -158,8 +174,10 @@ public class ApplicationRepositoryTest {
 
     private PrepareResult prepareAndActivateApp(File application) throws IOException {
         FilesApplicationPackage appDir = FilesApplicationPackage.fromFile(application);
-        long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, appDir.getAppDir());
-        return applicationRepository.prepareAndActivate(tenant, sessionId, prepareParams(), false, false, Instant.now());
+        ApplicationId applicationId = applicationId();
+        long sessionId = applicationRepository.createSession(applicationId, timeoutBudget, appDir.getAppDir());
+        return applicationRepository.prepareAndActivate(tenantRepository.getTenant(applicationId.tenant()),
+                                                        sessionId, prepareParams(), false, false, Instant.now());
     }
 
     private PrepareResult deployApp(File applicationPackage) {
@@ -175,6 +193,10 @@ public class ApplicationRepositoryTest {
     }
 
     private ApplicationId applicationId() {
+        return ApplicationId.from(tenant1, ApplicationName.from("testapp"), InstanceName.defaultName());
+    }
+
+    private ApplicationId applicationId(TenantName tenantName) {
         return ApplicationId.from(tenantName, ApplicationName.from("testapp"), InstanceName.defaultName());
     }
 
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
index 0e0c7401e0e..6ba394c340d 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
@@ -21,6 +21,7 @@ import org.apache.curator.framework.recipes.locks.InterProcessMutex;
 import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.framework.state.ConnectionStateListener;
 import org.apache.curator.retry.ExponentialBackoffRetry;
+import org.apache.zookeeper.data.Stat;
 
 import java.time.Duration;
 import java.util.Arrays;
@@ -298,6 +299,21 @@ public class Curator implements AutoCloseable {
         }
     }
 
+    /**
+     * Returns the stat data at the given path.
+     * Empty is returned if the path does not exist.
+     */
+    public Optional<Stat> getStat(Path path) {
+        if ( ! exists(path)) return Optional.empty();
+
+        try {
+            return Optional.of(framework().checkExists().forPath(path.getAbsolute()));
+        }
+        catch (Exception e) {
+            throw new RuntimeException("Could not get data at " + path.getAbsolute(), e);
+        }
+    }
+
     /** Returns the curator framework API */
     public CuratorFramework framework() {
         return curatorFramework;