diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2023-03-22 23:04:31 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-22 23:04:31 +0100 |
commit | 49996514d179ec2cf15d676836f8a4aacb13e48d (patch) | |
tree | f224237b56ee8b6fe4abaf5f9413248b2e806e52 /node-repository/src/main | |
parent | c0e8ae27d4fe623de21586ff711c7bcddeef3026 (diff) |
Revert "Disallow incremental non-exclusive container allocation"
Diffstat (limited to 'node-repository/src/main')
5 files changed, 24 insertions, 42 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 5738c8f3945..510c4041efb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -205,7 +205,7 @@ public class NodeRepository extends AbstractComponent { */ public boolean exclusiveAllocation(ClusterSpec clusterSpec) { return clusterSpec.isExclusive() || - ( clusterSpec.type().isContainer() && zone.system().isPublic() && !zone.environment().isTest() ) || + ( clusterSpec.type().isContainer() && zone.system().isPublic() && !zone.environment().isTest() ) || ( !zone().cloud().allowHostSharing() && !sharedHosts.value().isEnabled(clusterSpec.type().name())); } 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 910be942435..c6971f0fe02 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 @@ -194,34 +194,27 @@ class NodeAllocation { return false; } - /** - * Returns whether allocating the candidate on this host would violate exclusivity constraints. - * Note that while we currently require that exclusive allocations uses the entire host, - * this method also handles the case where smaller exclusive nodes are allocated on it. - */ private boolean violatesExclusivity(NodeCandidate candidate) { - if (candidate.parent.isEmpty()) return false; - - if (nodeRepository.exclusiveAllocation(cluster)) { - // Node must allocate the host entirely and not violate application or cluster type constraints - var parent = candidate.parent.get(); - if (!candidate.resources().isUnspecified() && - ! nodeRepository.resourcesCalculator().advertisedResourcesOf(parent.flavor()).compatibleWith(candidate.resources())) return true; - if (parent.exclusiveToApplicationId().isPresent() && !parent.exclusiveToApplicationId().get().equals(application)) return true; - if (parent.exclusiveToClusterType().isPresent() && !parent.exclusiveToClusterType().get().equals(cluster.type())) return true; - return false; + if (candidate.parentHostname().isEmpty()) return false; + + // In nodes which does not allow host sharing, exclusivity is violated if... + if ( ! nodeRepository.zone().cloud().allowHostSharing()) { + // TODO: Write this in a way that is simple to read + // If either the parent is dedicated to a cluster type different from this cluster + return ! candidate.parent.flatMap(Node::exclusiveToClusterType).map(cluster.type()::equals).orElse(true) || + // or this cluster is requiring exclusivity, but the host is exclusive to a different owner + (requestedNodes.isExclusive() && !candidate.parent.flatMap(Node::exclusiveToApplicationId).map(application::equals).orElse(false)); } - else { - // If any of the nodes on the host requires exclusivity to another application, allocating on it is a violation - for (Node nodeOnHost : allNodes.childrenOf(candidate.parentHostname().get())) { - if (nodeOnHost.allocation().isEmpty()) continue; - if (nodeOnHost.allocation().get().membership().cluster().isExclusive()) { - if (!nodeOnHost.allocation().get().owner().equals(application)) - return true; - } + + // In zones with shared hosts we require that if either of the nodes on the host requires exclusivity, + // then all the nodes on the host must have the same owner + for (Node nodeOnHost : allNodes.childrenOf(candidate.parentHostname().get())) { + if (nodeOnHost.allocation().isEmpty()) continue; + if (requestedNodes.isExclusive() || nodeOnHost.allocation().get().membership().cluster().isExclusive()) { + if ( ! nodeOnHost.allocation().get().owner().equals(application)) return true; } - return false; } + return false; } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java index 6efb9b6f4aa..24ea9361823 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockHostProvisioner.java @@ -47,7 +47,7 @@ public class MockHostProvisioner implements HostProvisioner { private int deprovisionedHosts = 0; private EnumSet<Behaviour> behaviours = EnumSet.noneOf(Behaviour.class); - private final Map<ClusterSpec.Type, Flavor> hostFlavors = new HashMap<>(); + private Map<ClusterSpec.Type, Flavor> hostFlavors = new HashMap<>(); public MockHostProvisioner(List<Flavor> flavors, MockNameResolver nameResolver, int memoryTaxGb) { this.flavors = List.copyOf(flavors); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java index 51eeb8717f5..0a614cc9b2b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java @@ -97,10 +97,10 @@ public class MockNodeRepository extends NodeRepository { defaultCloudAccount = zone.cloud().account(); curator.setZooKeeperEnsembleConnectionSpec("cfg1:1234,cfg2:1234,cfg3:1234"); - populate(zone); + populate(); } - private void populate(Zone zone) { + private void populate() { NodeRepositoryProvisioner provisioner = new NodeRepositoryProvisioner(this, Zone.defaultZone(), new MockProvisionServiceProvider()); List<Node> nodes = new ArrayList<>(); @@ -202,26 +202,16 @@ public class MockNodeRepository extends NodeRepository { .vespaVersion("6.42") .loadBalancerSettings(new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "arne")))) .build(); - ClusterResources min, max; - if (zone.system().isPublic()) { // resources must match one of the flavors used in this mock ("large"), since this is a container cluster - min = new ClusterResources(2, 1, new NodeResources(4, 32, 1600, 1)); - max = new ClusterResources(8, 2, new NodeResources(8, 64, 3200, 1)); - } - else { // resources must fit on actually provisioned hosts - min = new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1)); - max = new ClusterResources(8, 2, new NodeResources(4, 16, 1000, 1)); - } - activate(provisioner.prepare(app1Id, cluster1Id, - Capacity.from(min, - max, + Capacity.from(new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1)), + new ClusterResources(8, 2, new NodeResources(4, 16, 1000, 1)), IntRange.empty(), false, true, Optional.empty(), ClusterInfo.empty()), - null), app1Id, provisioner); + null), app1Id, provisioner); Application app1 = applications().get(app1Id).get(); Cluster cluster1 = app1.cluster(cluster1Id.id()).get(); cluster1 = cluster1.withSuggested(new Autoscaling(Autoscaling.Status.unavailable, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java index a8be7ac3af4..81947251e64 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockProvisionServiceProvider.java @@ -50,5 +50,4 @@ public class MockProvisionServiceProvider implements ProvisionServiceProvider { public HostResourcesCalculator getHostResourcesCalculator() { return hostResourcesCalculator; } - } |