diff options
10 files changed, 113 insertions, 69 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/EndpointStatus.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/EndpointStatus.java index 2f1b93158ab..4b326bc7430 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/EndpointStatus.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/EndpointStatus.java @@ -1,59 +1,45 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.application.v4.model; +import java.time.Instant; +import java.util.Objects; + /** - * Represent the operational status of a service endpoint (where the endpoint itself - * is identified by the container cluster id). - * - * The status of an endpoint may be assigned from the controller. + * Represent the routing status for all endpoints of a deployment. * * @author smorgrav */ public class EndpointStatus { + private final String agent; - private final String reason; private final Status status; - private final long epoch; + private final Instant changedAt; - public enum Status { - in, - out, - unknown; + public EndpointStatus(Status status, String agent, Instant changedAt) { + this.status = Objects.requireNonNull(status); + this.agent = Objects.requireNonNull(agent); + this.changedAt = Objects.requireNonNull(changedAt); } - public EndpointStatus(Status status, String reason, String agent, long epoch) { - this.status = status; - this.reason = reason; - this.agent = agent; - this.epoch = epoch; - } - - /** - * @return The agent responsible setting this status - */ - public String getAgent() { + /** Returns the agent responsible setting this status */ + public String agent() { return agent; } - /** - * @return The reason for this status (e.g. 'incident INCXXX') - */ - public String getReason() { - return reason; + /** Returns the current status */ + public Status status() { + return status; } - /** - * @return The current status - */ - public Status getStatus() { - return status; + /** Returns when this was last changed */ + public Instant changedAt() { + return changedAt; } - /** - * @return The epoch for when this status became active, in seconds - */ - public long getEpoch() { - return epoch; + public enum Status { + in, + out, + unknown; } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index 122f836c908..7f9b693f413 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.configserver; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; @@ -85,12 +84,12 @@ public interface ConfigServer { /** * Set new status for a endpoint of a single deployment. * - * @param deployment The deployment to change - * @param upstreamName The upstream to modify. Upstream name is a unique identifier for the global route of a - * deployment in the shared routing layer - * @param status The new status + * @param deployment The deployment to change + * @param upstreamNames The upstream names to modify. Upstream name is a unique identifier for the routing status + * of a cluster in a deployment + * @param status The new status */ - void setGlobalRotationStatus(DeploymentId deployment, String upstreamName, EndpointStatus status); + void setGlobalRotationStatus(DeploymentId deployment, List<String> upstreamNames, EndpointStatus status); /** * Set the new status for an entire zone. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 943d6ac7b18..c58bb0e5fab 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -74,6 +74,7 @@ public class RoutingController { private final RoutingPolicies routingPolicies; private final RotationRepository rotationRepository; private final BooleanFlag hideSharedRoutingEndpoint; + private final BooleanFlag changeRoutingStatusOfAllUpstreams; public RoutingController(Controller controller, RotationsConfig rotationsConfig) { this.controller = Objects.requireNonNull(controller, "controller must be non-null"); @@ -82,6 +83,7 @@ public class RoutingController { controller.applications(), controller.curator()); this.hideSharedRoutingEndpoint = Flags.HIDE_SHARED_ROUTING_ENDPOINT.bindTo(controller.flagSource()); + this.changeRoutingStatusOfAllUpstreams = Flags.CHANGE_ROUTING_STATUS_OF_ALL_UPSTREAMS.bindTo(controller.flagSource()); } /** Create a routing context for given deployment */ @@ -90,7 +92,8 @@ public class RoutingController { return new SharedDeploymentRoutingContext(deployment, this, controller.serviceRegistry().configServer(), - controller.clock()); + controller.clock(), + changeRoutingStatusOfAllUpstreams.value()); } return new ExclusiveDeploymentRoutingContext(deployment, this); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index 544822e3be3..8f37d287c1a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -133,10 +133,10 @@ public class Endpoint { return routingMethod.isShared() && scope == Scope.global; } - /** Returns the upstream ID of given deployment. This *must* match what the routing layer generates */ - public String upstreamIdOf(DeploymentId deployment) { + /** Returns the upstream name of given deployment. This *must* match what the routing layer generates */ + public String upstreamName(DeploymentId deployment) { if (!routingMethod.isShared()) throw new IllegalArgumentException("Routing method " + routingMethod + " does not have upstream name"); - return upstreamIdOf(cluster.value(), deployment.applicationId(), deployment.zoneId()); + return upstreamName(cluster.value(), deployment.applicationId(), deployment.zoneId()); } @Override @@ -269,7 +269,7 @@ public class Endpoint { return suffix; } - private static String upstreamIdOf(String name, ApplicationId application, ZoneId zone) { + private static String upstreamName(String name, ApplicationId application, ZoneId zone) { return Stream.of(namePart(name, ""), instancePart(Optional.of(application.instance()), ""), application.application().value(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index ed16c86c94e..cf85d862041 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -1555,7 +1555,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (primaryEndpoint.isPresent()) { DeploymentRoutingContext context = controller.routing().of(deploymentId); RoutingStatus status = context.routingStatus(); - array.addString(primaryEndpoint.get().upstreamIdOf(deploymentId)); + array.addString(primaryEndpoint.get().upstreamName(deploymentId)); Cursor statusObject = array.addObject(); statusObject.setString("status", status.value().name()); statusObject.setString("reason", ""); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java index 6fd8a3a84d5..e784666f22a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/context/DeploymentRoutingContext.java @@ -16,7 +16,7 @@ import com.yahoo.vespa.hosted.controller.routing.RoutingPolicyId; import com.yahoo.vespa.hosted.controller.routing.RoutingStatus; import java.time.Clock; -import java.time.Instant; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -71,48 +71,99 @@ public abstract class DeploymentRoutingContext implements RoutingContext { private final Clock clock; private final ConfigServer configServer; + private final boolean changeAllUpstreams; - public SharedDeploymentRoutingContext(DeploymentId deployment, RoutingController controller, ConfigServer configServer, Clock clock) { + public SharedDeploymentRoutingContext(DeploymentId deployment, RoutingController controller, ConfigServer configServer, Clock clock, boolean changeAllUpstreams) { super(deployment, RoutingMethod.shared, controller); this.clock = Objects.requireNonNull(clock); this.configServer = Objects.requireNonNull(configServer); + this.changeAllUpstreams = changeAllUpstreams; } @Override public void setRoutingStatus(RoutingStatus.Value value, RoutingStatus.Agent agent) { + if (!changeAllUpstreams) { + setLegacyRoutingStatus(value, agent); + return; + } + EndpointStatus newStatus = new EndpointStatus(value == RoutingStatus.Value.in + ? EndpointStatus.Status.in + : EndpointStatus.Status.out, + agent.name(), + clock.instant()); + try { + configServer.setGlobalRotationStatus(deployment, upstreamNames(), newStatus); + } catch (Exception e) { + throw new RuntimeException("Failed to change rotation status of " + deployment, e); + } + } + + @Override + public RoutingStatus routingStatus() { + if (!changeAllUpstreams) { + return legacyRoutingStatus(); + } + + // In a given deployment, all upstreams (clusters) share the same status, so we can query using any + // upstream name + String upstreamName = upstreamNames().get(0); + EndpointStatus status = configServer.getGlobalRotationStatus(deployment, upstreamName); + RoutingStatus.Agent agent; + try { + agent = RoutingStatus.Agent.valueOf(status.agent().toLowerCase()); + } catch (IllegalArgumentException e) { + agent = RoutingStatus.Agent.unknown; + } + return new RoutingStatus(status.status() == EndpointStatus.Status.in + ? RoutingStatus.Value.in + : RoutingStatus.Value.out, + agent, + status.changedAt()); + } + + private List<String> upstreamNames() { + List<String> upstreamNames = controller.readEndpointsOf(deployment) + .scope(Endpoint.Scope.zone) + .shared() + .mapToList(endpoint -> endpoint.upstreamName(deployment)); + if (upstreamNames.isEmpty()) { + throw new IllegalArgumentException("No upstream names found for " + deployment); + } + return upstreamNames; + } + + private void setLegacyRoutingStatus(RoutingStatus.Value value, RoutingStatus.Agent agent) { EndpointStatus newStatus = new EndpointStatus(value == RoutingStatus.Value.in ? EndpointStatus.Status.in : EndpointStatus.Status.out, - "", agent.name(), - clock.instant().getEpochSecond()); + clock.instant()); primaryEndpoint().ifPresent(endpoint -> { try { - configServer.setGlobalRotationStatus(deployment, endpoint.upstreamIdOf(deployment), newStatus); + configServer.setGlobalRotationStatus(deployment, List.of(endpoint.upstreamName(deployment)), newStatus); } catch (Exception e) { throw new RuntimeException("Failed to set rotation status of " + endpoint + " in " + deployment, e); } }); } - @Override - public RoutingStatus routingStatus() { + private RoutingStatus legacyRoutingStatus() { Optional<EndpointStatus> status = primaryEndpoint().map(endpoint -> { - var upstreamName = endpoint.upstreamIdOf(deployment); + var upstreamName = endpoint.upstreamName(deployment); return configServer.getGlobalRotationStatus(deployment, upstreamName); }); if (status.isEmpty()) return RoutingStatus.DEFAULT; RoutingStatus.Agent agent; try { - agent = RoutingStatus.Agent.valueOf(status.get().getAgent().toLowerCase()); + agent = RoutingStatus.Agent.valueOf(status.get().agent().toLowerCase()); } catch (IllegalArgumentException e) { agent = RoutingStatus.Agent.unknown; } - return new RoutingStatus(status.get().getStatus() == EndpointStatus.Status.in + return new RoutingStatus(status.get().status() == EndpointStatus.Status.in ? RoutingStatus.Value.in : RoutingStatus.Value.out, agent, - Instant.ofEpochSecond(status.get().getEpoch())); + status.get().changedAt()); } private Optional<Endpoint> primaryEndpoint() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java index 46d27911de4..e50c32d0e5d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java @@ -387,8 +387,8 @@ public class EndpointTest { "c2.i2.a2.t2.us-north-1.prod", Endpoint.of(instance2).target(EndpointId.of("ignored2"), ClusterSpec.Id.from("c2"), List.of(zone2)).on(Port.tls(4443)).in(SystemName.main) ); - tests1.forEach((expected, endpoint) -> assertEquals(expected, endpoint.upstreamIdOf(zone))); - tests2.forEach((expected, endpoint) -> assertEquals(expected, endpoint.upstreamIdOf(zone2))); + tests1.forEach((expected, endpoint) -> assertEquals(expected, endpoint.upstreamName(zone))); + tests2.forEach((expected, endpoint) -> assertEquals(expected, endpoint.upstreamName(zone2))); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index f2fc624630c..aa53d09be04 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -80,7 +80,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer private final Map<DeploymentId, Application> applications = new LinkedHashMap<>(); private final Set<ZoneId> inactiveZones = new HashSet<>(); - private final Map<String, EndpointStatus> endpoints = new HashMap<>(); + private final Map<DeploymentId, EndpointStatus> endpoints = new HashMap<>(); private final NodeRepositoryMock nodeRepository = new NodeRepositoryMock(); private final Map<DeploymentId, ServiceConvergence> serviceStatus = new HashMap<>(); private final Set<ApplicationId> disallowConvergenceCheckApplications = new HashSet<>(); @@ -536,8 +536,8 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public void setGlobalRotationStatus(DeploymentId deployment, String upstreamName, EndpointStatus status) { - endpoints.put(upstreamName, status); + public void setGlobalRotationStatus(DeploymentId deployment, List<String> upstreamNames, EndpointStatus status) { + endpoints.put(deployment, status); } @Override @@ -550,9 +550,9 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public EndpointStatus getGlobalRotationStatus(DeploymentId deployment, String endpoint) { - EndpointStatus result = new EndpointStatus(EndpointStatus.Status.in, "", "", 1497618757L); - return endpoints.getOrDefault(endpoint, result); + public EndpointStatus getGlobalRotationStatus(DeploymentId deployment, String upstreamName) { + EndpointStatus status = new EndpointStatus(EndpointStatus.Status.in, "", Instant.ofEpochSecond(1497618757L)); + return endpoints.getOrDefault(deployment, status); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 6e8445102c3..6cf3e89bdfe 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -52,7 +52,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.resource.MeteringData; import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceAllocation; import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshot; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMeteringClient; -import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; @@ -1877,7 +1876,7 @@ public class ApplicationApiTest extends ControllerContainerTest { RoutingStatus status = context.routingStatus(); assertEquals(value, status.value()); assertEquals(agent, status.agent()); - assertEquals(changedAt.truncatedTo(ChronoUnit.SECONDS), status.changedAt()); + assertEquals(changedAt, status.changedAt()); } private static class RequestBuilder implements Supplier<Request> { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 4e27733f631..85cee0d151b 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -386,6 +386,12 @@ public class Flags { "Takes effect on container reboot", ZONE_ID, HOSTNAME); + public static final UnboundBooleanFlag CHANGE_ROUTING_STATUS_OF_ALL_UPSTREAMS = defineFeatureFlag( + "change-routing-status-of-all-upstreams", false, + List.of("mpolden"), "2021-12-02", "2021-12-15", + "Whether the controller should send all known upstreams to configserver when changing routing status of a deployment", + "Takes effect on the next change in routing status"); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, |