diff options
author | jonmv <venstad@gmail.com> | 2023-03-03 13:44:12 +0100 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2023-03-03 13:44:12 +0100 |
commit | a8fbfb36cc5b80a476097ebc9ab994bb1be130f9 (patch) | |
tree | 894f3c98e7c0fced3a1dafba7ee7b8c7bc62c454 | |
parent | 8a0918e280bf7ebd8f92ec84f76a5d32aa573470 (diff) |
Let implementations short-circuit LB (re)configuration
8 files changed, 29 insertions, 30 deletions
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 9895cd68004..226fb785bf6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -8,7 +8,6 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; -import com.yahoo.config.provision.Tags; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; 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 f166e73da77..e228d31384c 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 @@ -110,9 +110,18 @@ public class LoadBalancerInstance { return ports; } - /** Prepends the given service IDs, possibly replacing those we have in this. */ + /** Updates this with new data, from a reconfiguration. */ + public LoadBalancerInstance with(Set<Real> reals, ZoneEndpoint settings, Optional<PrivateServiceId> serviceId) { + List<PrivateServiceId> ids = new ArrayList<>(serviceIds); + serviceId.filter(id -> ! ids.contains(id)).ifPresent(ids::add); + return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, + reals, settings, ids, + cloudAccount); + } + + /** Prepends the given service IDs, possibly changing the order of those we have in this. */ public LoadBalancerInstance withServiceIds(List<PrivateServiceId> serviceIds) { - List<PrivateServiceId> ids = new ArrayList<>(serviceIds()); + List<PrivateServiceId> ids = new ArrayList<>(serviceIds); for (PrivateServiceId id : this.serviceIds) if ( ! ids.contains(id)) ids.add(id); return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, ids, cloudAccount); } 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 82f679bbe34..ceedbcf89c2 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 @@ -22,12 +22,13 @@ public interface LoadBalancerService { /** * Configures load balancers for the given specification. Implementations are expected to be idempotent * + * @param instance Load balancer instance to reconfigure * @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, or removing an unused private endpoint service load balancer). * @return The (re)configured load balancer instance */ - LoadBalancerInstance configure(LoadBalancerSpec spec, boolean force); + LoadBalancerInstance configure(LoadBalancerInstance instance, LoadBalancerSpec spec, boolean 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 d35c11783dd..1f354dc7081 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 @@ -68,25 +68,17 @@ public class LoadBalancerServiceMock implements LoadBalancerService { } @Override - public LoadBalancerInstance configure(LoadBalancerSpec spec, boolean force) { - if (throwOnCreate) throw new IllegalStateException("Did not expect a new load balancer to be created"); + public LoadBalancerInstance configure(LoadBalancerInstance instance, LoadBalancerSpec spec, boolean force) { var id = new LoadBalancerId(spec.application(), spec.cluster()); var oldInstance = Objects.requireNonNull(instances.get(id), "expected existing load balancer " + id); if (!force && !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() ? List.of(PrivateServiceId.of("service")) : List.of(), - spec.cloudAccount()); - instances.put(id, instance); - return instance; + var updated = instance.with(spec.reals(), + spec.settings(), + spec.settings().isPrivateEndpoint() ? Optional.of(PrivateServiceId.of("service")) : Optional.empty()); + instances.put(id, updated); + return updated; } @Override 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 432b56c7b48..22367f72666 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 @@ -33,8 +33,8 @@ public class SharedLoadBalancerService implements LoadBalancerService { } @Override - public LoadBalancerInstance configure(LoadBalancerSpec spec, boolean force) { - return create(spec); + public LoadBalancerInstance configure(LoadBalancerInstance instance, LoadBalancerSpec spec, boolean force) { + return instance.with(spec.reals(), spec.settings(), Optional.empty()); } private LoadBalancerInstance create(LoadBalancerSpec spec) { 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 b1beb615bc3..f864ab18920 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 @@ -112,7 +112,8 @@ 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)); - LoadBalancerInstance instance = service.configure(new LoadBalancerSpec(lb.id().application(), lb.id().cluster(), reals, + LoadBalancerInstance instance = service.configure(lb.instance().get(), + 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()); 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 494d50f6a45..4f83ad69893 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 @@ -259,7 +259,7 @@ public class LoadBalancerProvisioner { return Optional.empty(); // Will cause activation to fail, but lets us proceed with more preparations. } - /** Provision or reconfigure a load balancer instance, if necessary */ + /** Reconfigure a load balancer instance, if necessary */ private Optional<LoadBalancerInstance> configureInstance(LoadBalancerId id, NodeList nodes, LoadBalancer currentLoadBalancer, ZoneEndpoint zoneEndpoint, @@ -268,14 +268,11 @@ public class LoadBalancerProvisioner { id.application().serializedForm()) .value(); 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 { - return Optional.of(service.configure(new LoadBalancerSpec(id.application(), id.cluster(), reals, zoneEndpoint, cloudAccount), - shouldDeactivateRouting || currentLoadBalancer.state() != LoadBalancer.State.active) - .withServiceIds(currentLoadBalancer.instance().map(LoadBalancerInstance::serviceIds).orElse(List.of()))); + return Optional.of(service.configure(currentLoadBalancer.instance().orElseThrow(() -> new IllegalArgumentException("expected existing instance for " + id)), + 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"); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java index ccfe96d462c..1a6058bed39 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java @@ -30,8 +30,8 @@ public class SharedLoadBalancerServiceTest { public void test_create_lb() { LoadBalancerSpec spec = new LoadBalancerSpec(applicationId, clusterId, reals, ZoneEndpoint.defaultEndpoint, CloudAccount.empty); - loadBalancerService.provision(spec); - var lb = loadBalancerService.configure(spec, false); + + var lb = loadBalancerService.configure(loadBalancerService.provision(spec), spec, false); assertEquals(Optional.of(HostName.of("vip.example.com")), lb.hostname()); assertEquals(Optional.empty(), lb.dnsZone()); |