From 91596d876fbb5da8c142f160708dc99086e2fddf Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 11 Jun 2021 10:18:31 +0200 Subject: Log load balancer state transitions --- .../provision/maintenance/LoadBalancerExpirer.java | 4 +- .../persistence/CuratorDatabaseClient.java | 17 ++++- .../provisioning/LoadBalancerProvisioner.java | 87 ++++++++++------------ 3 files changed, 57 insertions(+), 51 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java index ac9d8d6671a..c3d6f5c42b8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java @@ -65,7 +65,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { Instant now = nodeRepository().clock().instant(); Instant expiry = now.minus(reservedExpiry); patchLoadBalancers(lb -> lb.state() == State.reserved && lb.changedAt().isBefore(expiry), - lb -> db.writeLoadBalancer(lb.with(State.inactive, now))); + lb -> db.writeLoadBalancer(lb.with(State.inactive, now), lb.state())); } /** Deprovision inactive load balancers that have expired */ @@ -114,7 +114,7 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { attempts.add(1); LOG.log(Level.INFO, () -> "Removing reals from inactive load balancer " + lb.id() + ": " + Sets.difference(lb.instance().get().reals(), reals)); service.create(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals), true); - db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals)))); + db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals))), lb.state()); } catch (Exception e) { failed.add(lb.id()); lastException.set(e); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index 364af6308a1..0205cc6c818 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -482,18 +482,29 @@ public class CuratorDatabaseClient { return read(loadBalancerPath(id), LoadBalancerSerializer::fromJson); } - public void writeLoadBalancer(LoadBalancer loadBalancer) { + public void writeLoadBalancer(LoadBalancer loadBalancer, LoadBalancer.State fromState) { NestedTransaction transaction = new NestedTransaction(); - writeLoadBalancers(List.of(loadBalancer), transaction); + writeLoadBalancers(List.of(loadBalancer), fromState, transaction); transaction.commit(); } - public void writeLoadBalancers(Collection loadBalancers, NestedTransaction transaction) { + public void writeLoadBalancers(Collection loadBalancers, LoadBalancer.State fromState, NestedTransaction transaction) { CuratorTransaction curatorTransaction = db.newCuratorTransactionIn(transaction); loadBalancers.forEach(loadBalancer -> { curatorTransaction.add(createOrSet(loadBalancerPath(loadBalancer.id()), LoadBalancerSerializer.toJson(loadBalancer))); }); + transaction.onCommitted(() -> { + for (var lb : loadBalancers) { + if (lb.state() == fromState) continue; + if (fromState == null) { + log.log(Level.INFO, () -> "Creating " + lb.id() + " in " + lb.state()); + } else { + log.log(Level.INFO, () -> "Moving " + lb.id() + " from " + fromState + + " to " + lb.state()); + } + } + }); } public void removeLoadBalancer(LoadBalancerId loadBalancer) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index c114aa58a05..71884829b62 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -7,7 +7,6 @@ import com.yahoo.config.provision.ApplicationTransaction; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeType; -import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.exception.LoadBalancerServiceException; import com.yahoo.transaction.NestedTransaction; @@ -61,7 +60,7 @@ public class LoadBalancerProvisioner { for (var id : db.readLoadBalancerIds()) { try (var lock = db.lock(id.application())) { var loadBalancer = db.readLoadBalancer(id); - loadBalancer.ifPresent(db::writeLoadBalancer); + loadBalancer.ifPresent(lb -> db.writeLoadBalancer(lb, lb.state())); } } } @@ -81,11 +80,9 @@ public class LoadBalancerProvisioner { if (application.instance().isTester()) return; // Do not provision for tester instances try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); - NodeList nodes = nodesOf(clusterId, application); LoadBalancerId loadBalancerId = requireNonClashing(new LoadBalancerId(application, clusterId)); - ApplicationTransaction transaction = new ApplicationTransaction(new ProvisionLock(application, lock), new NestedTransaction()); - provision(transaction, loadBalancerId, nodes, false); - transaction.nested().commit(); + NodeList nodes = nodesOf(clusterId, application); + prepare(loadBalancerId, nodes); } } @@ -100,14 +97,15 @@ public class LoadBalancerProvisioner { * Calling this when no load balancer has been prepared for given cluster is a no-op. */ public void activate(Set clusters, ApplicationTransaction transaction) { + Set activatingClusters = clusters.stream() + .map(LoadBalancerProvisioner::effectiveId) + .collect(Collectors.toSet()); for (var cluster : loadBalancedClustersOf(transaction.application()).entrySet()) { - // Provision again to ensure that load balancer instance is re-configured with correct nodes - provision(transaction, cluster.getKey(), cluster.getValue()); + if (!activatingClusters.contains(cluster.getKey())) continue; + activate(transaction, cluster.getKey(), cluster.getValue()); } // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed - var surplusLoadBalancers = surplusLoadBalancersOf(transaction.application(), clusters.stream() - .map(LoadBalancerProvisioner::effectiveId) - .collect(Collectors.toSet())); + var surplusLoadBalancers = surplusLoadBalancersOf(transaction.application(), activatingClusters); deactivate(surplusLoadBalancers, transaction.nested()); } @@ -140,7 +138,7 @@ public class LoadBalancerProvisioner { var deactivatedLoadBalancers = loadBalancers.stream() .map(lb -> lb.with(LoadBalancer.State.inactive, now)) .collect(Collectors.toList()); - db.writeLoadBalancers(deactivatedLoadBalancers, transaction); + db.writeLoadBalancers(deactivatedLoadBalancers, LoadBalancer.State.active, transaction); } /** Find all load balancer IDs owned by given tenant and application */ @@ -165,52 +163,41 @@ public class LoadBalancerProvisioner { return loadBalancerId; } - /** Idempotently provision a load balancer for given application and cluster */ - private void provision(ApplicationTransaction transaction, LoadBalancerId id, NodeList nodes, boolean activate) { + private void prepare(LoadBalancerId id, NodeList nodes) { Instant now = nodeRepository.clock().instant(); Optional loadBalancer = db.readLoadBalancer(id); - if (loadBalancer.isEmpty() && activate) return; // Nothing to activate as this load balancer was never prepared - - Set reals = realsOf(nodes); - Optional instance = provisionInstance(id, reals, loadBalancer); + Optional instance = provisionInstance(id, nodes, loadBalancer); LoadBalancer newLoadBalancer; + LoadBalancer.State fromState = null; if (loadBalancer.isEmpty()) { newLoadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now); } else { - LoadBalancer.State state = activate && instance.isPresent() - ? LoadBalancer.State.active - : loadBalancer.get().state(); - newLoadBalancer = loadBalancer.get().with(instance).with(state, now); - if (loadBalancer.get().state() != newLoadBalancer.state()) { - log.log(Level.INFO, () -> "Moving " + newLoadBalancer.id() + " from " + loadBalancer.get().state() + - " to " + newLoadBalancer.state()); - } - } - - if (activate) { - db.writeLoadBalancers(List.of(newLoadBalancer), transaction.nested()); - } else { - // Always store load balancer so that LoadBalancerExpirer can expire partially provisioned load balancers - db.writeLoadBalancer(newLoadBalancer); - } - - // Signal that load balancer is not ready yet - if (instance.isEmpty()) { - throw new LoadBalancerServiceException("Could not (re)configure " + id + ", targeting: " + - reals + ". The operation will be retried on next deployment", - null); + newLoadBalancer = loadBalancer.get().with(instance); + fromState = newLoadBalancer.state(); } + // Always store load balancer so that LoadBalancerExpirer can expire partially provisioned load balancers + db.writeLoadBalancer(newLoadBalancer, fromState); + requireInstance(id, instance); } - private void provision(ApplicationTransaction transaction, ClusterSpec.Id clusterId, NodeList nodes) { - provision(transaction, new LoadBalancerId(transaction.application(), clusterId), nodes, true); + private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, NodeList nodes) { + Instant now = nodeRepository.clock().instant(); + LoadBalancerId id = new LoadBalancerId(transaction.application(), cluster); + Optional loadBalancer = db.readLoadBalancer(id); + if (loadBalancer.isEmpty()) throw new IllegalArgumentException("Could not active load balancer that was never prepared: " + id); + + Optional instance = provisionInstance(id, nodes, loadBalancer); + LoadBalancer.State state = instance.isPresent() ? LoadBalancer.State.active : loadBalancer.get().state(); + LoadBalancer newLoadBalancer = loadBalancer.get().with(instance).with(state, now); + db.writeLoadBalancers(List.of(newLoadBalancer), loadBalancer.get().state(), transaction.nested()); + requireInstance(id, instance); } /** Provision or reconfigure a load balancer instance, if necessary */ - private Optional provisionInstance(LoadBalancerId id, Set reals, - Optional currentLoadBalancer) { + private Optional provisionInstance(LoadBalancerId id, NodeList nodes, Optional currentLoadBalancer) { + Set reals = realsOf(nodes); if (hasReals(currentLoadBalancer, reals)) return currentLoadBalancer.get().instance(); - log.log(Level.INFO, () -> "Creating " + id + ", targeting: " + reals); + log.log(Level.INFO, () -> "Provisioning instance for " + id + ", targeting: " + reals); try { return Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals), allowEmptyReals(currentLoadBalancer))); @@ -241,7 +228,7 @@ public class LoadBalancerProvisioner { /** Returns real servers for given nodes */ private Set realsOf(NodeList nodes) { - var reals = new LinkedHashSet(); + Set reals = new LinkedHashSet(); for (var node : nodes) { for (var ip : reachableIpAddresses(node)) { reals.add(new Real(HostName.from(node.hostname()), ip)); @@ -289,6 +276,14 @@ public class LoadBalancerProvisioner { return reachable; } + private static void requireInstance(LoadBalancerId id, Optional instance) { + if (instance.isEmpty()) { + // Signal that load balancer is not ready yet + throw new LoadBalancerServiceException("Could not (re)configure " + id + ". The operation will be retried on next deployment", + null); + } + } + private static ClusterSpec.Id effectiveId(ClusterSpec cluster) { return cluster.combinedId().orElse(cluster.id()); } -- cgit v1.2.3 From 1032d77abd632500a7128c9bbd3a8e494d2dc287 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 11 Jun 2021 15:37:16 +0200 Subject: Test that service supports node/cluster type when activating --- .../vespa/hosted/provision/lb/LoadBalancerService.java | 5 +---- .../vespa/hosted/provision/lb/LoadBalancerServiceMock.java | 14 ++++++++++++++ .../provision/lb/PassthroughLoadBalancerService.java | 6 ++++++ .../hosted/provision/lb/SharedLoadBalancerService.java | 2 +- .../provision/provisioning/LoadBalancerProvisioner.java | 5 ++++- .../provisioning/LoadBalancerProvisionerTest.java | 7 +++++++ 6 files changed, 33 insertions(+), 6 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java index fba0993f2f9..40f9b330634 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerService.java @@ -29,10 +29,7 @@ public interface LoadBalancerService { Protocol protocol(); /** Returns whether load balancers created by this service can forward traffic to given node and cluster type */ - default boolean canForwardTo(NodeType nodeType, ClusterSpec.Type clusterType) { - return (nodeType == NodeType.tenant && clusterType.isContainer()) || - nodeType.isConfigServerLike(); - } + boolean supports(NodeType nodeType, ClusterSpec.Type clusterType); /** Load balancer protocols */ enum Protocol { 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 b912087da46..f752cbc4349 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 @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.NodeType; import java.util.Collections; import java.util.HashMap; @@ -18,6 +19,7 @@ public class LoadBalancerServiceMock implements LoadBalancerService { private final Map instances = new HashMap<>(); private boolean throwOnCreate = false; + private boolean supportsProvisioning = true; public Map instances() { return Collections.unmodifiableMap(instances); @@ -28,6 +30,18 @@ public class LoadBalancerServiceMock implements LoadBalancerService { return this; } + public LoadBalancerServiceMock supportsProvisioning(boolean supportsProvisioning) { + this.supportsProvisioning = supportsProvisioning; + return this; + } + + @Override + public boolean supports(NodeType nodeType, ClusterSpec.Type clusterType) { + if (!supportsProvisioning) return false; + return (nodeType == NodeType.tenant && clusterType.isContainer()) || + nodeType.isConfigServerLike(); + } + @Override public Protocol protocol() { return Protocol.ipv4; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerService.java index 7667672e470..9a6a65eca69 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/PassthroughLoadBalancerService.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.lb; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.NodeType; import java.util.Comparator; import java.util.Optional; @@ -35,4 +36,9 @@ public class PassthroughLoadBalancerService implements LoadBalancerService { return Protocol.ipv4; } + @Override + public boolean supports(NodeType nodeType, ClusterSpec.Type clusterType) { + return true; + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java index 33a3c138d70..e17e5a5a449 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java @@ -66,7 +66,7 @@ public class SharedLoadBalancerService implements LoadBalancerService { } @Override - public boolean canForwardTo(NodeType nodeType, ClusterSpec.Type clusterType) { + public boolean supports(NodeType nodeType, ClusterSpec.Type clusterType) { // Shared routing layer only supports routing to tenant nodes return nodeType == NodeType.tenant && clusterType.isContainer(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 71884829b62..b6600574f94 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -76,7 +76,7 @@ public class LoadBalancerProvisioner { * Calling this for irrelevant node or cluster types is a no-op. */ public void prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes) { - if (!service.canForwardTo(requestedNodes.type(), cluster.type())) return; // Nothing to provision for this node and cluster type + if (!service.supports(requestedNodes.type(), cluster.type())) return; // Nothing to provision for this node and cluster type if (application.instance().isTester()) return; // Do not provision for tester instances try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); @@ -102,6 +102,9 @@ public class LoadBalancerProvisioner { .collect(Collectors.toSet()); for (var cluster : loadBalancedClustersOf(transaction.application()).entrySet()) { if (!activatingClusters.contains(cluster.getKey())) continue; + + Node clusterNode = cluster.getValue().first().get(); + if (!service.supports(clusterNode.type(), clusterNode.allocation().get().membership().cluster().type())) continue; activate(transaction, cluster.getKey(), cluster.getValue()); } // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed 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 bdc3bdfd816..16fe5ef241a 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 @@ -300,6 +300,13 @@ public class LoadBalancerProvisionerTest { assertTrue("Load balancer has instance", loadBalancers.get(0).instance().isPresent()); } + @Test + public void provisioning_load_balancer_for_unsupported_cluster_fails_gracefully() { + tester.loadBalancerService().supportsProvisioning(false); + tester.activate(app1, prepare(app1, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs")))); + assertTrue("No load balancer provisioned", tester.nodeRepository().loadBalancers().list(app1).asList().isEmpty()); + } + private void dirtyNodesOf(ApplicationId application) { tester.nodeRepository().nodes().deallocate(tester.nodeRepository().nodes().list().owner(application).asList(), Agent.system, this.getClass().getSimpleName()); } -- cgit v1.2.3