diff options
author | Martin Polden <mpolden@mpolden.no> | 2020-11-12 14:56:19 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2020-11-12 16:37:05 +0100 |
commit | 0bf5758475b7399d7e8502a27093eef5ab8f324a (patch) | |
tree | 7becbb952c34cd4f072a97f646cc2639f2b5076d /controller-server | |
parent | 4ab1c5903a1f5f1d3c8fd8a8cec802e5e0ab4a73 (diff) |
Simplify rotation check
Diffstat (limited to 'controller-server')
2 files changed, 45 insertions, 10 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiHandler.java index ace176bd91e..9e73e9c9747 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiHandler.java @@ -18,7 +18,6 @@ import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; @@ -205,8 +204,8 @@ public class RoutingApiHandler extends AuditLoggingRequestHandler { var status = in ? GlobalRouting.Status.in : GlobalRouting.Status.out; var agent = GlobalRouting.Agent.operator; // Always operator as this is an operator API - // Set rotation status, if any assigned - if (rotationCanRouteTo(deployment.zoneId(), instance)) { + // Set rotation status, if rotations can route to this zone + if (rotationCanRouteTo(deployment.zoneId())) { var endpointStatus = new EndpointStatus(in ? EndpointStatus.Status.in : EndpointStatus.Status.out, "", agent.name(), controller.clock().instant().getEpochSecond()); @@ -241,7 +240,7 @@ public class RoutingApiHandler extends AuditLoggingRequestHandler { for (var zone : zones) { var deploymentId = new DeploymentId(instance.id(), zone); // Include status from rotation - if (rotationCanRouteTo(zone, instance)) { + if (rotationCanRouteTo(zone)) { var rotationStatus = controller.routing().globalRotationStatus(deploymentId); // Status is equal across all global endpoints, as the status is per deployment, not per endpoint. var endpointStatus = rotationStatus.values().stream().findFirst(); @@ -277,11 +276,12 @@ public class RoutingApiHandler extends AuditLoggingRequestHandler { } - /** Returns whether instance has an assigned rotation that can route to given zone */ - private boolean rotationCanRouteTo(ZoneId zone, Instance instance) { - return !instance.rotations().isEmpty() && - instance.deployments().containsKey(zone) && - controller.zoneRegistry().routingMethods(zone).get(0).isShared(); + /** Returns whether a rotation can route traffic to given zone */ + private boolean rotationCanRouteTo(ZoneId zone) { + // A system may support multiple routing methods, i.e. it has both exclusively routed zones and zones using + // shared routing. When changing or reading routing status in the context of a specific deployment, rotation + // status should only be considered if the zone supports shared routing. + return controller.zoneRegistry().routingMethods(zone).stream().anyMatch(RoutingMethod::isShared); } private static void zoneStatusToSlime(Cursor object, ZoneId zone, GlobalRouting globalRouting, RoutingMethod method) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiTest.java index 9bd8485db77..175d7d3705b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/routing/RoutingApiTest.java @@ -262,7 +262,7 @@ public class RoutingApiTest extends ControllerContainerTest { // TODO(mpolden): Remove this once a zone supports either of routing policy and rotation @Test - public void mixed_routing() { + public void mixed_routing_single_zone() { var westZone = ZoneId.from("prod", "us-west-1"); var eastZone = ZoneId.from("prod", "us-east-3"); @@ -303,6 +303,41 @@ public class RoutingApiTest extends ControllerContainerTest { } @Test + public void mixed_routing_multiple_zones() { + var westZone = ZoneId.from("prod", "us-west-1"); + var eastZone = ZoneId.from("prod", "us-east-3"); + + // One shared and one exclusive zone + deploymentTester.controllerTester().zoneRegistry().setRoutingMethod(ZoneApiMock.from(westZone), + RoutingMethod.shared); + deploymentTester.controllerTester().zoneRegistry().setRoutingMethod(ZoneApiMock.from(eastZone), + RoutingMethod.exclusive); + + // Deploy application + var context = deploymentTester.newDeploymentContext(); + var applicationPackage = new ApplicationPackageBuilder() + .region(westZone.region()) + .region(eastZone.region()) + .athenzIdentity(AthenzDomain.from("domain"), AthenzService.from("service")) + .compileVersion(RoutingController.DIRECT_ROUTING_MIN_VERSION) + .endpoint("endpoint1", "default", westZone.region().value()) + .endpoint("endpoint2", "default", eastZone.region().value()) + .build(); + context.submit(applicationPackage).deploy(); + + // GET status for zone using shared routing + tester.assertResponse(operatorRequest("http://localhost:8080/routing/v1/status/tenant/tenant/application/application/instance/default/environment/prod/region/us-west-1", + "", Request.Method.GET), + new File("rotation/deployment-status-initial.json")); + + // GET status for zone using exclusive routing + tester.assertResponse(operatorRequest("http://localhost:8080/routing/v1/status/tenant/tenant/application/application/instance/default/environment/prod/region/us-east-3", + "", Request.Method.GET), + "{\"deployments\":[{\"routingMethod\":\"exclusive\",\"instance\":\"tenant:application:default\"," + + "\"environment\":\"prod\",\"region\":\"us-east-3\",\"status\":\"in\",\"agent\":\"system\",\"changedAt\":0}]}"); + } + + @Test public void invalid_requests() { // GET non-existent application tester.assertResponse(operatorRequest("http://localhost:8080/routing/v1/status/tenant/t1/application/a1/instance/default/environment/prod/region/us-west-1", |