From da6003f48965c02f5d6e92ddd40d73ba9fa38379 Mon Sep 17 00:00:00 2001 From: jonmv Date: Fri, 17 Feb 2023 10:59:36 +0100 Subject: Store multiple endpoint services, but still show only the latest one --- .../hosted/provision/lb/LoadBalancerInstance.java | 22 ++++++++++++++++------ .../provision/lb/LoadBalancerServiceMock.java | 5 +++-- .../provision/lb/SharedLoadBalancerService.java | 3 ++- .../persistence/LoadBalancerSerializer.java | 17 ++++++++++++++--- .../provisioning/LoadBalancerProvisioner.java | 5 +++-- 5 files changed, 38 insertions(+), 14 deletions(-) (limited to 'node-repository/src/main/java/com/yahoo') 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 75d37c6f9ba..c4b30f2f5ec 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 @@ -6,6 +6,8 @@ import com.google.common.collect.ImmutableSortedSet; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ZoneEndpoint; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -25,12 +27,12 @@ public class LoadBalancerInstance { private final Set networks; private final Set reals; private final ZoneEndpoint settings; - private final Optional serviceId; + private final List serviceIds; private final CloudAccount cloudAccount; public LoadBalancerInstance(Optional hostname, Optional ipAddress, Optional dnsZone, Set ports, Set networks, Set reals, - ZoneEndpoint settings, Optional serviceId, CloudAccount cloudAccount) { + ZoneEndpoint settings, List serviceIds, CloudAccount cloudAccount) { this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null"); this.ipAddress = Objects.requireNonNull(ipAddress, "ip must be non-null"); this.dnsZone = Objects.requireNonNull(dnsZone, "dnsZone must be non-null"); @@ -38,7 +40,7 @@ public class LoadBalancerInstance { this.networks = ImmutableSortedSet.copyOf(Objects.requireNonNull(networks, "networks must be non-null")); this.reals = ImmutableSortedSet.copyOf(Objects.requireNonNull(reals, "targets must be non-null")); this.settings = Objects.requireNonNull(settings, "settings must be non-null"); - this.serviceId = Objects.requireNonNull(serviceId, "private service id must be non-null"); + this.serviceIds = Objects.requireNonNull(serviceIds, "private service id must be non-null"); this.cloudAccount = Objects.requireNonNull(cloudAccount, "cloudAccount must be non-null"); if (hostname.isEmpty() == ipAddress.isEmpty()) { @@ -83,8 +85,13 @@ public class LoadBalancerInstance { } /** ID of any private endpoint service configured for this load balancer. */ + // TODO jonmv: remove public Optional serviceId() { - return serviceId; + return serviceIds.isEmpty() ? Optional.empty() : Optional.of(serviceIds.get(serviceIds.size() - 1)); + } + + public List serviceIds() { + return serviceIds; } /** Cloud account of this load balancer */ @@ -103,8 +110,11 @@ public class LoadBalancerInstance { return ports; } - public LoadBalancerInstance withServiceId(Optional serviceId) { - return new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, serviceId, cloudAccount); + /** Prepends the given service IDs, possibly replacing those we have in this. */ + public LoadBalancerInstance withServiceIds(List serviceIds) { + List 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/LoadBalancerServiceMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java index 2649ddc37aa..d35c11783dd 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 @@ -9,6 +9,7 @@ import com.yahoo.config.provision.ZoneEndpoint; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -60,7 +61,7 @@ public class LoadBalancerServiceMock implements LoadBalancerService { 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.settings().isPrivateEndpoint() ? List.of(PrivateServiceId.of("service")) : List.of(), spec.cloudAccount()); instances.put(id, instance); return instance; @@ -82,7 +83,7 @@ public class LoadBalancerServiceMock implements LoadBalancerService { 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.settings().isPrivateEndpoint() ? List.of(PrivateServiceId.of("service")) : List.of(), spec.cloudAccount()); instances.put(id, instance); return instance; 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 15120a66d8a..432b56c7b48 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 @@ -5,6 +5,7 @@ import ai.vespa.http.DomainName; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeType; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -46,7 +47,7 @@ public class SharedLoadBalancerService implements LoadBalancerService { Set.of(), spec.reals(), spec.settings(), - Optional.empty(), + List.of(), spec.cloudAccount()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java index 6bac1dab3dd..b85d96c6b54 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java @@ -21,7 +21,9 @@ import com.yahoo.vespa.hosted.provision.lb.Real; import java.io.IOException; import java.io.UncheckedIOException; import java.time.Instant; +import java.util.ArrayList; import java.util.LinkedHashSet; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.function.Function; @@ -52,6 +54,7 @@ public class LoadBalancerSerializer { private static final String ipAddressField = "ipAddress"; private static final String portField = "port"; private static final String serviceIdField = "serviceId"; + private static final String serviceIdsField = "serviceIds"; private static final String cloudAccountField = "cloudAccount"; private static final String settingsField = "settings"; private static final String publicField = "public"; @@ -86,7 +89,11 @@ public class LoadBalancerSerializer { .ifPresent(settings -> toSlime(root.setObject(settingsField), settings)); loadBalancer.instance() .flatMap(LoadBalancerInstance::serviceId) - .ifPresent(serviceId -> root.setString(serviceIdField, serviceId.value())); + .ifPresent(serviceId -> root.setString(serviceIdField, serviceId.value())); // TODO: remove after winter vacation '23 + loadBalancer.instance().stream() + .map(LoadBalancerInstance::serviceIds).flatMap(List::stream) + .map(PrivateServiceId::value) + .forEach(root.setArray(serviceIdsField)::addString); loadBalancer.instance() .map(LoadBalancerInstance::cloudAccount) .filter(cloudAccount -> ! cloudAccount.isUnspecified()) @@ -120,9 +127,13 @@ public class LoadBalancerSerializer { Optional dnsZone = optionalString(object.field(dnsZoneField), DnsZone::new); ZoneEndpoint settings = zoneEndpoint(object.field(settingsField)); Optional serviceId = optionalString(object.field(serviceIdField), PrivateServiceId::of); + List serviceIds = new ArrayList<>(); + object.field(serviceIdsField).traverse((ArrayTraverser) (__, serviceIdObject) -> serviceIds.add(PrivateServiceId.of(serviceIdObject.asString()))); + if (serviceIds.isEmpty()) serviceId.ifPresent(serviceIds::add); // TODO: remove after winter vacation '23 CloudAccount cloudAccount = optionalString(object.field(cloudAccountField), CloudAccount::from).orElse(CloudAccount.empty); - Optional instance = hostname.isEmpty() && ipAddress.isEmpty() ? Optional.empty() : - Optional.of(new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, serviceId, cloudAccount)); + Optional instance = hostname.isEmpty() && ipAddress.isEmpty() + ? Optional.empty() + : Optional.of(new LoadBalancerInstance(hostname, ipAddress, dnsZone, ports, networks, reals, settings, serviceIds, cloudAccount)); return new LoadBalancer(LoadBalancerId.fromSerializedForm(object.field(idField).asString()), instance, 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 dc3b8f60d3e..494d50f6a45 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 @@ -252,7 +252,7 @@ public class LoadBalancerProvisioner { try { return Optional.of(service.provision(new LoadBalancerSpec(id.application(), id.cluster(), reals, settings, cloudAccount)) // Provisioning a private endpoint service requires hard resources to be ready, so we delay it until activation. - .withServiceId(currentLoadBalancer.flatMap(LoadBalancer::instance).flatMap(LoadBalancerInstance::serviceId))); + .withServiceIds(currentLoadBalancer.flatMap(LoadBalancer::instance).map(LoadBalancerInstance::serviceIds).orElse(List.of()))); } catch (Exception e) { log.log(Level.WARNING, e, () -> "Could not provision " + id + ". The operation will be retried on next deployment"); } @@ -274,7 +274,8 @@ public class LoadBalancerProvisioner { 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)); + shouldDeactivateRouting || currentLoadBalancer.state() != LoadBalancer.State.active) + .withServiceIds(currentLoadBalancer.instance().map(LoadBalancerInstance::serviceIds).orElse(List.of()))); } catch (Exception e) { log.log(Level.WARNING, e, () -> "Could not (re)configure " + id + ", targeting: " + reals + ". The operation will be retried on next deployment"); -- cgit v1.2.3 From 31cabaa2fccab81a03b10a89025f20e44f6e5dc3 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 17 Feb 2023 12:25:42 +0100 Subject: Defensive copy Co-authored-by: Valerij Fredriksen --- .../java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'node-repository/src/main/java/com/yahoo') 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 c4b30f2f5ec..f166e73da77 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 @@ -40,7 +40,7 @@ public class LoadBalancerInstance { this.networks = ImmutableSortedSet.copyOf(Objects.requireNonNull(networks, "networks must be non-null")); this.reals = ImmutableSortedSet.copyOf(Objects.requireNonNull(reals, "targets must be non-null")); this.settings = Objects.requireNonNull(settings, "settings must be non-null"); - this.serviceIds = Objects.requireNonNull(serviceIds, "private service id must be non-null"); + this.serviceIds = List.copyOf(Objects.requireNonNull(serviceIds, "private service id must be non-null")); this.cloudAccount = Objects.requireNonNull(cloudAccount, "cloudAccount must be non-null"); if (hostname.isEmpty() == ipAddress.isEmpty()) { -- cgit v1.2.3