diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2021-04-26 16:19:34 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2021-04-26 16:19:34 +0200 |
commit | ac9a1c3af6585ed311ec78961e5b8c872eda3486 (patch) | |
tree | b6b9f785edcf25f99fef3e2fb3109c398fec5d2d /node-repository | |
parent | c14ea82f74693e9897142d1ae48f4c97d7430cea (diff) |
Revert "Always store load balancer when provisioning"
This reverts commit 0f87ab9c4b45d5a7a2310216d51d2c9fb6401aca.
Diffstat (limited to 'node-repository')
10 files changed, 72 insertions, 132 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java index a6bb57bef29..6f7b7c4d57d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancer.java @@ -5,7 +5,6 @@ import com.yahoo.vespa.hosted.provision.maintenance.LoadBalancerExpirer; import java.time.Instant; import java.util.Objects; -import java.util.Optional; /** * Represents a load balancer for an application's cluster. This is immutable. @@ -15,18 +14,15 @@ import java.util.Optional; public class LoadBalancer { private final LoadBalancerId id; - private final Optional<LoadBalancerInstance> instance; + private final LoadBalancerInstance instance; private final State state; private final Instant changedAt; - public LoadBalancer(LoadBalancerId id, Optional<LoadBalancerInstance> instance, State state, Instant changedAt) { + public LoadBalancer(LoadBalancerId id, LoadBalancerInstance instance, State state, Instant changedAt) { this.id = Objects.requireNonNull(id, "id must be non-null"); this.instance = Objects.requireNonNull(instance, "instance must be non-null"); this.state = Objects.requireNonNull(state, "state must be non-null"); this.changedAt = Objects.requireNonNull(changedAt, "changedAt must be non-null"); - if (state == State.active && instance.isEmpty()) { - throw new IllegalArgumentException("Load balancer instance is required in state " + state); - } } /** An identifier for this load balancer. The ID is unique inside the zone */ @@ -35,7 +31,7 @@ public class LoadBalancer { } /** The instance associated with this */ - public Optional<LoadBalancerInstance> instance() { + public LoadBalancerInstance instance() { return instance; } @@ -62,7 +58,7 @@ public class LoadBalancer { } /** Returns a copy of this with instance set to given instance */ - public LoadBalancer with(Optional<LoadBalancerInstance> instance) { + public LoadBalancer with(LoadBalancerInstance instance) { return new LoadBalancer(id, instance, state, changedAt); } 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 9665d8872de..2ef12177eaf 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 @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancerSpec; import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; import java.time.Duration; -import java.time.Instant; import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; @@ -57,10 +56,10 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { /** Move reserved load balancer that have expired to inactive */ private void expireReserved() { - 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))); + var expiry = nodeRepository().clock().instant().minus(reservedExpiry); + patchLoadBalancers(lb -> lb.state() == State.reserved && + lb.changedAt().isBefore(expiry), + lb -> db.writeLoadBalancer(lb.with(State.inactive, nodeRepository().clock().instant()))); } /** Deprovision inactive load balancers that have expired */ @@ -96,14 +95,13 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { var failed = new ArrayList<LoadBalancerId>(); var lastException = new AtomicReference<Exception>(); patchLoadBalancers(lb -> lb.state() == State.inactive, lb -> { - if (lb.instance().isEmpty()) return; var allocatedNodes = allocatedNodes(lb.id()).stream().map(Node::hostname).collect(Collectors.toSet()); - var reals = new LinkedHashSet<>(lb.instance().get().reals()); + var reals = new LinkedHashSet<>(lb.instance().reals()); // Remove any real no longer allocated to this application reals.removeIf(real -> !allocatedNodes.contains(real.hostname().value())); try { 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().withReals(reals))); } catch (Exception e) { failed.add(lb.id()); lastException.set(e); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java index 4cd01167293..dddc535b36a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancers; import java.util.Comparator; import java.util.LinkedHashSet; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.TreeSet; @@ -71,7 +70,6 @@ public class NodeAcl { loadBalancers.list(allocation.owner()).asList() .stream() .map(LoadBalancer::instance) - .flatMap(Optional::stream) .map(LoadBalancerInstance::networks) .forEach(trustedNetworks::addAll); }); 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 04c8f012b8b..66172521d4c 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 @@ -18,7 +18,6 @@ import java.io.UncheckedIOException; import java.time.Instant; import java.util.LinkedHashSet; import java.util.Optional; -import java.util.Set; import java.util.function.Function; /** @@ -51,22 +50,21 @@ public class LoadBalancerSerializer { Cursor root = slime.setObject(); root.setString(idField, loadBalancer.id().serializedForm()); - // TODO(mpolden): Stop writing this field for empty instance after 2021-06-01 - root.setString(hostnameField, loadBalancer.instance().map(instance -> instance.hostname().value()).orElse("")); + root.setString(hostnameField, loadBalancer.instance().hostname().toString()); root.setString(stateField, asString(loadBalancer.state())); root.setLong(changedAtField, loadBalancer.changedAt().toEpochMilli()); - loadBalancer.instance().flatMap(LoadBalancerInstance::dnsZone).ifPresent(dnsZone -> root.setString(dnsZoneField, dnsZone.id())); + loadBalancer.instance().dnsZone().ifPresent(dnsZone -> root.setString(dnsZoneField, dnsZone.id())); Cursor portArray = root.setArray(portsField); - loadBalancer.instance().ifPresent(instance -> instance.ports().forEach(portArray::addLong)); + loadBalancer.instance().ports().forEach(portArray::addLong); Cursor networkArray = root.setArray(networksField); - loadBalancer.instance().ifPresent(instance -> instance.networks().forEach(networkArray::addString)); + loadBalancer.instance().networks().forEach(networkArray::addString); Cursor realArray = root.setArray(realsField); - loadBalancer.instance().ifPresent(instance -> instance.reals().forEach(real -> { + loadBalancer.instance().reals().forEach(real -> { Cursor realObject = realArray.addObject(); realObject.setString(hostnameField, real.hostname().value()); realObject.setString(ipAddressField, real.ipAddress()); realObject.setLong(portField, real.port()); - })); + }); try { return SlimeUtils.toJsonBytes(slime); } catch (IOException e) { @@ -77,7 +75,7 @@ public class LoadBalancerSerializer { public static LoadBalancer fromJson(byte[] data) { Cursor object = SlimeUtils.jsonToSlime(data).get(); - Set<Real> reals = new LinkedHashSet<>(); + var reals = new LinkedHashSet<Real>(); object.field(realsField).traverse((ArrayTraverser) (i, realObject) -> { reals.add(new Real(HostName.from(realObject.field(hostnameField).asString()), realObject.field(ipAddressField).asString(), @@ -85,19 +83,20 @@ public class LoadBalancerSerializer { }); - Set<Integer> ports = new LinkedHashSet<>(); + var ports = new LinkedHashSet<Integer>(); object.field(portsField).traverse((ArrayTraverser) (i, port) -> ports.add((int) port.asLong())); - Set<String> networks = new LinkedHashSet<>(); + var networks = new LinkedHashSet<String>(); object.field(networksField).traverse((ArrayTraverser) (i, network) -> networks.add(network.asString())); - Optional<HostName> hostname = optionalString(object.field(hostnameField), Function.identity()).filter(s -> !s.isEmpty()).map(HostName::from); - Optional<DnsZone> dnsZone = optionalString(object.field(dnsZoneField), DnsZone::new); - Optional<LoadBalancerInstance> instance = hostname.map(h -> new LoadBalancerInstance(h, dnsZone, ports, - networks, reals)); - return new LoadBalancer(LoadBalancerId.fromSerializedForm(object.field(idField).asString()), - instance, + new LoadBalancerInstance( + HostName.from(object.field(hostnameField).asString()), + optionalString(object.field(dnsZoneField), DnsZone::new), + ports, + networks, + reals + ), stateFromString(object.field(stateField).asString()), Instant.ofEpochMilli(object.field(changedAtField).asLong())); } 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 f53eb189ec1..09d19300c59 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 @@ -170,35 +170,18 @@ public class LoadBalancerProvisioner { Instant now = nodeRepository.clock().instant(); Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id); if (loadBalancer.isEmpty() && activate) return; // Nothing to activate as this load balancer was never prepared - - Set<Real> reals = realsOf(nodes); - Optional<LoadBalancerInstance> instance = provisionInstance(id, reals, loadBalancer); + LoadBalancerInstance instance = provisionInstance(id, realsOf(nodes), loadBalancer); LoadBalancer newLoadBalancer; 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); + var newState = activate ? LoadBalancer.State.active : loadBalancer.get().state(); + newLoadBalancer = loadBalancer.get().with(instance).with(newState, now); if (loadBalancer.get().state() != newLoadBalancer.state()) { log.log(Level.FINE, "Moving " + newLoadBalancer.id() + " to state " + 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); - } + db.writeLoadBalancers(List.of(newLoadBalancer), transaction.nested()); } private void provision(ApplicationTransaction transaction, ClusterSpec.Id clusterId, NodeList nodes) { @@ -206,18 +189,17 @@ public class LoadBalancerProvisioner { } /** Provision or reconfigure a load balancer instance, if necessary */ - private Optional<LoadBalancerInstance> provisionInstance(LoadBalancerId id, Set<Real> reals, - Optional<LoadBalancer> currentLoadBalancer) { + private LoadBalancerInstance provisionInstance(LoadBalancerId id, Set<Real> reals, + Optional<LoadBalancer> currentLoadBalancer) { if (hasReals(currentLoadBalancer, reals)) return currentLoadBalancer.get().instance(); log.log(Level.FINE, "Creating " + id + ", targeting: " + reals); try { - return Optional.of(service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals), - allowEmptyReals(currentLoadBalancer))); + return service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals), + allowEmptyReals(currentLoadBalancer)); } catch (Exception e) { - log.log(Level.WARNING, "Could not (re)configure " + id + ", targeting: " + - reals + ". The operation will be retried on next deployment", e); + throw new LoadBalancerServiceException("Failed to (re)configure " + id + ", targeting: " + + reals + ". The operation will be retried on next deployment", e); } - return Optional.empty(); } /** Returns the nodes allocated to the given load balanced cluster */ @@ -264,8 +246,7 @@ public class LoadBalancerProvisioner { /** Returns whether load balancer has given reals */ private static boolean hasReals(Optional<LoadBalancer> loadBalancer, Set<Real> reals) { if (loadBalancer.isEmpty()) return false; - if (loadBalancer.get().instance().isEmpty()) return false; - return loadBalancer.get().instance().get().reals().equals(reals); + return loadBalancer.get().instance().reals().equals(reals); } /** Returns whether to allow given load balancer to have no reals */ @@ -292,4 +273,5 @@ public class LoadBalancerProvisioner { return cluster.combinedId().orElse(cluster.id()); } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java index bcd0af4e121..1ef449555d9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java @@ -9,7 +9,6 @@ import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; -import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerList; import com.yahoo.vespa.hosted.provision.node.filter.ApplicationFilter; @@ -66,23 +65,21 @@ public class LoadBalancersResponse extends HttpResponse { lbObject.setString("tenant", lb.id().application().tenant().value()); lbObject.setString("instance", lb.id().application().instance().value()); lbObject.setString("cluster", lb.id().cluster().value()); - lb.instance().ifPresent(instance -> lbObject.setString("hostname", instance.hostname().value())); - lb.instance().flatMap(LoadBalancerInstance::dnsZone).ifPresent(dnsZone -> lbObject.setString("dnsZone", dnsZone.id())); + lbObject.setString("hostname", lb.instance().hostname().value()); + lb.instance().dnsZone().ifPresent(dnsZone -> lbObject.setString("dnsZone", dnsZone.id())); Cursor networkArray = lbObject.setArray("networks"); - lb.instance().ifPresent(instance -> instance.networks().forEach(networkArray::addString)); + lb.instance().networks().forEach(networkArray::addString); Cursor portArray = lbObject.setArray("ports"); - lb.instance().ifPresent(instance -> instance.ports().forEach(portArray::addLong)); + lb.instance().ports().forEach(portArray::addLong); Cursor realArray = lbObject.setArray("reals"); - lb.instance().ifPresent(instance -> { - instance.reals().forEach(real -> { - Cursor realObject = realArray.addObject(); - realObject.setString("hostname", real.hostname().value()); - realObject.setString("ipAddress", real.ipAddress()); - realObject.setLong("port", real.port()); - }); + lb.instance().reals().forEach(real -> { + Cursor realObject = realArray.addObject(); + realObject.setString("hostname", real.hostname().value()); + realObject.setString("ipAddress", real.ipAddress()); + realObject.setLong("port", real.port()); }); }); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java index 4d19c2c1d41..fec6e40fd35 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirerTest.java @@ -63,7 +63,7 @@ public class LoadBalancerExpirerTest { tester.nodeRepository().nodes().setReady(tester.nodeRepository().nodes().list(Node.State.dirty).asList(), Agent.system, getClass().getSimpleName()); expirer.maintain(); assertEquals(Set.of(), tester.loadBalancerService().instances().get(lb1).reals()); - assertEquals(Set.of(), loadBalancers.get().get(lb1).instance().get().reals()); + assertEquals(Set.of(), loadBalancers.get().get(lb1).instance().reals()); // Expirer defers removal of load balancer until expiration time passes expirer.maintain(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java index 51d6f7fb2fb..bcb78844666 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java @@ -30,7 +30,7 @@ public class LoadBalancerSerializerTest { "application1", "default"), ClusterSpec.Id.from("qrs")), - Optional.of(new LoadBalancerInstance( + new LoadBalancerInstance( HostName.from("lb-host"), Optional.of(new DnsZone("zone-id-1")), ImmutableSet.of(4080, 4443), @@ -40,19 +40,19 @@ public class LoadBalancerSerializerTest { 4080), new Real(HostName.from("real-2"), "127.0.0.2", - 4080)))), + 4080))), LoadBalancer.State.active, now); var serialized = LoadBalancerSerializer.fromJson(LoadBalancerSerializer.toJson(loadBalancer)); assertEquals(loadBalancer.id(), serialized.id()); - assertEquals(loadBalancer.instance().get().hostname(), serialized.instance().get().hostname()); - assertEquals(loadBalancer.instance().get().dnsZone(), serialized.instance().get().dnsZone()); - assertEquals(loadBalancer.instance().get().ports(), serialized.instance().get().ports()); - assertEquals(loadBalancer.instance().get().networks(), serialized.instance().get().networks()); + assertEquals(loadBalancer.instance().hostname(), serialized.instance().hostname()); + assertEquals(loadBalancer.instance().dnsZone(), serialized.instance().dnsZone()); + assertEquals(loadBalancer.instance().ports(), serialized.instance().ports()); + assertEquals(loadBalancer.instance().networks(), serialized.instance().networks()); assertEquals(loadBalancer.state(), serialized.state()); assertEquals(loadBalancer.changedAt().truncatedTo(MILLIS), serialized.changedAt()); - assertEquals(loadBalancer.instance().get().reals(), serialized.instance().get().reals()); + assertEquals(loadBalancer.instance().reals(), serialized.instance().reals()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java index 744322c9edc..b7e671a7b76 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java @@ -180,7 +180,7 @@ public class AclProvisioningTest { // Load balancer is allocated to application var loadBalancers = tester.nodeRepository().loadBalancers().list(application); assertEquals(1, loadBalancers.asList().size()); - var lbNetworks = loadBalancers.asList().get(0).instance().get().networks(); + var lbNetworks = loadBalancers.asList().get(0).instance().networks(); assertEquals(2, lbNetworks.size()); // ACL for nodes with allocation trust their respective load balancer networks, if any 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 eb3bdff484d..a3780db4789 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 @@ -10,7 +10,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; -import com.yahoo.config.provision.exception.LoadBalancerServiceException; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; @@ -60,7 +59,7 @@ public class LoadBalancerProvisionerTest { clusterRequest(ClusterSpec.Type.container, containerCluster1), clusterRequest(ClusterSpec.Type.content, contentCluster)); assertEquals(1, lbApp1.get().size()); - assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbApp1.get().get(0).instance().get().reals().size()); + assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbApp1.get().get(0).instance().reals().size()); tester.activate(app1, nodes); tester.activate(app2, prepare(app2, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs")))); assertEquals(1, lbApp2.get().size()); @@ -68,11 +67,11 @@ public class LoadBalancerProvisionerTest { // Reals are configured after activation assertEquals(app1, lbApp1.get().get(0).id().application()); assertEquals(containerCluster1, lbApp1.get().get(0).id().cluster()); - assertEquals(Collections.singleton(4443), lbApp1.get().get(0).instance().get().ports()); - assertEquals("127.0.0.1", get(lbApp1.get().get(0).instance().get().reals(), 0).ipAddress()); - assertEquals(4443, get(lbApp1.get().get(0).instance().get().reals(), 0).port()); - assertEquals("127.0.0.2", get(lbApp1.get().get(0).instance().get().reals(), 1).ipAddress()); - assertEquals(4443, get(lbApp1.get().get(0).instance().get().reals(), 1).port()); + assertEquals(Collections.singleton(4443), lbApp1.get().get(0).instance().ports()); + assertEquals("127.0.0.1", get(lbApp1.get().get(0).instance().reals(), 0).ipAddress()); + assertEquals(4443, get(lbApp1.get().get(0).instance().reals(), 0).port()); + assertEquals("127.0.0.2", get(lbApp1.get().get(0).instance().reals(), 1).ipAddress()); + assertEquals(4443, get(lbApp1.get().get(0).instance().reals(), 1).port()); // A container is failed Supplier<List<Node>> containers = () -> tester.getNodes(app1).container().asList(); @@ -84,13 +83,13 @@ public class LoadBalancerProvisionerTest { clusterRequest(ClusterSpec.Type.container, containerCluster1), clusterRequest(ClusterSpec.Type.content, contentCluster))); LoadBalancer loadBalancer = tester.nodeRepository().loadBalancers().list(app1).asList().get(0); - assertEquals(2, loadBalancer.instance().get().reals().size()); - assertTrue("Failed node is removed", loadBalancer.instance().get().reals().stream() + assertEquals(2, loadBalancer.instance().reals().size()); + assertTrue("Failed node is removed", loadBalancer.instance().reals().stream() .map(Real::hostname) .map(HostName::value) .noneMatch(hostname -> hostname.equals(toFail.hostname()))); - assertEquals(containers.get().get(0).hostname(), get(loadBalancer.instance().get().reals(), 0).hostname().value()); - assertEquals(containers.get().get(1).hostname(), get(loadBalancer.instance().get().reals(), 1).hostname().value()); + assertEquals(containers.get().get(0).hostname(), get(loadBalancer.instance().reals(), 0).hostname().value()); + assertEquals(containers.get().get(1).hostname(), get(loadBalancer.instance().reals(), 1).hostname().value()); assertSame("State is unchanged", LoadBalancer.State.active, loadBalancer.state()); // Add another container cluster to first app @@ -111,7 +110,6 @@ public class LoadBalancerProvisionerTest { .collect(Collectors.toList()); List<HostName> reals = lbApp1.get().stream() .map(LoadBalancer::instance) - .flatMap(Optional::stream) .map(LoadBalancerInstance::reals) .flatMap(Collection::stream) .map(Real::hostname) @@ -154,7 +152,7 @@ public class LoadBalancerProvisionerTest { .findFirst() .orElseThrow()); - // Next redeploy does not create a new load balancer instance because reals are unchanged + // Next redeploy does not create a new load balancer instance tester.loadBalancerService().throwOnCreate(true); tester.activate(app1, prepare(app1, clusterRequest(ClusterSpec.Type.container, containerCluster1), @@ -210,7 +208,7 @@ public class LoadBalancerProvisionerTest { var combinedId = ClusterSpec.Id.from("container1"); var nodes = prepare(app1, clusterRequest(ClusterSpec.Type.combined, ClusterSpec.Id.from("content1"), Optional.of(combinedId))); assertEquals(1, lbs.get().size()); - assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().get().reals().size()); + assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size()); tester.activate(app1, nodes); assertSame(LoadBalancer.State.active, lbs.get().get(0).state()); assertEquals(combinedId, lbs.get().get(0).id().cluster()); @@ -224,7 +222,7 @@ public class LoadBalancerProvisionerTest { var nodes = prepare(configServerApp, Capacity.fromRequiredNodeType(NodeType.config), clusterRequest(ClusterSpec.Type.admin, cluster)); assertEquals(1, lbs.get().size()); - assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().get().reals().size()); + assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size()); tester.activate(configServerApp, nodes); assertSame(LoadBalancer.State.active, lbs.get().get(0).state()); assertEquals(cluster, lbs.get().get(0).id().cluster()); @@ -238,7 +236,7 @@ public class LoadBalancerProvisionerTest { var nodes = prepare(controllerApp, Capacity.fromRequiredNodeType(NodeType.controller), clusterRequest(ClusterSpec.Type.container, cluster)); assertEquals(1, lbs.get().size()); - assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().get().reals().size()); + assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().reals().size()); tester.activate(controllerApp, nodes); assertSame(LoadBalancer.State.active, lbs.get().get(0).state()); assertEquals(cluster, lbs.get().get(0).id().cluster()); @@ -255,7 +253,7 @@ public class LoadBalancerProvisionerTest { // instance1 is deployed tester.activate(instance1, prepare(instance1, clusterRequest(ClusterSpec.Type.container, devCluster))); - // instance2 clashes because instance name matches instance1 cluster name + // instance2 clashes because cluster name matches instance1 try { prepare(instance2, clusterRequest(ClusterSpec.Type.container, defaultCluster)); fail("Expected exception"); @@ -265,38 +263,10 @@ public class LoadBalancerProvisionerTest { // instance2 changes cluster name and does not clash tester.activate(instance2, prepare(instance2, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("qrs")))); - // instance3 does not clash + // instance3 clashes because instance name matches instance2 cluster tester.activate(instance3, prepare(instance3, clusterRequest(ClusterSpec.Type.container, defaultCluster))); } - @Test - public void provisioning_load_balancer_fails_initially() { - Supplier<List<LoadBalancer>> lbs = () -> tester.nodeRepository().loadBalancers().list(app1).asList(); - ClusterSpec.Id cluster = ClusterSpec.Id.from("qrs1"); - - // Provisioning load balancer fails on deployment - tester.loadBalancerService().throwOnCreate(true); - try { - prepare(app1, clusterRequest(ClusterSpec.Type.container, cluster)); - fail("Expected exception"); - } catch (LoadBalancerServiceException ignored) {} - List<LoadBalancer> loadBalancers = lbs.get(); - assertEquals(1, loadBalancers.size()); - assertSame(LoadBalancer.State.reserved, loadBalancers.get(0).state()); - assertTrue("Load balancer has no instance", loadBalancers.get(0).instance().isEmpty()); - - // Next deployment succeeds - tester.loadBalancerService().throwOnCreate(false); - Set<HostSpec> nodes = prepare(app1, clusterRequest(ClusterSpec.Type.container, cluster)); - loadBalancers = lbs.get(); - assertSame(LoadBalancer.State.reserved, loadBalancers.get(0).state()); - assertTrue("Load balancer has instance", loadBalancers.get(0).instance().isPresent()); - tester.activate(app1, nodes); - loadBalancers = lbs.get(); - assertSame(LoadBalancer.State.active, loadBalancers.get(0).state()); - assertTrue("Load balancer has instance", loadBalancers.get(0).instance().isPresent()); - } - private void dirtyNodesOf(ApplicationId application) { tester.nodeRepository().nodes().deallocate(tester.nodeRepository().nodes().list().owner(application).asList(), Agent.system, this.getClass().getSimpleName()); } |