diff options
author | Martin Polden <mpolden@mpolden.no> | 2022-10-28 11:15:01 +0200 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2022-10-28 11:19:42 +0200 |
commit | 56907fe7dab5bab3ec09d96c5b23e4ba1c4ba0b6 (patch) | |
tree | f9541b0bf4f630d1d7c9c114fe347df6142fc2bf /node-repository | |
parent | 365e3b9780aa02cf9955e384f5eed90e01ee1e86 (diff) |
Never offer hosts in wrong account
Diffstat (limited to 'node-repository')
2 files changed, 49 insertions, 20 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 56038ff1fe9..fe634a77997 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -122,6 +122,7 @@ class NodeAllocation { if ( candidate.state() == Node.State.active && candidate.wantToFail()) continue; // don't accept; causes failing if ( indexes.contains(membership.index())) continue; // duplicate index (just to be sure) if ( requiredHostFlavor.isPresent() && ! candidate.parent.map(node -> node.flavor().name()).equals(requiredHostFlavor)) continue; + if ( candidate.parent.isPresent() && ! candidate.parent.get().cloudAccount().equals(requestedNodes.cloudAccount())) continue; // wrong account boolean resizeable = requestedNodes.considerRetiring() && candidate.isResizable; boolean acceptToRetire = acceptToRetire(candidate); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index bced4daed34..ecc2264cf94 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -44,6 +44,7 @@ import org.junit.Test; import java.time.Duration; import java.time.Instant; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Optional; @@ -437,16 +438,15 @@ public class DynamicProvisioningMaintainerTest { final ApplicationId hostApp; final ApplicationId configSrvApp; switch (hostType) { - case confighost: + case confighost -> { hostApp = new ConfigServerHostApplication().getApplicationId(); configSrvApp = new ConfigServerApplication().getApplicationId(); - break; - case controllerhost: + } + case controllerhost -> { hostApp = new ControllerHostApplication().getApplicationId(); configSrvApp = new ControllerApplication().getApplicationId(); - break; - default: - throw new IllegalArgumentException("Unexpected config server host like node type: " + hostType); + } + default -> throw new IllegalArgumentException("Unexpected config server host like node type: " + hostType); } Cloud cloud = Cloud.builder().dynamicProvisioning(true).build(); @@ -551,23 +551,32 @@ public class DynamicProvisioningMaintainerTest { // Deployment requests capacity in custom account ClusterSpec spec = ProvisioningTester.contentClusterSpec(); ClusterResources resources = new ClusterResources(2, 1, new NodeResources(16, 24, 100, 1)); - CloudAccount cloudAccount = new CloudAccount("012345678912"); - Capacity capacity = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount)); - List<HostSpec> prepared = provisioningTester.prepare(applicationId, spec, capacity); + CloudAccount cloudAccount0 = new CloudAccount("000000000000"); + Capacity capacity0 = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount0)); + List<HostSpec> prepared = provisioningTester.prepare(applicationId, spec, capacity0); // Hosts are provisioned in requested account - tester.maintainer.maintain(); - List<ProvisionedHost> newHosts = tester.hostProvisioner.provisionedHosts(); - assertEquals(2, newHosts.size()); - assertTrue(newHosts.stream().allMatch(host -> host.cloudAccount().equals(cloudAccount))); - for (var host : newHosts) { - provisioningTester.nodeRepository().nodes().setReady(host.hostHostname(), Agent.operator, getClass().getSimpleName()); - } - provisioningTester.prepareAndActivateInfraApplication(DynamicProvisioningTester.tenantHostApp, NodeType.host); - NodeList activeHosts = provisioningTester.nodeRepository().nodes().list(Node.State.active).nodeType(NodeType.host); - assertEquals(2, activeHosts.size()); - assertTrue(activeHosts.stream().allMatch(host -> host.cloudAccount().equals(cloudAccount))); + provisionHostsIn(cloudAccount0, 2, tester); + assertEquals(2, provisioningTester.activate(applicationId, prepared).size()); + NodeList allNodes0 = tester.nodeRepository.nodes().list(); + + // Redeployment in different account provisions a new set of hosts + CloudAccount cloudAccount1 = new CloudAccount("100000000000"); + Capacity capacity1 = Capacity.from(resources, resources, false, true, Optional.of(cloudAccount1)); + prepared = provisioningTester.prepare(applicationId, spec, capacity1); + provisionHostsIn(cloudAccount1, 2, tester); assertEquals(2, provisioningTester.activate(applicationId, prepared).size()); + + // No nodes or hosts are reused + NodeList allNodes1 = tester.nodeRepository.nodes().list(); + NodeList activeNodes0 = allNodes0.state(Node.State.active).owner(applicationId); + NodeList activeNodes1 = allNodes1.state(Node.State.active).owner(applicationId); + assertTrue("New set of nodes is activated", + Collections.disjoint(activeNodes0.asList(), + activeNodes1.asList())); + assertTrue("New set of parents are used", + Collections.disjoint(allNodes0.parentsOf(activeNodes0).asList(), + allNodes1.parentsOf(activeNodes1).asList())); } @Test @@ -627,6 +636,25 @@ public class DynamicProvisioningMaintainerTest { assertEquals(0, tester.nodeRepository.nodes().list().rebuilding(true).size()); } + private void provisionHostsIn(CloudAccount cloudAccount, int count, DynamicProvisioningTester tester) { + tester.maintainer.maintain(); + List<String> provisionedHostnames = tester.hostProvisioner.provisionedHosts().stream() + .filter(host -> host.cloudAccount().equals(cloudAccount)) + .map(ProvisionedHost::hostHostname) + .toList(); + assertEquals(count, provisionedHostnames.size()); + for (var hostname : provisionedHostnames) { + tester.provisioningTester.nodeRepository().nodes().setReady(hostname, Agent.operator, getClass().getSimpleName()); + } + tester.provisioningTester.prepareAndActivateInfraApplication(DynamicProvisioningTester.tenantHostApp, NodeType.host); + NodeList activeHosts = tester.provisioningTester.nodeRepository().nodes() + .list(Node.State.active) + .nodeType(NodeType.host) + .matching(host -> provisionedHostnames.contains(host.hostname())); + assertTrue(activeHosts.stream().allMatch(host -> host.cloudAccount().equals(cloudAccount))); + assertEquals(count, activeHosts.size()); + } + private void assertCfghost3IsActive(DynamicProvisioningTester tester) { assertEquals(5, tester.nodeRepository.nodes().list(Node.State.active).size()); assertEquals(3, tester.nodeRepository.nodes().list(Node.State.active).nodeType(NodeType.confighost).size()); |