diff options
10 files changed, 174 insertions, 63 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 58c576d3f44..369366a1f08 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 @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.lb; import com.yahoo.vespa.hosted.provision.maintenance.LoadBalancerExpirer; +import java.time.Instant; import java.util.Objects; /** @@ -14,12 +15,14 @@ public class LoadBalancer { private final LoadBalancerId id; private final LoadBalancerInstance instance; - private final boolean inactive; + private final State state; + private final Instant changedAt; - public LoadBalancer(LoadBalancerId id, LoadBalancerInstance instance, boolean inactive) { + 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.inactive = inactive; + this.state = Objects.requireNonNull(state, "state must be non-null"); + this.changedAt = Objects.requireNonNull(changedAt, "changedAt must be non-null"); } /** An identifier for this load balancer. The ID is unique inside the zone */ @@ -32,17 +35,48 @@ public class LoadBalancer { return instance; } - /** - * Returns whether this load balancer is inactive. Inactive load balancers are eventually removed by - * {@link LoadBalancerExpirer}. Inactive load balancers may be reactivated if a deleted cluster is redeployed. - */ - public boolean inactive() { - return inactive; + /** The current state of this */ + public State state() { + return state; } - /** Return a copy of this that is set inactive */ - public LoadBalancer deactivate() { - return new LoadBalancer(id, instance, true); + /** Returns when this was last changed */ + public Instant changedAt() { + return changedAt; + } + + /** Returns a copy of this with state set to given state */ + public LoadBalancer with(State state, Instant changedAt) { + if (changedAt.isBefore(this.changedAt)) { + throw new IllegalArgumentException("Invalid changeAt: '" + changedAt + "' is before existing value '" + + this.changedAt + "'"); + } + if (this.state == State.active && state == State.reserved) { + throw new IllegalArgumentException("Invalid state transition: " + this.state + " -> " + state); + } + return new LoadBalancer(id, instance, state, changedAt); + } + + /** Returns a copy of this with instance set to given instance */ + public LoadBalancer with(LoadBalancerInstance instance) { + return new LoadBalancer(id, instance, state, changedAt); + } + + public enum State { + + /** This load balancer has been provisioned and reserved for an application */ + reserved, + + /** + * The load balancer has been deactivated and is ready to be removed. Inactive load balancers are eventually + * removed by {@link LoadBalancerExpirer}. Inactive load balancers may be reactivated if a deleted cluster is + * redeployed. + */ + inactive, + + /** The load balancer is in active use by an application */ + active, + } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java index ba7a83169ad..c0bb53ddfe4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerList.java @@ -29,7 +29,7 @@ public class LoadBalancerList { /** Returns the subset of load balancers that are inactive */ public LoadBalancerList inactive() { - return of(loadBalancers.stream().filter(LoadBalancer::inactive)); + return of(loadBalancers.stream().filter(lb -> lb.state() == LoadBalancer.State.inactive)); } public List<LoadBalancer> asList() { 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 371ed4d2496..61ca19a4cb9 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 @@ -484,7 +484,7 @@ public class CuratorDatabaseClient { } private Optional<LoadBalancer> readLoadBalancer(LoadBalancerId id) { - return read(loadBalancerPath(id), LoadBalancerSerializer::fromJson); + return read(loadBalancerPath(id), (data) -> LoadBalancerSerializer.fromJson(data, clock.instant())); } public void writeLoadBalancer(LoadBalancer loadBalancer) { 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 fd2294c1b5d..d04dd2b5c18 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 @@ -15,9 +15,9 @@ import com.yahoo.vespa.hosted.provision.lb.Real; import java.io.IOException; 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; /** @@ -36,12 +36,13 @@ public class LoadBalancerSerializer { private static final String idField = "id"; private static final String hostnameField = "hostname"; + private static final String stateField = "state"; + private static final String changedAtField = "changedAt"; private static final String dnsZoneField = "dnsZone"; private static final String inactiveField = "inactive"; private static final String portsField = "ports"; private static final String networksField = "networks"; private static final String realsField = "reals"; - private static final String nameField = "name"; private static final String ipAddressField = "ipAddress"; private static final String portField = "port"; @@ -51,6 +52,8 @@ public class LoadBalancerSerializer { root.setString(idField, loadBalancer.id().serializedForm()); root.setString(hostnameField, loadBalancer.instance().hostname().toString()); + root.setString(stateField, asString(loadBalancer.state())); + root.setLong(changedAtField, loadBalancer.changedAt().toEpochMilli()); loadBalancer.instance().dnsZone().ifPresent(dnsZone -> root.setString(dnsZoneField, dnsZone.id())); Cursor portArray = root.setArray(portsField); loadBalancer.instance().ports().forEach(portArray::addLong); @@ -63,8 +66,6 @@ public class LoadBalancerSerializer { realObject.setString(ipAddressField, real.ipAddress()); realObject.setLong(portField, real.port()); }); - root.setBool(inactiveField, loadBalancer.inactive()); - try { return SlimeUtils.toJsonBytes(slime); } catch (IOException e) { @@ -72,10 +73,10 @@ public class LoadBalancerSerializer { } } - public static LoadBalancer fromJson(byte[] data) { + public static LoadBalancer fromJson(byte[] data, Instant defaultChangedAt) { 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(), @@ -83,25 +84,61 @@ 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())); return new LoadBalancer(LoadBalancerId.fromSerializedForm(object.field(idField).asString()), new LoadBalancerInstance( HostName.from(object.field(hostnameField).asString()), - optionalField(object.field(dnsZoneField), DnsZone::new), + optionalString(object.field(dnsZoneField), DnsZone::new), ports, networks, reals ), - object.field(inactiveField).asBool()); + stateFromSlime(object), + instantFromSlime(object.field(changedAtField), defaultChangedAt)); + } + + private static Instant instantFromSlime(Cursor field, Instant defaultValue) { + return optionalValue(field, (value) -> Instant.ofEpochMilli(value.asLong())).orElse(defaultValue); + } + + private static LoadBalancer.State stateFromSlime(Inspector object) { + var inactiveValue = optionalValue(object.field(inactiveField), Inspector::asBool); + if (inactiveValue.isPresent()) { // TODO(mpolden): Remove reading of "inactive" field after June 2019 + return inactiveValue.get() ? LoadBalancer.State.inactive : LoadBalancer.State.active; + } else { + return stateFromString(object.field(stateField).asString()); + } + } + + private static <T> Optional<T> optionalValue(Inspector field, Function<Inspector, T> fieldMapper) { + return Optional.of(field).filter(Inspector::valid).map(fieldMapper); + } + + private static <T> Optional<T> optionalString(Inspector field, Function<String, T> fieldMapper) { + return optionalValue(field, Inspector::asString).map(fieldMapper); } - private static <T> Optional<T> optionalField(Inspector field, Function<String, T> fieldMapper) { - return Optional.of(field).filter(Inspector::valid).map(Inspector::asString).map(fieldMapper); + private static String asString(LoadBalancer.State state) { + switch (state) { + case active: return "active"; + case inactive: return "inactive"; + case reserved: return "reserved"; + default: throw new IllegalArgumentException("No serialization defined for state enum '" + state + "'"); + } + } + + private static LoadBalancer.State stateFromString(String state) { + switch (state) { + case "active": return LoadBalancer.State.active; + case "inactive": return LoadBalancer.State.inactive; + case "reserved": return LoadBalancer.State.reserved; + default: throw new IllegalArgumentException("No serialization defined for state string '" + 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 372dca84a53..a37bdf6804b 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 @@ -57,7 +57,7 @@ public class LoadBalancerProvisioner { LoadBalancerInstance instance = create(application, kv.getKey().id(), kv.getValue()); // Load balancer is always re-activated here to avoid reallocation if an application/cluster is // deleted and then redeployed. - LoadBalancer loadBalancer = new LoadBalancer(id, instance, false); + LoadBalancer loadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.active, nodeRepository.clock().instant()); loadBalancers.put(loadBalancer.id(), loadBalancer); db.writeLoadBalancer(loadBalancer); } @@ -73,8 +73,9 @@ public class LoadBalancerProvisioner { public void deactivate(ApplicationId application, NestedTransaction transaction) { try (Mutex applicationLock = nodeRepository.lock(application)) { try (Mutex loadBalancersLock = db.lockLoadBalancers()) { + var now = nodeRepository.clock().instant(); List<LoadBalancer> deactivatedLoadBalancers = nodeRepository.loadBalancers().owner(application).asList().stream() - .map(LoadBalancer::deactivate) + .map(lb -> lb.with(LoadBalancer.State.inactive, now)) .collect(Collectors.toList()); db.writeLoadBalancers(deactivatedLoadBalancers, transaction); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java index d31834567ab..bfbf7775031 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/LoadBalancersResponse.java @@ -55,6 +55,8 @@ public class LoadBalancersResponse extends HttpResponse { loadBalancers().forEach(lb -> { Cursor lbObject = loadBalancerArray.addObject(); lbObject.setString("id", lb.id().serializedForm()); + lbObject.setString("state", lb.state().name()); + lbObject.setLong("changedAt", lb.changedAt().toEpochMilli()); lbObject.setString("application", lb.id().application().application().value()); lbObject.setString("tenant", lb.id().application().tenant().value()); lbObject.setString("instance", lb.id().application().instance().value()); @@ -76,9 +78,9 @@ public class LoadBalancersResponse extends HttpResponse { realObject.setLong("port", real.port()); }); - lbObject.setArray("rotations"); // To avoid changing the API. This can be removed when clients stop expecting this - - lbObject.setBool("inactive", lb.inactive()); + // TODO(mpolden): The following fields preserves API compatibility. These can be removed once clients stop expecting them + lbObject.setArray("rotations"); + lbObject.setBool("inactive", lb.state() == LoadBalancer.State.inactive); }); new JsonFormat(true).encode(stream, slime); 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 d7942cdb6e7..c0ea64d4c68 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 @@ -21,6 +21,8 @@ import java.util.function.Supplier; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; /** @@ -49,8 +51,8 @@ public class LoadBalancerExpirerTest { // Remove one application deactivates load balancers for that application removeApplication(app1); - assertTrue(loadBalancers.get().get(lb1).inactive()); - assertFalse(loadBalancers.get().get(lb2).inactive()); + assertSame(LoadBalancer.State.inactive, loadBalancers.get().get(lb1).state()); + assertNotSame(LoadBalancer.State.inactive, loadBalancers.get().get(lb2).state()); // Expirer defers removal while nodes are still allocated to application expirer.maintain(); @@ -62,12 +64,12 @@ public class LoadBalancerExpirerTest { assertFalse("Inactive load balancer removed", tester.loadBalancerService().instances().containsKey(lb1)); // Active load balancer is left alone - assertFalse(loadBalancers.get().get(lb2).inactive()); + assertSame(LoadBalancer.State.active, loadBalancers.get().get(lb2).state()); assertTrue("Active load balancer is not removed", tester.loadBalancerService().instances().containsKey(lb2)); } private void dirtyNodesOf(ApplicationId application) { - tester.nodeRepository().setDirty(tester.nodeRepository().getNodes(application), Agent.system, "unit-test"); + tester.nodeRepository().setDirty(tester.nodeRepository().getNodes(application), Agent.system, this.getClass().getSimpleName()); } private void removeApplication(ApplicationId application) { 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 460764b50db..b78b4120b81 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 @@ -12,9 +12,13 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancerInstance; import com.yahoo.vespa.hosted.provision.lb.Real; import org.junit.Test; +import java.nio.charset.StandardCharsets; +import java.time.Instant; import java.util.Optional; +import static java.time.temporal.ChronoUnit.MILLIS; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; /** * @author mpolden @@ -23,31 +27,61 @@ public class LoadBalancerSerializerTest { @Test public void test_serialization() { - LoadBalancer loadBalancer = new LoadBalancer(new LoadBalancerId(ApplicationId.from("tenant1", - "application1", - "default"), - ClusterSpec.Id.from("qrs")), - new LoadBalancerInstance( - HostName.from("lb-host"), - Optional.of(new DnsZone("zone-id-1")), - ImmutableSet.of(4080, 4443), - ImmutableSet.of("10.2.3.4/24"), - ImmutableSet.of(new Real(HostName.from("real-1"), - "127.0.0.1", - 4080), - new Real(HostName.from("real-2"), - "127.0.0.2", - 4080))), - false); - - LoadBalancer serialized = LoadBalancerSerializer.fromJson(LoadBalancerSerializer.toJson(loadBalancer)); + var now = Instant.now(); + var loadBalancer = new LoadBalancer(new LoadBalancerId(ApplicationId.from("tenant1", + "application1", + "default"), + ClusterSpec.Id.from("qrs")), + new LoadBalancerInstance( + HostName.from("lb-host"), + Optional.of(new DnsZone("zone-id-1")), + ImmutableSet.of(4080, 4443), + ImmutableSet.of("10.2.3.4/24"), + ImmutableSet.of(new Real(HostName.from("real-1"), + "127.0.0.1", + 4080), + new Real(HostName.from("real-2"), + "127.0.0.2", + 4080))), + LoadBalancer.State.active, + now); + + var serialized = LoadBalancerSerializer.fromJson(LoadBalancerSerializer.toJson(loadBalancer), now); assertEquals(loadBalancer.id(), serialized.id()); 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.inactive(), serialized.inactive()); + assertEquals(loadBalancer.state(), serialized.state()); + assertEquals(loadBalancer.changedAt().truncatedTo(MILLIS), serialized.changedAt()); assertEquals(loadBalancer.instance().reals(), serialized.instance().reals()); } + @Test + public void test_serialization_legacy() { // TODO(mpolden): Remove after June 2019 + var now = Instant.now(); + + var deserialized = LoadBalancerSerializer.fromJson(legacyJson(true).getBytes(StandardCharsets.UTF_8), now); + assertSame(LoadBalancer.State.inactive, deserialized.state()); + assertEquals(now, deserialized.changedAt()); + + deserialized = LoadBalancerSerializer.fromJson(legacyJson(false).getBytes(StandardCharsets.UTF_8), now); + assertSame(LoadBalancer.State.active, deserialized.state()); + } + + private static String legacyJson(boolean inactive) { + return "{\n" + + " \"id\": \"tenant1:application1:default:qrs\",\n" + + " \"hostname\": \"lb-host\",\n" + + " \"dnsZone\": \"zone-id-1\",\n" + + " \"ports\": [\n" + + " 4080,\n" + + " 4443\n" + + " ],\n" + + " \"networks\": [],\n" + + " \"reals\": [],\n" + + " \"inactive\": " + inactive + "\n" + + "}\n"; + } + } 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 f97460713a5..07cc3aa634c 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 @@ -5,9 +5,9 @@ import com.google.common.collect.Iterators; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.HostSpec; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.RotationName; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.Node; @@ -26,7 +26,6 @@ import java.util.function.Supplier; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; /** @@ -112,18 +111,18 @@ public class LoadBalancerProvisionerTest { removeTransaction.commit(); assertEquals(2, loadBalancers.get().size()); - assertTrue("Deactivated load balancers", loadBalancers.get().stream().allMatch(LoadBalancer::inactive)); + assertTrue("Deactivated load balancers", loadBalancers.get().stream().allMatch(lb -> lb.state() == LoadBalancer.State.inactive)); // Application is redeployed with one cluster and load balancer is re-activated tester.activate(app1, prepare(app1, clusterRequest(ClusterSpec.Type.container, containerCluster1), clusterRequest(ClusterSpec.Type.content, contentCluster))); - assertFalse("Re-activated load balancer for " + containerCluster1, - loadBalancers.get().stream() - .filter(lb -> lb.id().cluster().equals(containerCluster1)) - .findFirst() - .orElseThrow() - .inactive()); + assertEquals("Re-activated load balancer for " + containerCluster1, LoadBalancer.State.active, + loadBalancers.get().stream() + .filter(lb -> lb.id().cluster().equals(containerCluster1)) + .map(LoadBalancer::state) + .findFirst() + .orElseThrow()); } private ClusterSpec clusterRequest(ClusterSpec.Type type, ClusterSpec.Id id) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json index d2c4d0ac857..67d2c3bfa4b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/load-balancers.json @@ -2,6 +2,8 @@ "loadBalancers": [ { "id": "tenant4:application4:instance4:id4", + "state": "active", + "changedAt": 123, "application": "application4", "tenant": "tenant4", "instance": "instance4", |