diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2023-02-09 06:06:45 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-02-09 06:06:45 +0100 |
commit | 49b74383aed7b1d3b25f83d0e4bf350232b71b2b (patch) | |
tree | 7314d07d17a8a79cee7e956e0511531f1c4569ad /node-repository/src/main/java/com | |
parent | 3fe7e34a7a8d953b4142005dfcfbe0d83c791eee (diff) | |
parent | 78c1f3d29ddb4754a9b184b890ab9906b9afa069 (diff) |
Merge pull request #25947 from vespa-engine/jonmv/private-endpoints-3
Provision LB resources on prepare, (re)configure on activate
Diffstat (limited to 'node-repository/src/main/java/com')
5 files changed, 67 insertions, 95 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java index 2856a38075b..ee8ecbada7f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java @@ -97,8 +97,8 @@ public class LoadBalancerInstance { return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, serviceId, cloudAccount); } - public LoadBalancerInstance withServiceId(PrivateServiceId serviceId) { - return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, Optional.of(serviceId), cloudAccount); + public LoadBalancerInstance withSettings(ZoneEndpoint settings) { + return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, serviceId, cloudAccount); } private static Set<Integer> requirePorts(Set<Integer> ports) { 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 17b7b16141e..82f679bbe34 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,17 +29,6 @@ public interface LoadBalancerService { */ LoadBalancerInstance configure(LoadBalancerSpec spec, boolean force); - /** - * Create a load balancer from the given specification. Implementations are expected to be idempotent - * - * @param spec Load balancer specification - * @param force Whether reconfiguration should be forced (e.g. allow configuring an empty set of reals on a - * pre-existing load balancer). - * @return The provisioned load balancer instance - */ - // TODO jonmv: remove - default LoadBalancerInstance create(LoadBalancerSpec spec, boolean force) { provision(spec); return configure(spec, force); } - /** Permanently remove given load balancer */ void remove(LoadBalancer loadBalancer); 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 e3510baf026..2649ddc37aa 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 @@ -89,28 +89,6 @@ public class LoadBalancerServiceMock implements LoadBalancerService { } @Override - public LoadBalancerInstance create(LoadBalancerSpec spec, boolean force) { - if (throwOnCreate) throw new IllegalStateException("Did not expect a new load balancer to be created"); - var id = new LoadBalancerId(spec.application(), spec.cluster()); - var oldInstance = instances.get(id); - if (!force && oldInstance != null && !oldInstance.reals().isEmpty() && spec.reals().isEmpty()) { - throw new IllegalArgumentException("Refusing to remove all reals from load balancer " + id); - } - var instance = new LoadBalancerInstance( - Optional.of(DomainName.of("lb-" + spec.application().toShortString() + "-" + spec.cluster().value())), - Optional.empty(), - Optional.of(new DnsZone("zone-id-1")), - Collections.singleton(4443), - ImmutableSet.of("10.2.3.0/24", "10.4.5.0/24"), - spec.reals(), - spec.settings(), - spec.settings().isPrivateEndpoint() ? Optional.of(PrivateServiceId.of("service")) : Optional.empty(), - spec.cloudAccount()); - instances.put(id, instance); - return instance; - } - - @Override public void remove(LoadBalancer loadBalancer) { instances.remove(loadBalancer.id()); } 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 44c92434a0f..b1beb615bc3 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 @@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer.State; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; +import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerService; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerSpec; import com.yahoo.vespa.hosted.provision.persistence.CuratorDb; @@ -111,10 +112,10 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { try { 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, - lb.instance().get().settings(), lb.instance().get().cloudAccount()), - true); - db.writeLoadBalancer(lb.with(lb.instance().map(instance -> instance.withReals(reals))), lb.state()); + LoadBalancerInstance instance = service.configure(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals, + lb.instance().get().settings(), lb.instance().get().cloudAccount()), + true); + db.writeLoadBalancer(lb.with(Optional.of(instance)), 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/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index fc03ddcc3b0..4cdcda9fd6a 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 @@ -92,8 +92,7 @@ public class LoadBalancerProvisioner { try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); LoadBalancerId loadBalancerId = requireNonClashing(new LoadBalancerId(application, clusterId)); - NodeList nodes = nodesOf(clusterId, application); - prepare(loadBalancerId, nodes, cluster.zoneEndpoint(), requestedNodes.cloudAccount()); + prepare(loadBalancerId, cluster.zoneEndpoint(), requestedNodes.cloudAccount()); } } @@ -189,27 +188,22 @@ public class LoadBalancerProvisioner { return loadBalancerId; } - private void prepare(LoadBalancerId id, NodeList nodes, ZoneEndpoint zoneEndpoint, CloudAccount cloudAccount) { + private void prepare(LoadBalancerId id, ZoneEndpoint zoneEndpoint, CloudAccount cloudAccount) { Instant now = nodeRepository.clock().instant(); Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id); - Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, zoneEndpoint, cloudAccount, true); LoadBalancer newLoadBalancer; - LoadBalancer.State fromState = null; - if (loadBalancer.isEmpty()) { - newLoadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now); - } else { - newLoadBalancer = loadBalancer.get().with(instance); - fromState = newLoadBalancer.state(); - } - if (!inAccount(cloudAccount, newLoadBalancer)) { - // We have a load balancer, but in the wrong account. Load balancer must be removed before we can - // provision a new one in the wanted account - newLoadBalancer = newLoadBalancer.with(LoadBalancer.State.removable, now); + LoadBalancer.State fromState = loadBalancer.map(LoadBalancer::state).orElse(null); + if ( loadBalancer.isPresent() + && ( ! inAccount(cloudAccount, loadBalancer.get()) + || ! hasCorrectVisibility(loadBalancer.get(), zoneEndpoint))) { + // We have a load balancer, but with the wrong account or visibility. + // Load balancer must be removed before we can provision a new one with the wanted visibility + newLoadBalancer = loadBalancer.get().with(LoadBalancer.State.removable, now); } - if (!hasCorrectVisibility(newLoadBalancer, zoneEndpoint)) { - // We have a load balancer, but with the wrong visibility. Load balancer must be removed before we can - // provision a new one with the wanted visibility - newLoadBalancer = newLoadBalancer.with(LoadBalancer.State.removable, now); + else { + Optional<LoadBalancerInstance> instance = provisionInstance(id, loadBalancer, zoneEndpoint, cloudAccount); + newLoadBalancer = loadBalancer.isEmpty() ? new LoadBalancer(id, instance, LoadBalancer.State.reserved, now) + : loadBalancer.get().with(instance); } // Always store the load balancer. LoadBalancerExpirer will remove unwanted ones db.writeLoadBalancer(newLoadBalancer, fromState); @@ -228,42 +222,57 @@ public class LoadBalancerProvisioner { if (loadBalancer.isEmpty()) throw new IllegalArgumentException("Could not activate load balancer that was never prepared: " + id); if (loadBalancer.get().instance().isEmpty()) throw new IllegalArgumentException("Activating " + id + ", but prepare never provisioned a load balancer instance"); - Optional<LoadBalancerInstance> instance = provisionInstance(id, nodes, loadBalancer, settings, loadBalancer.get().instance().get().cloudAccount(), false); + Optional<LoadBalancerInstance> instance = configureInstance(id, nodes, loadBalancer.get(), settings, loadBalancer.get().instance().get().cloudAccount()); 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, newLoadBalancer, loadBalancer.get().instance().get().cloudAccount(), settings); } - /** Provision or reconfigure a load balancer instance, if necessary */ - private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, NodeList nodes, + /** Provision a load balancer instance, if necessary */ + private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, Optional<LoadBalancer> currentLoadBalancer, ZoneEndpoint zoneEndpoint, - CloudAccount cloudAccount, - boolean preparing) { + CloudAccount cloudAccount) { + Set<Real> reals = currentLoadBalancer.flatMap(LoadBalancer::instance) + .map(LoadBalancerInstance::reals) + .orElse(Set.of()); // Targeted reals are changed on activation. + ZoneEndpoint settings = new ZoneEndpoint(zoneEndpoint.isPublicEndpoint(), + zoneEndpoint.isPrivateEndpoint(), + currentLoadBalancer.flatMap(LoadBalancer::instance) + .map(LoadBalancerInstance::settings) + .map(ZoneEndpoint::allowedUrns) + .orElse(List.of())); // Allowed URNs are changed on activation. + if ( currentLoadBalancer.isPresent() + && currentLoadBalancer.get().instance().isPresent() + && currentLoadBalancer.get().instance().get().settings().equals(settings)) + return currentLoadBalancer.get().instance(); + + log.log(Level.INFO, () -> "Provisioning instance for " + id); + try { + return Optional.of(service.provision(new LoadBalancerSpec(id.application(), id.cluster(), reals, settings, cloudAccount))); + } catch (Exception e) { + log.log(Level.WARNING, e, () -> "Could not provision " + id + ". The operation will be retried on next deployment"); + } + return Optional.empty(); // Will cause activation to fail, but lets us proceed with more preparations. + } + + /** Provision or reconfigure a load balancer instance, if necessary */ + private Optional<LoadBalancerInstance> configureInstance(LoadBalancerId id, NodeList nodes, + LoadBalancer currentLoadBalancer, + ZoneEndpoint zoneEndpoint, + CloudAccount cloudAccount) { boolean shouldDeactivateRouting = deactivateRouting.with(FetchVector.Dimension.APPLICATION_ID, id.application().serializedForm()) .value(); - Set<Real> reals; - if (shouldDeactivateRouting) { - reals = Set.of(); - } else { - reals = realsOf(nodes); - } - if (isUpToDate(currentLoadBalancer, reals, zoneEndpoint, preparing)) - return currentLoadBalancer.get().instance(); - log.log(Level.INFO, () -> "Provisioning instance for " + id + ", targeting: " + reals); + Set<Real> reals = shouldDeactivateRouting ? Set.of() : realsOf(nodes); + if (isUpToDate(currentLoadBalancer, reals, zoneEndpoint)) + return currentLoadBalancer.instance(); + + log.log(Level.INFO, () -> "Configuring instance for " + id + ", targeting: " + reals); try { - // Override settings at activation, otherwise keep existing ones. - ZoneEndpoint settings = ! preparing ? zoneEndpoint - : currentLoadBalancer.flatMap(LoadBalancer::instance) - .map(LoadBalancerInstance::settings) - .orElse(zoneEndpoint); - LoadBalancerInstance created = service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals, settings, cloudAccount), - shouldDeactivateRouting || allowEmptyReals(currentLoadBalancer)); - if (created.serviceId().isEmpty() && currentLoadBalancer.flatMap(LoadBalancer::instance).flatMap(LoadBalancerInstance::serviceId).isPresent()) - created = created.withServiceId(currentLoadBalancer.flatMap(LoadBalancer::instance).flatMap(LoadBalancerInstance::serviceId).get()); - return Optional.of(created); + return Optional.of(service.configure(new LoadBalancerSpec(id.application(), id.cluster(), reals, zoneEndpoint, cloudAccount), + shouldDeactivateRouting || currentLoadBalancer.state() != LoadBalancer.State.active)); } catch (Exception e) { log.log(Level.WARNING, e, () -> "Could not (re)configure " + id + ", targeting: " + reals + ". The operation will be retried on next deployment"); @@ -319,16 +328,11 @@ public class LoadBalancerProvisioner { } /** Returns whether load balancer has given reals, and settings if specified */ - private static boolean isUpToDate(Optional<LoadBalancer> loadBalancer, Set<Real> reals, ZoneEndpoint zoneEndpoint, boolean preparing) { - if (loadBalancer.isEmpty()) return false; - if (loadBalancer.get().instance().isEmpty()) return false; - return loadBalancer.get().instance().get().reals().equals(reals) - && (preparing || loadBalancer.get().instance().get().settings().equals(zoneEndpoint)); - } - - /** Returns whether to allow given load balancer to have no reals */ - private static boolean allowEmptyReals(Optional<LoadBalancer> loadBalancer) { - return loadBalancer.isPresent() && loadBalancer.get().state() != LoadBalancer.State.active; + private static boolean isUpToDate(LoadBalancer loadBalancer, Set<Real> reals, ZoneEndpoint zoneEndpoint) { + if (loadBalancer.instance().isEmpty()) + throw new IllegalStateException("Expected a load balancer instance to be present, for " + loadBalancer.id()); + return loadBalancer.instance().get().reals().equals(reals) + && loadBalancer.instance().get().settings().equals(zoneEndpoint); } /** Find IP addresses reachable by the load balancer service */ @@ -345,12 +349,12 @@ public class LoadBalancerProvisioner { private static void requireInstance(LoadBalancerId id, LoadBalancer loadBalancer, CloudAccount cloudAccount, ZoneEndpoint zoneEndpoint) { if (loadBalancer.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"); + throw new LoadBalancerServiceException("Could not provision " + id + ". The operation will be retried on next deployment"); } - if (!inAccount(cloudAccount, loadBalancer)) { + if ( ! inAccount(cloudAccount, loadBalancer)) { throw new LoadBalancerServiceException("Could not (re)configure " + id + " due to change in cloud account. The operation will be retried on next deployment"); } - if (!hasCorrectVisibility(loadBalancer, zoneEndpoint)) { + if ( ! hasCorrectVisibility(loadBalancer, zoneEndpoint)) { throw new LoadBalancerServiceException("Could not (re)configure " + id + " due to change in load balancer visibility. The operation will be retried on next deployment"); } } |