From cff2eeda1d48fdca3c2379eca8e1318ccb804656 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 18 Nov 2019 14:04:40 +0100 Subject: Create routing policy for empty deployment spec Manual deployments can have a empty deployment spec, without any instances. This change ensures that we create a routing policy and update DNS for such deployments. --- .../controller/maintenance/RoutingPolicies.java | 84 ++++++++++++---------- .../maintenance/RoutingPoliciesTest.java | 45 +++++++++++- 2 files changed, 87 insertions(+), 42 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java index 98483763a0d..70134d5a86d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java @@ -1,7 +1,6 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; -import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; @@ -75,10 +74,11 @@ public class RoutingPolicies { */ public void refresh(ApplicationId application, DeploymentSpec deploymentSpec, ZoneId zone) { if (!controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) return; - var lbs = new AllocatedLoadBalancers(application, zone, controller.serviceRegistry().configServer().getLoadBalancers(application, zone)); + var lbs = new AllocatedLoadBalancers(application, zone, controller.serviceRegistry().configServer().getLoadBalancers(application, zone), + deploymentSpec); try (var lock = db.lockRoutingPolicies()) { - removeObsoleteEndpointsFromDns(lbs, deploymentSpec, lock); - storePoliciesOf(lbs, deploymentSpec, lock); + removeObsoleteEndpointsFromDns(lbs, lock); + storePoliciesOf(lbs, lock); removeObsoletePolicies(lbs, lock); registerEndpointsInDns(lbs, lock); } @@ -104,27 +104,25 @@ public class RoutingPolicies { } /** Store routing policies for given route */ - private void storePoliciesOf(AllocatedLoadBalancers loadBalancers, DeploymentSpec spec, @SuppressWarnings("unused") Lock lock) { - Set policies = new LinkedHashSet<>(get(loadBalancers.application)); + private void storePoliciesOf(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { + var policies = new LinkedHashSet<>(get(loadBalancers.application)); for (LoadBalancer loadBalancer : loadBalancers.list) { - spec.instance(loadBalancer.application().instance()).ifPresent(instanceSpec -> { - RoutingPolicy policy = createPolicy(loadBalancers.application, instanceSpec, loadBalancers.zone, loadBalancer); - if (!policies.add(policy)) { - policies.remove(policy); - policies.add(policy); - } - }); + var endpointIds = loadBalancers.endpointIdsOf(loadBalancer); + var policy = createPolicy(loadBalancers.application, loadBalancers.zone, loadBalancer, endpointIds); + if (!policies.add(policy)) { + // Update existing policy + policies.remove(policy); + policies.add(policy); + } } db.writeRoutingPolicies(loadBalancers.application, policies); } /** Create a policy for given load balancer and register a CNAME for it */ - private RoutingPolicy createPolicy(ApplicationId application, DeploymentInstanceSpec instanceSpec, ZoneId zone, - LoadBalancer loadBalancer) { - var endpoints = endpointIdsOf(loadBalancer, zone, instanceSpec); - var routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, - loadBalancer.hostname(), loadBalancer.dnsZone(), - endpoints); + private RoutingPolicy createPolicy(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer, + Set endpointIds) { + var routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, loadBalancer.hostname(), + loadBalancer.dnsZone(), endpointIds); var name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName()); var data = RecordData.fqdn(loadBalancer.hostname().value()); controller.nameServiceForwarder().createCname(name, data, Priority.normal); @@ -150,26 +148,24 @@ public class RoutingPolicies { } /** Remove unreferenced global endpoints for given route from DNS */ - private void removeObsoleteEndpointsFromDns(AllocatedLoadBalancers loadBalancers, DeploymentSpec deploymentSpec, @SuppressWarnings("unused") Lock lock) { + private void removeObsoleteEndpointsFromDns(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { var zonePolicies = get(loadBalancers.application, loadBalancers.zone); var removalCandidates = routingTableFrom(zonePolicies).keySet(); - var activeRoutingIds = routingIdsFrom(loadBalancers, deploymentSpec); + var activeRoutingIds = routingIdsFrom(loadBalancers); removalCandidates.removeAll(activeRoutingIds); for (var id : removalCandidates) { - Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.endpointId(), controller.system()); + var endpoint = RoutingPolicy.endpointOf(id.application(), id.endpointId(), controller.system()); controller.nameServiceForwarder().removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()), Priority.normal); } } /** Compute routing IDs from given load balancers */ - private static Set routingIdsFrom(AllocatedLoadBalancers loadBalancers, DeploymentSpec spec) { + private static Set routingIdsFrom(AllocatedLoadBalancers loadBalancers) { Set routingIds = new LinkedHashSet<>(); for (var loadBalancer : loadBalancers.list) { - spec.instance(loadBalancer.application().instance()).ifPresent(instanceSpec -> { - for (var endpointId : endpointIdsOf(loadBalancer, loadBalancers.zone, instanceSpec)) { - routingIds.add(new RoutingId(loadBalancer.application(), endpointId)); - } - }); + for (var endpointId : loadBalancers.endpointIdsOf(loadBalancer)) { + routingIds.add(new RoutingId(loadBalancer.application(), endpointId)); + } } return Collections.unmodifiableSet(routingIds); } @@ -187,29 +183,39 @@ public class RoutingPolicies { return routingTable; } - /** Compute all endpoint IDs of given load balancer */ - private static Set endpointIdsOf(LoadBalancer loadBalancer, ZoneId zone, DeploymentInstanceSpec spec) { - return spec.endpoints().stream() - .filter(endpoint -> endpoint.containerId().equals(loadBalancer.cluster().value())) - .filter(endpoint -> endpoint.regions().contains(zone.region())) - .map(com.yahoo.config.application.api.Endpoint::endpointId) - .map(EndpointId::of) - .collect(Collectors.toSet()); - } - /** Load balancers allocated to a deployment */ private static class AllocatedLoadBalancers { private final ApplicationId application; private final ZoneId zone; private final List list; + private final DeploymentSpec deploymentSpec; - private AllocatedLoadBalancers(ApplicationId application, ZoneId zone, List loadBalancers) { + private AllocatedLoadBalancers(ApplicationId application, ZoneId zone, List loadBalancers, + DeploymentSpec deploymentSpec) { this.application = application; this.zone = zone; this.list = loadBalancers.stream() .filter(AllocatedLoadBalancers::shouldUpdatePolicy) .collect(Collectors.toUnmodifiableList()); + this.deploymentSpec = deploymentSpec; + } + + /** Compute all endpoint IDs for given load balancer */ + private Set endpointIdsOf(LoadBalancer loadBalancer) { + if (zone.environment().isManuallyDeployed()) { // Manual deployments do not have any configurable endpoints + return Set.of(); + } + var instanceSpec = deploymentSpec.instance(loadBalancer.application().instance()); + if (instanceSpec.isEmpty()) { + return Set.of(); + } + return instanceSpec.get().endpoints().stream() + .filter(endpoint -> endpoint.containerId().equals(loadBalancer.cluster().value())) + .filter(endpoint -> endpoint.regions().contains(zone.region())) + .map(com.yahoo.config.application.api.Endpoint::endpointId) + .map(EndpointId::of) + .collect(Collectors.toSet()); } private static boolean shouldUpdatePolicy(LoadBalancer loadBalancer) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java index bc184715589..c9ec03bd1af 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java @@ -1,12 +1,14 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; +import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Instance; +import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; @@ -16,6 +18,7 @@ import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import org.junit.Test; import java.net.URI; @@ -51,7 +54,7 @@ public class RoutingPoliciesTest { .build(); @Test - public void maintains_global_routing_policies() { + public void global_routing_policies() { int clustersPerZone = 2; int numberOfDeployments = 2; var applicationPackage = new ApplicationPackageBuilder() @@ -137,10 +140,9 @@ public class RoutingPoliciesTest { } @Test - public void maintains_routing_policies_per_zone() { + public void zone_routing_policies() { // Deploy application int clustersPerZone = 2; - int buildNumber = 42; provisionLoadBalancers(clustersPerZone, context1.instanceId(), zone1, zone2); context1.submit(applicationPackage).deploy(); @@ -238,6 +240,43 @@ public class RoutingPoliciesTest { tester.controller().applications().clusterEndpoints(context1.deploymentIdIn(zone1))); } + @Test + public void manual_deployment_creates_routing_policy() { + // Empty application package is valid in manually deployed environments + var emptyApplicationPackage = new ApplicationPackageBuilder().build(); + var zone = ZoneId.from("dev", "us-east-1"); + tester.controllerTester().serviceRegistry().zoneRegistry().setZones(ZoneApiMock.from(zone.environment(), zone.region())); + provisionLoadBalancers(1, context1.instanceId(), zone); + + // Deploy to dev + tester.controller().applications().deploy(context1.instanceId(), zone, Optional.of(emptyApplicationPackage), DeployOptions.none()); + assertEquals("DeploymentSpec is not persisted", DeploymentSpec.empty, context1.application().deploymentSpec()); + context1.flushDnsUpdates(); + + // Routing policy is created and DNS is updated + assertEquals(1, policies(context1.instance()).size()); + assertEquals(Set.of("c0.app1.tenant1.us-east-1.dev.vespa.oath.cloud"), recordNames()); + } + + @Test + public void manual_deployment_creates_routing_policy_with_non_empty_spec() { + // Initial deployment + context1.submit(applicationPackage).deploy(); + var zone = ZoneId.from("dev", "us-east-1"); + tester.controllerTester().serviceRegistry().zoneRegistry().setZones(ZoneApiMock.from(zone.environment(), zone.region())); + + // Deploy to dev under different instance + var devInstance = context1.application().id().instance("user"); + provisionLoadBalancers(1, devInstance, zone); + tester.controller().applications().deploy(devInstance, zone, Optional.of(applicationPackage), DeployOptions.none()); + assertEquals("DeploymentSpec is persisted", applicationPackage.deploymentSpec(), context1.application().deploymentSpec()); + context1.flushDnsUpdates(); + + // Routing policy is created and DNS is updated + assertEquals(1, policies(tester.instance(devInstance)).size()); + assertEquals(Set.of("c0.user.app1.tenant1.us-east-1.dev.vespa.oath.cloud"), recordNames()); + } + private Set policies(Instance instance) { return tester.controller().curator().readRoutingPolicies(instance.id()); } -- cgit v1.2.3