From f91ff91230f9004561e1d705010ab2f15bf8d47d Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Mon, 3 Jul 2023 15:50:58 +0200 Subject: Avoid allocating non-exclusive children to exclusive hosts of different application --- .../hosted/provision/provisioning/NodeAllocation.java | 6 ++++-- .../provisioning/DynamicProvisioningTest.java | 19 ++++++++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) (limited to 'node-repository/src') 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 41a7dd70e9f..a2e0e59e329 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 @@ -207,8 +207,10 @@ class NodeAllocation { // 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 - (nodeRepository.exclusiveAllocation(cluster) && !candidate.parent.flatMap(Node::exclusiveToApplicationId).map(application::equals).orElse(false)); + // or the parent is dedicated to a different application + ! candidate.parent.flatMap(Node::exclusiveToApplicationId).map(application::equals).orElse(true) || + // or this cluster requires exclusivity, but the host is not exclusive + (nodeRepository.exclusiveAllocation(cluster) && candidate.parent.flatMap(Node::exclusiveToApplicationId).isEmpty()); } // In zones with shared hosts we require that if either of the nodes on the host requires exclusivity, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java index 7dbed807480..e1e83ad2fb3 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java @@ -214,7 +214,8 @@ public class DynamicProvisioningTest { @Test public void retires_on_exclusivity_violation() { - var tester = tester(true); + var tester = tester(false); + tester.flagSource().withJacksonFlag(PermanentFlags.SHARED_HOST.id(), new SharedHost(List.of(new HostResources(1., 1., 1., 1., "fast", "local", null, 10, "x86_64"))), SharedHost.class); ApplicationId application1 = ProvisioningTester.applicationId(); NodeResources resources = new NodeResources(4, 80, 100, 1); prepareAndActivate(application1, clusterSpec("mycluster"), 4, 1, resources, tester); @@ -229,8 +230,20 @@ public class DynamicProvisioningTest { // Redeploy without exclusive again is no-op prepareAndActivate(application1, clusterSpec("mycluster"), 4, 1, smallerExclusiveResources, tester); - assertEquals(8, tester.nodeRepository().nodes().list().owner(application1).size()); - assertEquals(initialNodes, tester.nodeRepository().nodes().list().owner(application1).retired()); + NodeList nodes = tester.nodeRepository().nodes().list(); + assertEquals(8, nodes.owner(application1).size()); + assertEquals(initialNodes, nodes.owner(application1).retired()); + + // Remove the old retired nodes and make 2 random parents of current nodes violate exclusivity + tester.patchNodes(initialNodes.asList(), node -> node.removable(true)); + NodeList exclusiveViolators = nodes.owner(application1).not().retired().first(2); + List parents = exclusiveViolators.mapToList(node -> nodes.parentOf(node).get()); + tester.patchNode(parents.get(0), node -> node.withExclusiveToApplicationId(ApplicationId.defaultId())); + tester.patchNode(parents.get(1), node -> node.withExclusiveToClusterType(ClusterSpec.Type.container)); + + prepareAndActivate(application1, clusterSpec("mycluster"), 4, 1, smallerExclusiveResources, tester); + assertEquals(10, tester.nodeRepository().nodes().list().owner(application1).size()); + assertEquals(exclusiveViolators, tester.nodeRepository().nodes().list().owner(application1).retired()); } @Test -- cgit v1.2.3