From 877d4c1ea464b3e6495cf3cea33813c5dc942d5e Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Tue, 26 Sep 2017 11:51:21 +0200 Subject: Refactor ControllerTester --- .../vespa/hosted/controller/ControllerTest.java | 14 +-- .../vespa/hosted/controller/ControllerTester.java | 127 ++++++++++++--------- .../controller/deployment/DeploymentTester.java | 36 ++++-- .../maintenance/DeploymentExpirerTest.java | 2 +- .../maintenance/FailureRedeployerTest.java | 11 +- .../controller/maintenance/UpgraderTest.java | 6 +- .../controller/versions/VersionStatusTest.java | 8 +- 7 files changed, 122 insertions(+), 82 deletions(-) (limited to 'controller-server') 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 3ab98d31a82..91fca0d37d1 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 @@ -217,7 +217,7 @@ public class ControllerTest { app1 = applications.require(app1.id()); assertEquals("First deployment gets system version", systemVersion, app1.deployedVersion().get()); - assertEquals(systemVersion, tester.configServerClientMock().lastPrepareVersion.get()); + assertEquals(systemVersion, tester.configServer().lastPrepareVersion.get()); // Unexpected deployment tester.deploy(productionUsWest1, app1, applicationPackage); @@ -240,7 +240,7 @@ public class ControllerTest { app1 = applications.require(app1.id()); assertEquals("Application change preserves version", systemVersion, app1.deployedVersion().get()); - assertEquals(systemVersion, tester.configServerClientMock().lastPrepareVersion.get()); + assertEquals(systemVersion, tester.configServer().lastPrepareVersion.get()); // A deployment to the new region gets the same version applicationPackage = new ApplicationPackageBuilder() @@ -251,7 +251,7 @@ public class ControllerTest { tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3); app1 = applications.require(app1.id()); assertEquals("Application change preserves version", systemVersion, app1.deployedVersion().get()); - assertEquals(systemVersion, tester.configServerClientMock().lastPrepareVersion.get()); + assertEquals(systemVersion, tester.configServer().lastPrepareVersion.get()); // Version upgrade changes system version Change.VersionChange change = new Change.VersionChange(newSystemVersion); @@ -263,7 +263,7 @@ public class ControllerTest { app1 = applications.require(app1.id()); assertEquals("Version upgrade changes version", newSystemVersion, app1.deployedVersion().get()); - assertEquals(newSystemVersion, tester.configServerClientMock().lastPrepareVersion.get()); + assertEquals(newSystemVersion, tester.configServer().lastPrepareVersion.get()); } /** Adds a new version, higher than the current system version, makes it the system version and returns it */ @@ -600,14 +600,14 @@ public class ControllerTest { @Test public void testCleanupOfStaleDeploymentData() throws IOException { DeploymentTester tester = new DeploymentTester(); - tester.controllerTester().getZoneRegistryMock().setSystem(SystemName.cd); + tester.controllerTester().zoneRegistry().setSystem(SystemName.cd); Supplier> statuses = () -> tester.application(ApplicationId.from("vespa", "canary", "default")).deploymentJobs().jobStatus(); // Current system version, matches version in test data Version version = Version.fromString("6.141.117"); - tester.configServerClientMock().setDefaultConfigServerVersion(version); + tester.configServer().setDefaultConfigServerVersion(version); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); @@ -637,7 +637,7 @@ public class ControllerTest { // New version is released version = Version.fromString("6.142.1"); - tester.configServerClientMock().setDefaultConfigServerVersion(version); + tester.configServer().setDefaultConfigServerVersion(version); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); tester.upgrader().maintain(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 63feb01f1f2..b5419bca0a5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -9,7 +9,6 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.Tenant; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.GitRevision; @@ -22,6 +21,8 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; +import com.yahoo.vespa.hosted.controller.api.integration.athens.mock.AthensDbMock; +import com.yahoo.vespa.hosted.controller.api.integration.athens.mock.AthensMock; import com.yahoo.vespa.hosted.controller.api.integration.chef.ChefMock; import com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService; import com.yahoo.vespa.hosted.controller.api.integration.entity.MemoryEntityService; @@ -29,7 +30,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.github.GitHubMock; import com.yahoo.vespa.hosted.controller.api.integration.jira.JiraMock; import com.yahoo.vespa.hosted.controller.api.integration.routing.MemoryGlobalRoutingService; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.cost.CostMock; import com.yahoo.vespa.hosted.controller.cost.MockInsightBackend; import com.yahoo.vespa.hosted.controller.integration.MockMetricsService; @@ -40,8 +40,6 @@ import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.routing.MockRoutingGenerator; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.rotation.MemoryRotationRepository; -import com.yahoo.vespa.hosted.controller.api.integration.athens.mock.AthensMock; -import com.yahoo.vespa.hosted.controller.api.integration.athens.mock.AthensDbMock; import java.util.Optional; @@ -50,72 +48,69 @@ import static org.junit.Assert.assertTrue; /** * Convenience methods for controller tests. - * This completely wraps TestEnvironment to make it easier to get rid of that in the future. - * + * * @author bratseth + * @author mpolden */ public final class ControllerTester { - private final ControllerDb db = new MemoryControllerDb(); - private final AthensDbMock athensDb = new AthensDbMock(); - private final ManualClock clock = new ManualClock(); - private final ConfigServerClientMock configServerClientMock = new ConfigServerClientMock(); - private final ZoneRegistryMock zoneRegistryMock = new ZoneRegistryMock(); - private final GitHubMock gitHubMock = new GitHubMock(); - private final CuratorDb curator = new MockCuratorDb(); - private final MemoryNameService memoryNameService = new MemoryNameService(); - private Controller controller = createController(db, curator, configServerClientMock, clock, gitHubMock, - zoneRegistryMock, athensDb, memoryNameService); - - private static final Controller createController(ControllerDb db, CuratorDb curator, - ConfigServerClientMock configServerClientMock, ManualClock clock, - GitHubMock gitHubClientMock, ZoneRegistryMock zoneRegistryMock, - AthensDbMock athensDb, MemoryNameService nameService) { - Controller controller = new Controller(db, - curator, - new MemoryRotationRepository(), - gitHubClientMock, - new JiraMock(), - new MemoryEntityService(), - new MemoryGlobalRoutingService(), - zoneRegistryMock, - new CostMock(new MockInsightBackend()), - configServerClientMock, - new MockMetricsService(), - nameService, - new MockRoutingGenerator(), - new ChefMock(), - clock, - new AthensMock(athensDb)); - controller.updateVersionStatus(VersionStatus.compute(controller)); - return controller; + private final ControllerDb db; + private final AthensDbMock athensDb; + private final ManualClock clock; + private final ConfigServerClientMock configServer; + private final ZoneRegistryMock zoneRegistry; + private final GitHubMock gitHub; + private final CuratorDb curator; + private final MemoryNameService nameService; + + private Controller controller; + + public ControllerTester() { + this(new MemoryControllerDb(), new AthensDbMock(), new ManualClock(), new ConfigServerClientMock(), + new ZoneRegistryMock(), new GitHubMock(), new MockCuratorDb(), new MemoryNameService()); + } + + public ControllerTester(ManualClock clock) { + this(new MemoryControllerDb(), new AthensDbMock(), clock, new ConfigServerClientMock(), + new ZoneRegistryMock(), new GitHubMock(), new MockCuratorDb(), new MemoryNameService()); + } + + private ControllerTester(ControllerDb db, AthensDbMock athensDb, ManualClock clock, + ConfigServerClientMock configServer, ZoneRegistryMock zoneRegistry, + GitHubMock gitHub, CuratorDb curator, MemoryNameService nameService) { + this.db = db; + this.athensDb = athensDb; + this.clock = clock; + this.configServer = configServer; + this.zoneRegistry = zoneRegistry; + this.gitHub = gitHub; + this.curator = curator; + this.nameService = nameService; + this.controller = createController(db, curator, configServer, clock, gitHub, zoneRegistry, + athensDb, nameService); } public Controller controller() { return controller; } + public CuratorDb curator() { return curator; } + public ManualClock clock() { return clock; } + public AthensDbMock athensDb() { return athensDb; } - public MemoryNameService nameService() { return memoryNameService; } - /** Create a new controller instance. Useful to verify that controller state is rebuilt from persistence */ - public final void createNewController() { - controller = createController(db, curator, configServerClientMock, clock, gitHubMock, zoneRegistryMock, - athensDb, memoryNameService); - } + public MemoryNameService nameService() { return nameService; } - public ZoneRegistryMock getZoneRegistryMock() { return zoneRegistryMock; } + public ZoneRegistryMock zoneRegistry() { return zoneRegistry; } - public ConfigServerClientMock configServerClientMock() { return configServerClientMock; } + public ConfigServerClientMock configServer() { return configServer; } - public GitHubMock gitHubClientMock () { return gitHubMock; } + public GitHubMock gitHub() { return gitHub; } - /** Set the application with the given id to currently be in the progress of rolling out the given change */ - public void setDeploying(ApplicationId id, Optional change) { - try (Lock lock = controller.applications().lock(id)) { - controller.applications().store(controller.applications().require(id).withDeploying(change), lock); - } + /** Create a new controller instance. Useful to verify that controller state is rebuilt from persistence */ + public final void createNewController() { + controller = createController(db, curator, configServer, clock, gitHub, zoneRegistry, athensDb, nameService); } - + /** Creates the given tenant and application and deploys it */ public Application createAndDeploy(String tenantName, String domainName, String applicationName, Environment environment, long projectId, Long propertyId) { return createAndDeploy(tenantName, domainName, applicationName, toZone(environment), projectId, propertyId); @@ -203,4 +198,28 @@ public final class ControllerTester { InstanceName.from(instance)); } + private static Controller createController(ControllerDb db, CuratorDb curator, + ConfigServerClientMock configServerClientMock, ManualClock clock, + GitHubMock gitHubClientMock, ZoneRegistryMock zoneRegistryMock, + AthensDbMock athensDb, MemoryNameService nameService) { + Controller controller = new Controller(db, + curator, + new MemoryRotationRepository(), + gitHubClientMock, + new JiraMock(), + new MemoryEntityService(), + new MemoryGlobalRoutingService(), + zoneRegistryMock, + new CostMock(new MockInsightBackend()), + configServerClientMock, + new MockMetricsService(), + nameService, + new MockRoutingGenerator(), + new ChefMock(), + clock, + new AthensMock(athensDb)); + controller.updateVersionStatus(VersionStatus.compute(controller)); + return controller; + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 3edbcea9843..44309b43a5f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -30,26 +30,48 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; /** + * This class provides convenience methods for testing deployments + * * @author bratseth + * @author mpolden */ public class DeploymentTester { - private ControllerTester tester = new ControllerTester(); - - private Upgrader upgrader = new Upgrader(tester.controller(), Duration.ofMinutes(2), - new JobControl(tester.curator())); - private FailureRedeployer failureRedeployer = new FailureRedeployer(tester.controller(), Duration.ofMinutes(2), - new JobControl(tester.curator())); + private final ControllerTester tester; + private final Upgrader upgrader; + private final FailureRedeployer failureRedeployer; + + public DeploymentTester() { + this(new ControllerTester()); + } + + public DeploymentTester(ControllerTester tester) { + this.tester = tester; + this.upgrader = new Upgrader(tester.controller(), Duration.ofMinutes(2), + new JobControl(tester.curator())); + this.failureRedeployer = new FailureRedeployer(tester.controller(), + Duration.ofMinutes(2), + new JobControl(tester.curator())); + } public Upgrader upgrader() { return upgrader; } + public FailureRedeployer failureRedeployer() { return failureRedeployer; } + public Controller controller() { return tester.controller(); } + public ApplicationController applications() { return tester.controller().applications(); } + public BuildSystem buildSystem() { return tester.controller().applications().deploymentTrigger().buildSystem(); } + public DeploymentTrigger deploymentTrigger() { return tester.controller().applications().deploymentTrigger(); } + public ManualClock clock() { return tester.clock(); } + public ControllerTester controllerTester() { return tester; } + public ConfigServerClientMock configServer() { return tester.configServer(); } + public Application application(String name) { return application(ApplicationId.from("tenant1", name, "default")); } @@ -63,8 +85,6 @@ public class DeploymentTester { .filter(c -> c instanceof Change.VersionChange) .map(Change.VersionChange.class::cast); } - - public ConfigServerClientMock configServerClientMock() { return tester.configServerClientMock(); } public void updateVersionStatus(Version currentVersion) { controller().updateVersionStatus(VersionStatus.compute(controller(), currentVersion)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java index 4c53a6d37e4..ee4f3631b54 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java @@ -22,7 +22,7 @@ public class DeploymentExpirerTest { @Test public void testDeploymentExpiry() throws IOException, InterruptedException { ControllerTester tester = new ControllerTester(); - tester.getZoneRegistryMock().setDeploymentTimeToLive(new Zone(Environment.dev, RegionName.from("us-east-1")), Duration.ofDays(14)); + tester.zoneRegistry().setDeploymentTimeToLive(new Zone(Environment.dev, RegionName.from("us-east-1")), Duration.ofDays(14)); DeploymentExpirer expirer = new DeploymentExpirer(tester.controller(), Duration.ofDays(10), tester.clock(), new JobControl(new MockCuratorDb())); ApplicationId devApp = tester.createAndDeploy("tenant1", "domain1", "app1", Environment.dev, 123).id(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java index f5953889f31..2d1b7463dea 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java @@ -243,11 +243,11 @@ public class FailureRedeployerTest { @Test public void retryIgnoresStaleJobData() throws Exception { DeploymentTester tester = new DeploymentTester(); - tester.controllerTester().getZoneRegistryMock().setSystem(SystemName.cd); + tester.controllerTester().zoneRegistry().setSystem(SystemName.cd); // Current system version, matches version in test data Version version = Version.fromString("6.141.117"); - tester.configServerClientMock().setDefaultConfigServerVersion(version); + tester.configServer().setDefaultConfigServerVersion(version); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); @@ -266,7 +266,7 @@ public class FailureRedeployerTest { // New version is released version = Version.fromString("6.142.1"); - tester.configServerClientMock().setDefaultConfigServerVersion(version); + tester.configServer().setDefaultConfigServerVersion(version); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); tester.upgrader().maintain(); @@ -299,10 +299,11 @@ public class FailureRedeployerTest { @Test public void ignoresPullRequestInstances() throws Exception { DeploymentTester tester = new DeploymentTester(); + tester.controllerTester().zoneRegistry().setSystem(SystemName.cd); // Current system version, matches version in test data Version version = Version.fromString("6.42.1"); - tester.configServerClientMock().setDefaultConfigServerVersion(version); + tester.configServer().setDefaultConfigServerVersion(version); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); @@ -327,7 +328,7 @@ public class FailureRedeployerTest { // Current system version, matches version in test data Version version = Version.fromString("6.42.1"); - tester.configServerClientMock().setDefaultConfigServerVersion(version); + tester.configServer().setDefaultConfigServerVersion(version); tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index 3046a89efe6..bffd3060dfb 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -54,7 +54,7 @@ public class UpgraderTest { assertEquals("New system version: Should upgrade Canaries", 2, tester.buildSystem().jobs().size()); tester.completeUpgrade(canary0, version, "canary"); - assertEquals(version, tester.configServerClientMock().lastPrepareVersion.get()); + assertEquals(version, tester.configServer().lastPrepareVersion.get()); tester.updateVersionStatus(version); tester.upgrader().maintain(); @@ -104,7 +104,7 @@ public class UpgraderTest { assertEquals("New system version: Should upgrade Canaries", 2, tester.buildSystem().jobs().size()); tester.completeUpgrade(canary0, version, "canary"); - assertEquals(version, tester.configServerClientMock().lastPrepareVersion.get()); + assertEquals(version, tester.configServer().lastPrepareVersion.get()); tester.updateVersionStatus(version); tester.upgrader().maintain(); @@ -184,7 +184,7 @@ public class UpgraderTest { assertEquals("New system version: Should upgrade Canaries", 2, tester.buildSystem().jobs().size()); tester.completeUpgrade(canary0, version, "canary"); - assertEquals(version, tester.configServerClientMock().lastPrepareVersion.get()); + assertEquals(version, tester.configServer().lastPrepareVersion.get()); tester.updateVersionStatus(version); tester.upgrader().maintain(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index a2fb5f38457..c4a3bd9cd81 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -46,7 +46,7 @@ public class VersionStatusTest { public void testSystemVersionIsControllerVersionIfConfigserversAreNewer() { ControllerTester tester = new ControllerTester(); Version largerThanCurrent = new Version(Vtag.currentVersion.getMajor() + 1); - tester.configServerClientMock().setDefaultConfigServerVersion(largerThanCurrent); + tester.configServer().setDefaultConfigServerVersion(largerThanCurrent); VersionStatus versionStatus = VersionStatus.compute(tester.controller()); assertEquals(Vtag.currentVersion, versionStatus.systemVersion().get().versionNumber()); } @@ -55,7 +55,7 @@ public class VersionStatusTest { public void testSystemVersionIsVersionOfOldestConfigServer() throws URISyntaxException { ControllerTester tester = new ControllerTester(); Version oldest = new Version(5); - tester.configServerClientMock().configServerVersions().put(new URI("http://cfg.prod.corp-us-east-1.test"), oldest); + tester.configServer().configServerVersions().put(new URI("http://cfg.prod.corp-us-east-1.test"), oldest); VersionStatus versionStatus = VersionStatus.compute(tester.controller()); assertEquals(oldest, versionStatus.systemVersion().get().versionNumber()); } @@ -95,7 +95,7 @@ public class VersionStatusTest { assertEquals("The version of this controller, the default config server version, plus the two versions above exist", 4, versions.size()); VespaVersion v0 = versions.get(2); - assertEquals(tester.configServerClientMock().getDefaultConfigServerVersion(), v0.versionNumber()); + assertEquals(tester.configServer().getDefaultConfigServerVersion(), v0.versionNumber()); assertEquals(0, v0.statistics().failing().size()); assertEquals(0, v0.statistics().production().size()); @@ -242,7 +242,7 @@ public class VersionStatusTest { ControllerTester tester = new ControllerTester(); ApplicationController applications = tester.controller().applications(); - tester.gitHubClientMock() + tester.gitHub() .mockAny(false) .knownTag(Vtag.currentVersion.toFullString(), "foo") // controller .knownTag("6.1.0", "bar"); // config server -- cgit v1.2.3