From 3af1ae8732a211341a8cfe5fff883af7600acd84 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Sat, 29 Jun 2019 14:33:12 +0200 Subject: Always activate through MockDeployer --- .../hosted/provision/testutils/MockDeployer.java | 19 ++++--- .../OperatorChangeApplicationMaintainerTest.java | 58 +++++++--------------- .../PeriodicApplicationMaintainerTest.java | 50 +++++++------------ 3 files changed, 51 insertions(+), 76 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java index 299dc66c547..e628e823025 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java @@ -15,7 +15,6 @@ import com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -41,7 +40,7 @@ public class MockDeployer implements Deployer { @Inject @SuppressWarnings("unused") public MockDeployer() { - this(null, Clock.systemUTC(), Collections.emptyMap()); + this(null, Clock.systemUTC(), Map.of()); } /** @@ -53,7 +52,7 @@ public class MockDeployer implements Deployer { Map applications) { this.provisioner = provisioner; this.clock = clock; - this.applications = applications; + this.applications = new HashMap<>(applications); } public ReentrantLock lock() { @@ -73,8 +72,8 @@ public class MockDeployer implements Deployer { throw new RuntimeException(e); } try { - lastDeployTimes.put(id, clock.instant()); - return Optional.of(new MockDeployment(provisioner, applications.get(id))); + return Optional.ofNullable(applications.get(id)) + .map(application -> new MockDeployment(provisioner, application)); } finally { lock.unlock(); } @@ -90,6 +89,13 @@ public class MockDeployer implements Deployer { return Optional.ofNullable(lastDeployTimes.get(application)); } + public void removeApplication(ApplicationId applicationId) { + new MockDeployment(provisioner, new ApplicationContext(applicationId, List.of())).activate(); + + applications.remove(applicationId); + lastDeployTimes.remove(applicationId); + } + public class MockDeployment implements Deployment { private final NodeRepositoryProvisioner provisioner; @@ -116,6 +122,7 @@ public class MockDeployer implements Deployer { try (NestedTransaction t = new NestedTransaction()) { provisioner.activate(t, application.id(), preparedHosts); t.commit(); + lastDeployTimes.put(application.id, clock.instant()); } } @@ -136,7 +143,7 @@ public class MockDeployer implements Deployer { } public ApplicationContext(ApplicationId id, ClusterSpec cluster, Capacity capacity, int groups) { - this(id, Collections.singletonList(new ClusterContext(id, cluster, capacity, groups))); + this(id, List.of(new ClusterContext(id, cluster, capacity, groups))); } public ApplicationId id() { return id; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java index 5979ed37a98..e1ac0430ee4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java @@ -9,7 +9,6 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; -import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeResources; @@ -18,10 +17,8 @@ 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.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; -import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; @@ -36,7 +33,6 @@ import org.junit.Test; import java.time.Duration; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -62,33 +58,34 @@ public class OperatorChangeApplicationMaintainerTest { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true); - this.fixture = new Fixture(zone, nodeRepository, nodeFlavors, curator); + this.fixture = new Fixture(zone, nodeRepository, nodeFlavors); createReadyNodes(15, this.fixture.nodeResources, nodeRepository); createHostNodes(2, nodeRepository, nodeFlavors); // Create applications fixture.activate(); - OperatorChangeApplicationMaintainer maintainer = new OperatorChangeApplicationMaintainer(fixture.deployer, nodeRepository, clock, Duration.ofMinutes(1)); + assertEquals("Initial applications are deployed", 2, fixture.deployer.redeployments); + OperatorChangeApplicationMaintainer maintainer = new OperatorChangeApplicationMaintainer(fixture.deployer, nodeRepository, Duration.ofMinutes(1)); clock.advance(Duration.ofMinutes(2)); maintainer.maintain(); - assertEquals("No changes -> no redeployments", 0, fixture.deployer.redeployments); + assertEquals("No changes -> no redeployments", 2, fixture.deployer.redeployments); nodeRepository.fail(nodeRepository.getNodes(fixture.app1).get(3).hostname(), Agent.system, "Failing to unit test"); clock.advance(Duration.ofMinutes(2)); maintainer.maintain(); - assertEquals("System change -> no redeployments", 0, fixture.deployer.redeployments); + assertEquals("System change -> no redeployments", 2, fixture.deployer.redeployments); clock.advance(Duration.ofSeconds(1)); nodeRepository.fail(nodeRepository.getNodes(fixture.app2).get(4).hostname(), Agent.operator, "Manual node failing"); clock.advance(Duration.ofMinutes(2)); maintainer.maintain(); - assertEquals("Operator change -> redeployment", 1, fixture.deployer.redeployments); + assertEquals("Operator change -> redeployment", 3, fixture.deployer.redeployments); clock.advance(Duration.ofMinutes(2)); maintainer.maintain(); - assertEquals("No further operator changes -> no (new) redeployments", 1, fixture.deployer.redeployments); + assertEquals("No further operator changes -> no (new) redeployments", 3, fixture.deployer.redeployments); } private void createReadyNodes(int count, NodeRepository nodeRepository, NodeFlavors nodeFlavors) { @@ -118,50 +115,33 @@ public class OperatorChangeApplicationMaintainerTest { private class Fixture { - final NodeResources nodeResources = new NodeResources(2, 8, 50); final NodeRepository nodeRepository; - final NodeRepositoryProvisioner provisioner; - final Curator curator; + final MockDeployer deployer; + final NodeResources nodeResources = new NodeResources(2, 8, 50); final ApplicationId app1 = ApplicationId.from(TenantName.from("foo1"), ApplicationName.from("bar"), InstanceName.from("fuz")); final ApplicationId app2 = ApplicationId.from(TenantName.from("foo2"), ApplicationName.from("bar"), InstanceName.from("fuz")); final ClusterSpec clusterApp1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("test"), Version.fromString("6.42"), false); final ClusterSpec clusterApp2 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("test"), Version.fromString("6.42"), false); final int wantedNodesApp1 = 5; final int wantedNodesApp2 = 7; - MockDeployer deployer; // created on activation - Fixture(Zone zone, NodeRepository nodeRepository, NodeFlavors flavors, Curator curator) { + Fixture(Zone zone, NodeRepository nodeRepository, NodeFlavors flavors) { this.nodeRepository = nodeRepository; - this.curator = curator; - this.provisioner = new NodeRepositoryProvisioner(nodeRepository, flavors, zone, new MockProvisionServiceProvider(), new InMemoryFlagSource()); + NodeRepositoryProvisioner provisioner = new NodeRepositoryProvisioner( + nodeRepository, flavors, zone, new MockProvisionServiceProvider(), new InMemoryFlagSource()); + + Map apps = Map.of( + app1, new MockDeployer.ApplicationContext(app1, clusterApp1, Capacity.fromCount(wantedNodesApp1, nodeResources), 1), + app2, new MockDeployer.ApplicationContext(app2, clusterApp2, Capacity.fromCount(wantedNodesApp2, nodeResources), 1)); + this.deployer = new MockDeployer(provisioner, nodeRepository.clock(), apps); } void activate() { - activate(app1, clusterApp1, wantedNodesApp1, provisioner); - activate(app2, clusterApp2, wantedNodesApp2, provisioner); + deployer.deployFromLocalActive(app1, false).get().activate(); + deployer.deployFromLocalActive(app2, false).get().activate(); assertEquals(wantedNodesApp1, nodeRepository.getNodes(app1, Node.State.active).size()); assertEquals(wantedNodesApp2, nodeRepository.getNodes(app2, Node.State.active).size()); - - Map apps = new HashMap<>(); - apps.put(app1, new MockDeployer.ApplicationContext(app1, clusterApp1, - Capacity.fromCount(wantedNodesApp1, nodeResources), 1)); - apps.put(app2, new MockDeployer.ApplicationContext(app2, clusterApp2, - Capacity.fromCount(wantedNodesApp2, nodeResources), 1)); - this.deployer = new MockDeployer(provisioner, nodeRepository.clock(), apps); - } - - private void activate(ApplicationId applicationId, ClusterSpec cluster, int nodeCount, NodeRepositoryProvisioner provisioner) { - List hosts = provisioner.prepare(applicationId, cluster, Capacity.fromCount(nodeCount, nodeResources), 1, null); - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.activate(transaction, applicationId, hosts); - transaction.commit(); - } - - void remove(ApplicationId application) { - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.remove(transaction, application); - transaction.commit(); } NodeList getNodes(Node.State ... states) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java index dff18912288..0923d8df71a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java @@ -10,7 +10,6 @@ import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; -import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeResources; @@ -19,10 +18,8 @@ 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.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; -import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; @@ -46,7 +43,6 @@ import java.util.Map; import java.util.Optional; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; /** * @author bratseth @@ -68,7 +64,7 @@ public class PeriodicApplicationMaintainerTest { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true); - this.fixture = new Fixture(zone, nodeRepository, nodeFlavors, curator); + this.fixture = new Fixture(zone, nodeRepository, nodeFlavors); createReadyNodes(15, fixture.nodeResources, nodeRepository); createHostNodes(2, nodeRepository, nodeFlavors); @@ -149,27 +145,32 @@ public class PeriodicApplicationMaintainerTest { public void application_deploy_inhibits_redeploy_for_a_while() { fixture.activate(); + assertEquals("No deployment expected", 2, fixture.deployer.redeployments); + // Holds off on deployments a while after starting fixture.runApplicationMaintainer(); - assertFalse("No deployment expected", fixture.deployer.lastDeployTime(fixture.app1).isPresent()); - assertFalse("No deployment expected", fixture.deployer.lastDeployTime(fixture.app2).isPresent()); + assertEquals("No deployment expected", 2, fixture.deployer.redeployments); + // Exhaust initial wait period clock.advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1))); // First deployment of applications fixture.runApplicationMaintainer(); + assertEquals("No deployment expected", 4, fixture.deployer.redeployments); Instant firstDeployTime = clock.instant(); assertEquals(firstDeployTime, fixture.deployer.lastDeployTime(fixture.app1).get()); assertEquals(firstDeployTime, fixture.deployer.lastDeployTime(fixture.app2).get()); clock.advance(Duration.ofMinutes(5)); fixture.runApplicationMaintainer(); // Too soon: Not redeployed: + assertEquals("No deployment expected", 4, fixture.deployer.redeployments); assertEquals(firstDeployTime, fixture.deployer.lastDeployTime(fixture.app1).get()); assertEquals(firstDeployTime, fixture.deployer.lastDeployTime(fixture.app2).get()); clock.advance(Duration.ofMinutes(30)); fixture.runApplicationMaintainer(); // Redeployed: + assertEquals("No deployment expected", 6, fixture.deployer.redeployments); assertEquals(clock.instant(), fixture.deployer.lastDeployTime(fixture.app1).get()); assertEquals(clock.instant(), fixture.deployer.lastDeployTime(fixture.app2).get()); } @@ -238,12 +239,10 @@ public class PeriodicApplicationMaintainerTest { private class Fixture { - final NodeResources nodeResources = new NodeResources(2, 8, 50); final NodeRepository nodeRepository; - final NodeRepositoryProvisioner provisioner; - final Curator curator; final MockDeployer deployer; + final NodeResources nodeResources = new NodeResources(2, 8, 50); final ApplicationId app1 = ApplicationId.from(TenantName.from("foo1"), ApplicationName.from("bar"), InstanceName.from("fuz")); final ApplicationId app2 = ApplicationId.from(TenantName.from("foo2"), ApplicationName.from("bar"), InstanceName.from("fuz")); final ClusterSpec clusterApp1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("test"), Version.fromString("6.42"), false); @@ -253,39 +252,28 @@ public class PeriodicApplicationMaintainerTest { private final TestablePeriodicApplicationMaintainer maintainer; - Fixture(Zone zone, NodeRepository nodeRepository, NodeFlavors flavors, Curator curator) { + Fixture(Zone zone, NodeRepository nodeRepository, NodeFlavors flavors) { this.nodeRepository = nodeRepository; - this.curator = curator; - this.provisioner = new NodeRepositoryProvisioner(nodeRepository, flavors, zone, new MockProvisionServiceProvider(), new InMemoryFlagSource()); - - Map apps = new HashMap<>(); - apps.put(app1, new MockDeployer.ApplicationContext(app1, clusterApp1, - Capacity.fromCount(wantedNodesApp1, nodeResources), 1)); - apps.put(app2, new MockDeployer.ApplicationContext(app2, clusterApp2, - Capacity.fromCount(wantedNodesApp2, nodeResources), 1)); + NodeRepositoryProvisioner provisioner = new NodeRepositoryProvisioner( + nodeRepository, flavors, zone, new MockProvisionServiceProvider(), new InMemoryFlagSource()); + + Map apps = Map.of( + app1, new MockDeployer.ApplicationContext(app1, clusterApp1, Capacity.fromCount(wantedNodesApp1, nodeResources), 1), + app2, new MockDeployer.ApplicationContext(app2, clusterApp2, Capacity.fromCount(wantedNodesApp2, nodeResources), 1)); this.deployer = new MockDeployer(provisioner, nodeRepository.clock(), apps); this.maintainer = new TestablePeriodicApplicationMaintainer(deployer, nodeRepository, Duration.ofDays(1), // Long duration to prevent scheduled runs during test Duration.ofMinutes(30)); } void activate() { - activate(app1, clusterApp1, wantedNodesApp1, provisioner); - activate(app2, clusterApp2, wantedNodesApp2, provisioner); + deployer.deployFromLocalActive(app1, false).get().activate(); + deployer.deployFromLocalActive(app2, false).get().activate(); assertEquals(wantedNodesApp1, nodeRepository.getNodes(app1, Node.State.active).size()); assertEquals(wantedNodesApp2, nodeRepository.getNodes(app2, Node.State.active).size()); } - private void activate(ApplicationId applicationId, ClusterSpec cluster, int nodeCount, NodeRepositoryProvisioner provisioner) { - List hosts = provisioner.prepare(applicationId, cluster, Capacity.fromCount(nodeCount, nodeResources), 1, null); - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.activate(transaction, applicationId, hosts); - transaction.commit(); - } - void remove(ApplicationId application) { - NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator)); - provisioner.remove(transaction, application); - transaction.commit(); + deployer.removeApplication(application); } void runApplicationMaintainer() { -- cgit v1.2.3