From 43c926d71227997373e52652b9de3a4b6e537d72 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 24 Jun 2020 09:56:33 +0200 Subject: Use clock correctly and remove duplicate test --- .../server/maintenance/TenantsMaintainer.java | 16 ++++++------- .../config/server/ApplicationRepositoryTest.java | 28 ++-------------------- .../server/maintenance/MaintainerTester.java | 9 +++++-- .../server/maintenance/TenantsMaintainerTest.java | 8 +++++-- 4 files changed, 23 insertions(+), 38 deletions(-) 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 b3c67859098..d3865f287cd 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 @@ -4,8 +4,8 @@ package com.yahoo.vespa.config.server.maintenance; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.curator.Curator; +import java.time.Clock; import java.time.Duration; -import java.time.Instant; /** * Removes unused tenants (has no applications and was created more than 7 days ago) @@ -14,19 +14,19 @@ import java.time.Instant; */ public class TenantsMaintainer extends ConfigServerMaintainer { - private final Duration ttlForUnusedTenant; + static final Duration defaultTtlForUnusedTenant = Duration.ofDays(7); - TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval) { - this(applicationRepository, curator, interval, Duration.ofDays(7)); - } + private final Duration ttlForUnusedTenant; + private final Clock clock; - private TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, Duration ttlForUnusedTenant) { + TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, Clock clock) { super(applicationRepository, curator, interval, interval); - this.ttlForUnusedTenant = ttlForUnusedTenant; + this.ttlForUnusedTenant = defaultTtlForUnusedTenant; + this.clock = clock; } @Override protected void maintain() { - applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, Instant.now()); + applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, clock.instant()); } } 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 8745a9bf596..9781d12991a 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 @@ -63,7 +63,6 @@ import org.junit.rules.TemporaryFolder; import java.io.File; import java.io.IOException; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -101,7 +100,7 @@ public class ApplicationRepositoryTest { 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 final static ManualClock clock = new ManualClock(Instant.now()); private ApplicationRepository applicationRepository; private TenantRepository tenantRepository; @@ -133,6 +132,7 @@ public class ApplicationRepositoryTest { .fileReferencesDir(temporaryFolder.newFolder().getAbsolutePath()) .build()) .flagSource(flagSource) + .clock(clock) .build(); tenantRepository = new TenantRepository(componentRegistry, false); tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT); @@ -244,29 +244,6 @@ public class ApplicationRepositoryTest { assertEquals(200, response.getStatus()); } - @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); - 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.delete(applicationId()); - Set tenantsDeleted = applicationRepository.deleteUnusedTenants(Duration.ofMillis(1), now); - assertTrue(tenantsDeleted.contains(tenant1)); - assertFalse(tenantsDeleted.contains(tenant2)); - } - @Test public void deleteUnusedFileReferences() throws IOException { File fileReferencesDir = temporaryFolder.newFolder(); @@ -372,7 +349,6 @@ public class ApplicationRepositoryTest { @Test public void testDeletingInactiveSessions() throws IOException { - ManualClock clock = new ManualClock(Instant.now()); ConfigserverConfig configserverConfig = new ConfigserverConfig(new ConfigserverConfig.Builder() .configServerDBDir(temporaryFolder.newFolder("serverdb").getAbsolutePath()) diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java index c2489a1d3e9..4b37638cb50 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java @@ -10,15 +10,20 @@ import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; +import java.time.Clock; + class MaintainerTester { private final Curator curator; private final TenantRepository tenantRepository; private final ApplicationRepository applicationRepository; - MaintainerTester() { + MaintainerTester(Clock clock) { curator = new MockCurator(); - GlobalComponentRegistry componentRegistry = new TestComponentRegistry.Builder().curator(curator).build(); + GlobalComponentRegistry componentRegistry = new TestComponentRegistry.Builder() + .curator(curator) + .clock(clock) + .build(); tenantRepository = new TenantRepository(componentRegistry, false); applicationRepository = new ApplicationRepository(tenantRepository, new SessionHandlerTest.MockProvisioner(), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java index fdc6ffeacf0..53326a89293 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; +import com.yahoo.test.ManualClock; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.config.server.tenant.TenantRepository; @@ -20,7 +21,8 @@ public class TenantsMaintainerTest { @Test public void deleteTenantWithNoApplications() { - MaintainerTester tester = new MaintainerTester(); + ManualClock clock = new ManualClock("2020-06-01T00:00:00"); + MaintainerTester tester = new MaintainerTester(clock); TenantRepository tenantRepository = tester.tenantRepository(); ApplicationRepository applicationRepository = tester.applicationRepository(); File applicationPackage = new File("src/test/apps/app"); @@ -36,7 +38,8 @@ public class TenantsMaintainerTest { assertNotNull(tenantRepository.getTenant(shouldBeDeleted)); assertNotNull(tenantRepository.getTenant(shouldNotBeDeleted)); - new TenantsMaintainer(applicationRepository, tester.curator(), Duration.ofDays(1)).run(); + clock.advance(TenantsMaintainer.defaultTtlForUnusedTenant.plus(Duration.ofDays(1))); + new TenantsMaintainer(applicationRepository, tester.curator(), Duration.ofDays(1), clock).run(); tenantRepository.updateTenants(); // One tenant should now have been deleted @@ -59,4 +62,5 @@ public class TenantsMaintainerTest { private ApplicationId applicationId(TenantName tenantName) { return ApplicationId.from(tenantName, ApplicationName.from("foo"), InstanceName.defaultName()); } + } -- cgit v1.2.3