From 0fd353494243d6def5013e6799f4f9e22052b117 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Tue, 28 Nov 2023 11:33:43 +0100 Subject: Rewrite underscores in domain name --- .../com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java index 2c672d0eada..0807d13d7a8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java @@ -15,7 +15,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toMap; @@ -68,7 +67,7 @@ public class LoadBalancerServiceMock implements LoadBalancerService { public LoadBalancerInstance provision(LoadBalancerSpec spec) { if (throwOnCreate) throw new IllegalStateException("Did not expect a new load balancer to be created"); var instance = new LoadBalancerInstance( - Optional.of(DomainName.of("lb-" + spec.application().toShortString() + "-" + spec.cluster().value())), + Optional.of(DomainName.of("lb-" + spec.application().toShortString().replaceAll("_", "--") + "-" + spec.cluster().value())), Optional.empty(), Optional.empty(), Optional.of(new DnsZone("zone-id-1")), -- cgit v1.2.3 From 7fc4d391aee5bf8cade54507453d193370ad841d Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Tue, 28 Nov 2023 13:23:29 +0100 Subject: Remove unused method --- .../src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java index 04f08240e43..0fdb00f6584 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java @@ -133,11 +133,6 @@ public class Status { return new Status(reboot, vespaVersion, containerImage, failCount, wantToRetire, wantToDeprovision, wantToRebuild, preferToRetire, wantToFail, wantToUpgradeFlavor, osVersion, firmwareVerifiedAt); } - /** Returns a copy of this with wantToUpgradeFlavor set to given value */ - public Status withWantToUpgradeFlavor(boolean wantToUpgradeFlavor) { - return new Status(reboot, vespaVersion, containerImage, failCount, wantToRetire, wantToDeprovision, wantToRebuild, preferToRetire, wantToFail, wantToUpgradeFlavor, osVersion, firmwareVerifiedAt); - } - /** Returns a copy of this with the OS version set to given version */ public Status withOsVersion(OsVersion version) { return new Status(reboot, vespaVersion, containerImage, failCount, wantToRetire, wantToDeprovision, wantToRebuild, preferToRetire, wantToFail, wantToUpgradeFlavor, version, firmwareVerifiedAt); -- cgit v1.2.3 From 4bb1c9671f04d3c746dfe95aecc621db626a0a96 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Tue, 28 Nov 2023 14:02:22 +0100 Subject: Avoid moves to hosts that contain cluster nodes in any state --- .../yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java | 6 +++--- .../vespa/hosted/provision/maintenance/SwitchRebalancerTest.java | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java index 5e600b990ae..9095250827d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java @@ -69,9 +69,9 @@ public class SwitchRebalancer extends NodeMover { private NodeList clusterOf(Node node, NodeList allNodes) { ApplicationId application = node.allocation().get().owner(); ClusterSpec.Id cluster = node.allocation().get().membership().cluster().id(); - return allNodes.state(Node.State.active) - .owner(application) - .cluster(cluster); + // This considers all states to prevent unnecessary moves. E.g. we don't want to start moving nodes to a host + // which already contain a failed node in our cluster + return allNodes.owner(application).cluster(cluster); } /** Returns whether allocatedNode is on an exclusive switch */ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java index dd687c27a7f..95b993edc6e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java @@ -92,6 +92,8 @@ public class SwitchRebalancerTest { // Retired node becomes inactive and makes zone stable deactivate(tester, retired); + nodesIn(cluster, tester).state(Node.State.dirty) + .forEach(node -> tester.nodeRepository().nodes().removeRecursively(node, true)); } assertEquals("Rebalanced all clusters", clusters, rebalancedClusters); @@ -185,6 +187,8 @@ public class SwitchRebalancerTest { tester.assertSwitches(Set.of(switch0, switch1, switch2, switch3), app, spec.id()); retired = nodesIn(spec.id(), tester).state(Node.State.active).retired(); deactivate(tester, retired); + nodesIn(spec.id(), tester).state(Node.State.dirty) + .forEach(node -> tester.nodeRepository().nodes().removeRecursively(node.hostname())); // Next iteration does nothing tester.clock().advance(SwitchRebalancer.waitTimeAfterPreviousDeployment); -- cgit v1.2.3 From 85c6c53bbddad1aebb3e49fca4611fa2c9fd77be Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Tue, 28 Nov 2023 15:08:04 +0100 Subject: Consider only non-transient states --- .../vespa/hosted/provision/maintenance/SwitchRebalancer.java | 8 +++++--- .../vespa/hosted/provision/maintenance/SwitchRebalancerTest.java | 4 ---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java index 9095250827d..112f1fa96f6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancer.java @@ -69,9 +69,11 @@ public class SwitchRebalancer extends NodeMover { private NodeList clusterOf(Node node, NodeList allNodes) { ApplicationId application = node.allocation().get().owner(); ClusterSpec.Id cluster = node.allocation().get().membership().cluster().id(); - // This considers all states to prevent unnecessary moves. E.g. we don't want to start moving nodes to a host - // which already contain a failed node in our cluster - return allNodes.owner(application).cluster(cluster); + // This considers some non-active states to prevent unnecessary moves. E.g. we don't want to start moving nodes + // to a host which already contain a failed node in our cluster + return allNodes.state(Node.State.reserved, Node.State.active, Node.State.failed, Node.State.parked) + .owner(application) + .cluster(cluster); } /** Returns whether allocatedNode is on an exclusive switch */ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java index 95b993edc6e..dd687c27a7f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SwitchRebalancerTest.java @@ -92,8 +92,6 @@ public class SwitchRebalancerTest { // Retired node becomes inactive and makes zone stable deactivate(tester, retired); - nodesIn(cluster, tester).state(Node.State.dirty) - .forEach(node -> tester.nodeRepository().nodes().removeRecursively(node, true)); } assertEquals("Rebalanced all clusters", clusters, rebalancedClusters); @@ -187,8 +185,6 @@ public class SwitchRebalancerTest { tester.assertSwitches(Set.of(switch0, switch1, switch2, switch3), app, spec.id()); retired = nodesIn(spec.id(), tester).state(Node.State.active).retired(); deactivate(tester, retired); - nodesIn(spec.id(), tester).state(Node.State.dirty) - .forEach(node -> tester.nodeRepository().nodes().removeRecursively(node.hostname())); // Next iteration does nothing tester.clock().advance(SwitchRebalancer.waitTimeAfterPreviousDeployment); -- cgit v1.2.3