diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2023-09-07 14:00:38 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-07 14:00:38 +0200 |
commit | 4068b940848b9604889c05a4a8db1cc10719e2ad (patch) | |
tree | c32bdc24504d7f4f459a09b04be83b278c5e720a /node-repository | |
parent | b69cc1c8b2f894fe946bcbd235023da88a6d9e39 (diff) | |
parent | 7431e1acaa1508574af6b07cce9231a528317f43 (diff) |
Merge pull request #28436 from vespa-engine/mpolden/account-validation
Move account validation
Diffstat (limited to 'node-repository')
4 files changed, 53 insertions, 8 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index ea0fbcc7108..78538af6c3c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -3,11 +3,13 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeAllocationException; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.jdisc.Metric; +import com.yahoo.text.internal.SnippetGenerator; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; @@ -25,6 +27,7 @@ import java.util.function.Predicate; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; /** * Performs preparation of node activation changes for a cluster of an application. @@ -162,7 +165,7 @@ public class Preparer { private NodeAllocation prepareAllocation(ApplicationId application, ClusterSpec cluster, NodeSpec requested, Supplier<Integer> nextIndex, LockedNodeList allNodes) { - + validateAccount(requested.cloudAccount(), application, allNodes); NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requested, nextIndex, nodeRepository); NodePrioritizer prioritizer = new NodePrioritizer(allNodes, application, @@ -178,6 +181,23 @@ public class Preparer { return allocation; } + private void validateAccount(CloudAccount requestedAccount, ApplicationId application, LockedNodeList allNodes) { + CloudAccount effectiveAccount = requestedAccount.isUnspecified() ? nodeRepository.zone().cloud().account() : requestedAccount; + List<Node> nodesInOtherAccount = allNodes.owner(application).nodeType(NodeType.tenant).stream() + .filter(node -> !node.cloudAccount().equals(effectiveAccount)) + .toList(); + if (nodesInOtherAccount.isEmpty()) return; + + SnippetGenerator snippet = new SnippetGenerator(); + String hostnames = nodesInOtherAccount.stream() + .map(Node::hostname) + .collect(Collectors.joining(", ")); + String hostsSnippet = snippet.makeSnippet(hostnames, 100); + throw new IllegalArgumentException("Cannot allocate nodes in " + requestedAccount + " because " + + application + " has existing nodes in " + nodesInOtherAccount.get(0).cloudAccount() + + ": " + hostsSnippet + ". Deployment must be removed in order to change account"); + } + private boolean canProvisionDynamically(NodeType hostType) { return nodeRepository.zone().cloud().dynamicProvisioning() && (hostType == NodeType.host || hostType.isConfigServerHostLike()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java index 179d1d9a6ff..d51a3e72622 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java @@ -533,9 +533,21 @@ public class HostCapacityMaintainerTest { assertEquals(2, provisioningTester.activate(applicationId, prepared).size()); NodeList allNodes0 = tester.nodeRepository.nodes().list(); - // Redeployment in different account provisions a new set of hosts + // Redeployment in different account fails CloudAccount cloudAccount1 = CloudAccount.from("100000000000"); Capacity capacity1 = Capacity.from(resources, resources, IntRange.empty(), false, true, Optional.of(cloudAccount1), ClusterInfo.empty()); + try { + provisioningTester.prepare(applicationId, spec, capacity1); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Deployment must be removed in order to change account")); + } + + // Redeployment in different account succeeds after removing old hosts + provisioningTester.remove(applicationId); + for (var node : provisioningTester.nodeRepository().nodes().list().state(State.dirty)) { + provisioningTester.nodeRepository().nodes().removeRecursively(node, true); + } prepared = provisioningTester.prepare(applicationId, spec, capacity1); provisionHostsIn(cloudAccount1, 2, tester); assertEquals(2, provisioningTester.activate(applicationId, prepared).size()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index 2b36bacf1b1..cbe12a8202b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -12,6 +12,7 @@ import com.yahoo.config.provision.ClusterInfo; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.IntRange; import com.yahoo.config.provision.NodeResources; @@ -66,7 +67,8 @@ public class LoadBalancerProvisionerTest { private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); private final ProvisioningTester tester = new ProvisioningTester.Builder().flagSource(flagSource) - .zone(new Zone(Cloud.builder().allowEnclave(true).account(CloudAccount.from("001122334455")).build(), SystemName.main, Environment.prod, RegionName.defaultName())).build(); + .zone(new Zone(Cloud.builder().allowEnclave(true).account(CloudAccount.from("001122334455")).build(), + SystemName.main, Environment.prod, RegionName.defaultName())).build(); @Test public void provision_load_balancer() { @@ -413,7 +415,7 @@ public class LoadBalancerProvisionerTest { assertTrue(e.getMessage().contains("due to change in cloud account")); } - // Existing LB is removed + // Existing LB and nodes are removed loadBalancers = tester.nodeRepository().loadBalancers().list(); assertEquals(1, loadBalancers.size()); assertSame(LoadBalancer.State.removable, loadBalancers.first().get().state()); @@ -423,6 +425,8 @@ public class LoadBalancerProvisionerTest { new TestMetric()); expirer.run(); assertEquals(0, tester.nodeRepository().loadBalancers().list().in(LoadBalancer.State.removable).size()); + tester.deactivate(app1); + tester.nodeRepository().nodes().list().forEach(node -> tester.nodeRepository().nodes().removeRecursively(node, true)); // Next deployment provisions a new LB tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1")))); @@ -473,7 +477,11 @@ public class LoadBalancerProvisionerTest { if (capacity.type().isConfigServerLike()) { nodeCount = 3; } - tester.makeReadyNodes(specs.length * nodeCount, nodeResources, capacity.type()); + List<Node> provisioned = tester.makeProvisionedNodes(specs.length * nodeCount, (index) -> "host-" + index + ".yahoo.com", new Flavor(nodeResources), + Optional.empty(), capacity.type(), 0, false, + capacity.cloudAccount().filter(acc -> !acc.isUnspecified()) + .orElse(tester.nodeRepository().zone().cloud().account())); + tester.move(Node.State.ready, provisioned); Set<HostSpec> allNodes = new LinkedHashSet<>(); for (ClusterSpec spec : specs) { allNodes.addAll(tester.prepare(application, spec, capacity)); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 60dd9ce59ef..be8c84141b5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -8,6 +8,7 @@ import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.ApplicationTransaction; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.Cloud; +import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; @@ -421,6 +422,10 @@ public class ProvisioningTester { } public List<Node> makeProvisionedNodes(int n, Function<Integer, String> nodeNamer, Flavor flavor, Optional<TenantName> reservedTo, NodeType type, int ipAddressPoolSize, boolean dualStack) { + return makeProvisionedNodes(n, nodeNamer, flavor, reservedTo, type, ipAddressPoolSize, dualStack, nodeRepository.zone().cloud().account()); + } + + public List<Node> makeProvisionedNodes(int n, Function<Integer, String> nodeNamer, Flavor flavor, Optional<TenantName> reservedTo, NodeType type, int ipAddressPoolSize, boolean dualStack, CloudAccount cloudAccount) { if (ipAddressPoolSize == 0 && type == NodeType.host) { ipAddressPoolSize = 1; // Tenant hosts must have at least one IP in their pool } @@ -463,7 +468,7 @@ public class ProvisioningTester { } } Node.Builder builder = Node.create(hostname, IP.Config.of(hostIps, ipAddressPool), hostname, flavor, type) - .cloudAccount(nodeRepository.zone().cloud().account()); + .cloudAccount(cloudAccount); reservedTo.ifPresent(builder::reservedTo); nodes.add(builder.build()); } @@ -763,8 +768,8 @@ public class ProvisioningTester { flavor.minMainMemoryAvailableGb(resources.memoryGb()); flavor.minDiskAvailableGb(resources.diskGb()); flavor.bandwidth(resources.bandwidthGbps() * 1000); - flavor.fastDisk(resources.diskSpeed().compatibleWith(NodeResources.DiskSpeed.fast)); - flavor.remoteStorage(resources.storageType().compatibleWith(NodeResources.StorageType.remote)); + flavor.fastDisk(resources.diskSpeed().compatibleWith(com.yahoo.config.provision.NodeResources.DiskSpeed.fast)); + flavor.remoteStorage(resources.storageType().compatibleWith(com.yahoo.config.provision.NodeResources.StorageType.remote)); flavor.architecture(resources.architecture().toString()); flavor.gpuCount(resources.gpuResources().count()); flavor.gpuMemoryGb(resources.gpuResources().memoryGb()); |