diff options
13 files changed, 270 insertions, 306 deletions
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 bcad82a23c1..43ae29ee922 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 @@ -1,11 +1,12 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.configserver; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; 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.api.identifiers.Hostname; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import java.io.IOException; @@ -76,4 +77,7 @@ public interface ConfigServer { /** Get all load balancers in given zone */ List<LoadBalancer> getLoadBalancers(ZoneId zone); + /** Get all load balancers for application in given zone */ + List<LoadBalancer> getLoadBalancers(ApplicationId application, ZoneId zone); + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 9a489f8379d..8ca6373166b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -55,6 +55,7 @@ import com.yahoo.vespa.hosted.controller.concurrent.Once; import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; +import com.yahoo.vespa.hosted.controller.maintenance.RoutingPolicies; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.rotation.Rotation; import com.yahoo.vespa.hosted.controller.rotation.RotationId; @@ -115,9 +116,9 @@ public class ApplicationController { private final AccessControl accessControl; private final ConfigServer configServer; private final RoutingGenerator routingGenerator; + private final RoutingPolicies routingPolicies; private final Clock clock; private final BooleanFlag redirectLegacyDnsFlag; - private final DeploymentTrigger deploymentTrigger; ApplicationController(Controller controller, CuratorDb curator, @@ -130,6 +131,7 @@ public class ApplicationController { this.accessControl = accessControl; this.configServer = configServer; this.routingGenerator = routingGenerator; + this.routingPolicies = new RoutingPolicies(controller); this.clock = clock; this.redirectLegacyDnsFlag = Flags.REDIRECT_LEGACY_DNS_NAMES.bindTo(controller.flagSource()); @@ -327,6 +329,7 @@ public class ApplicationController { cnames = app.endpointsIn(controller.system()).asList().stream().map(Endpoint::dnsName).collect(Collectors.toSet()); // Include rotation ID to ensure that deployment can respond to health checks with rotation ID as Host header app.rotation().map(RotationId::asString).ifPresent(cnames::add); + // Update application with information from application package if ( ! preferOldestVersion && ! application.get().deploymentJobs().deployedInternally() @@ -422,6 +425,11 @@ public class ApplicationController { ConfigServer.PreparedApplication preparedApplication = configServer.deploy(deploymentId, deployOptions, cnames, rotationNames, applicationPackage.zippedContent()); + + // Refresh routing policies on successful deployment. At this point we can safely assume that the config server + // has allocated load balancers for the deployment. + routingPolicies.refresh(application, zone); + return new ActivateResult(new RevisionId(applicationPackage.hash()), preparedApplication.prepareResponse(), applicationPackage.zippedContent().length); } @@ -643,9 +651,10 @@ public class ApplicationController { private LockedApplication deactivate(LockedApplication application, ZoneId zone) { try { configServer.deactivate(new DeploymentId(application.get().id(), zone)); - } - catch (NotFoundException ignored) { + } catch (NotFoundException ignored) { // ok; already gone + } finally { + routingPolicies.refresh(application.get().id(), zone); } return application.withoutDeploymentIn(zone); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index aec4c7a915c..3f5f8273922 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -53,7 +53,6 @@ public class ControllerMaintenance extends AbstractComponent { private final JobRunner jobRunner; private final ContactInformationMaintainer contactInformationMaintainer; private final CostReportMaintainer costReportMaintainer; - private final RoutingPolicyMaintainer routingPolicyMaintainer; private final ResourceMeterMaintainer resourceMeterMaintainer; private final NameServiceDispatcher nameServiceDispatcher; @@ -86,7 +85,6 @@ public class ControllerMaintenance extends AbstractComponent { osVersionStatusUpdater = new OsVersionStatusUpdater(controller, maintenanceInterval, jobControl); contactInformationMaintainer = new ContactInformationMaintainer(controller, Duration.ofHours(12), jobControl, contactRetriever); costReportMaintainer = new CostReportMaintainer(controller, Duration.ofHours(2), reportConsumer, jobControl, nodeRepositoryClient, Clock.systemUTC(), selfHostedCostConfig); - routingPolicyMaintainer = new RoutingPolicyMaintainer(controller, Duration.ofMinutes(5), jobControl, curator); resourceMeterMaintainer = new ResourceMeterMaintainer(controller, Duration.ofMinutes(60), jobControl, nodeRepositoryClient, Clock.systemUTC(), metric, resourceSnapshotConsumer); nameServiceDispatcher = new NameServiceDispatcher(controller, Duration.ofSeconds(10), jobControl, nameService); } @@ -116,7 +114,6 @@ public class ControllerMaintenance extends AbstractComponent { jobRunner.deconstruct(); contactInformationMaintainer.deconstruct(); costReportMaintainer.deconstruct(); - routingPolicyMaintainer.deconstruct(); resourceMeterMaintainer.deconstruct(); nameServiceDispatcher.deconstruct(); } 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 new file mode 100644 index 00000000000..c9b46ff9dcf --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicies.java @@ -0,0 +1,187 @@ +// 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.provision.ApplicationId; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; +import com.yahoo.vespa.hosted.controller.api.integration.dns.AliasTarget; +import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; +import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; +import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; +import com.yahoo.vespa.hosted.controller.application.Endpoint; +import com.yahoo.vespa.hosted.controller.application.RoutingId; +import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; +import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; +import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Updates routing policies and their associated DNS records based on an deployment's load balancers. + * + * @author mortent + * @author mpolden + */ +public class RoutingPolicies { + + private final Controller controller; + private final CuratorDb db; + + public RoutingPolicies(Controller controller) { + this.controller = Objects.requireNonNull(controller, "controller must be non-null"); + this.db = controller.curator(); + try (Lock lock = db.lockRoutingPolicies()) { // Update serialized format + for (var policy : db.readRoutingPolicies().entrySet()) { + db.writeRoutingPolicies(policy.getKey(), policy.getValue()); + } + } + } + + + /** + * Refresh routing policies for application in given zone. This is idempotent and changes will only be performed if + * load balancers for given application have changed. + */ + public void refresh(ApplicationId application, ZoneId zone) { + var lbs = new LoadBalancers(application, zone, controller.applications().configServer() + .getLoadBalancers(application, zone)); + removeObsoleteEndpointsFromDns(lbs); + storePoliciesOf(lbs); + removeObsoletePolicies(lbs); + registerEndpointsInDns(lbs); + } + + /** Create global endpoints for given route, if any */ + private void registerEndpointsInDns(LoadBalancers loadBalancers) { + try (Lock lock = db.lockRoutingPolicies()) { + Map<RoutingId, List<RoutingPolicy>> routingTable = routingTableFrom(db.readRoutingPolicies(loadBalancers.application)); + + // Create DNS record for each routing ID + for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { + Endpoint endpoint = RoutingPolicy.endpointOf(routeEntry.getKey().application(), routeEntry.getKey().rotation(), + controller.system()); + Set<AliasTarget> targets = routeEntry.getValue() + .stream() + .filter(policy -> policy.dnsZone().isPresent()) + .map(policy -> new AliasTarget(policy.canonicalName(), + policy.dnsZone().get(), + policy.zone())) + .collect(Collectors.toSet()); + controller.nameServiceForwarder().createAlias(RecordName.from(endpoint.dnsName()), targets, Priority.normal); + } + } + } + + /** Store routing policies for given route */ + private void storePoliciesOf(LoadBalancers loadBalancers) { + try (Lock lock = db.lockRoutingPolicies()) { + Set<RoutingPolicy> policies = new LinkedHashSet<>(db.readRoutingPolicies(loadBalancers.application)); + for (LoadBalancer loadBalancer : loadBalancers.list) { + RoutingPolicy policy = createPolicy(loadBalancers.application, loadBalancers.zone, loadBalancer); + if (!policies.add(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, ZoneId zone, LoadBalancer loadBalancer) { + RoutingPolicy routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, + loadBalancer.hostname(), loadBalancer.dnsZone(), + loadBalancer.rotations()); + RecordName name = RecordName.from(routingPolicy.endpointIn(controller.system()).dnsName()); + RecordData data = RecordData.fqdn(loadBalancer.hostname().value()); + controller.nameServiceForwarder().createCname(name, data, Priority.normal); + return routingPolicy; + } + + /** Remove obsolete policies for given route and their CNAME records */ + private void removeObsoletePolicies(LoadBalancers loadBalancers) { + try (Lock lock = db.lockRoutingPolicies()) { + var allPolicies = new LinkedHashSet<>(db.readRoutingPolicies(loadBalancers.application)); + var removalCandidates = new HashSet<>(allPolicies); + var activeLoadBalancers = loadBalancers.list.stream() + .map(LoadBalancer::hostname) + .collect(Collectors.toSet()); + // Remove active load balancers and irrelevant zones from candidates + removalCandidates.removeIf(policy -> activeLoadBalancers.contains(policy.canonicalName()) || + !policy.zone().equals(loadBalancers.zone)); + for (var policy : removalCandidates) { + var dnsName = policy.endpointIn(controller.system()).dnsName(); + controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(dnsName), Priority.normal); + allPolicies.remove(policy); + } + db.writeRoutingPolicies(loadBalancers.application, allPolicies); + } + } + + /** Remove unreferenced global endpoints for given route from DNS */ + private void removeObsoleteEndpointsFromDns(LoadBalancers loadBalancers) { + try (Lock lock = db.lockRoutingPolicies()) { + var zonePolicies = db.readRoutingPolicies(loadBalancers.application).stream() + .filter(policy -> policy.zone().equals(loadBalancers.zone)) + .collect(Collectors.toUnmodifiableSet()); + var removalCandidates = routingTableFrom(zonePolicies).keySet(); + var activeRoutingIds = routingIdsFrom(loadBalancers.list); + removalCandidates.removeAll(activeRoutingIds); + for (var id : removalCandidates) { + Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.rotation(), controller.system()); + controller.nameServiceForwarder().removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()), Priority.normal); + } + } + } + + /** Compute routing IDs from given load balancers */ + private static Set<RoutingId> routingIdsFrom(List<LoadBalancer> loadBalancers) { + Set<RoutingId> routingIds = new LinkedHashSet<>(); + for (var loadBalancer : loadBalancers) { + for (var rotation : loadBalancer.rotations()) { + routingIds.add(new RoutingId(loadBalancer.application(), rotation)); + } + } + return Collections.unmodifiableSet(routingIds); + } + + /** Compute a routing table from given policies */ + private static Map<RoutingId, List<RoutingPolicy>> routingTableFrom(Set<RoutingPolicy> routingPolicies) { + var routingTable = new LinkedHashMap<RoutingId, List<RoutingPolicy>>(); + for (var policy : routingPolicies) { + for (var rotation : policy.rotations()) { + var id = new RoutingId(policy.owner(), rotation); + routingTable.putIfAbsent(id, new ArrayList<>()); + routingTable.get(id).add(policy); + } + } + return routingTable; + } + + /** Load balancers for a particular deployment */ + private static class LoadBalancers { + + private final ApplicationId application; + private final ZoneId zone; + private final List<LoadBalancer> list; + + private LoadBalancers(ApplicationId application, ZoneId zone, List<LoadBalancer> list) { + this.application = application; + this.zone = zone; + this.list = list; + } + + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java deleted file mode 100644 index 0ddc24147ee..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java +++ /dev/null @@ -1,224 +0,0 @@ -// 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.provision.ApplicationId; -import com.yahoo.config.provision.RotationName; -import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.log.LogLevel; -import com.yahoo.vespa.curator.Lock; -import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; -import com.yahoo.vespa.hosted.controller.api.integration.dns.AliasTarget; -import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; -import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; -import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; -import com.yahoo.vespa.hosted.controller.application.Endpoint; -import com.yahoo.vespa.hosted.controller.application.RoutingId; -import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; -import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder; -import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; -import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; - -import java.time.Duration; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.logging.Logger; -import java.util.stream.Collectors; - -/** - * Maintains routing policies and their DNS records for all exclusive load balancers in this system. - * - * @author mortent - * @author mpolden - */ -public class RoutingPolicyMaintainer extends Maintainer { - - private static final Logger log = Logger.getLogger(RoutingPolicyMaintainer.class.getName()); - - private final NameServiceForwarder nameServiceForwarder; - private final CuratorDb db; - - public RoutingPolicyMaintainer(Controller controller, - Duration interval, - JobControl jobControl, - CuratorDb db) { - super(controller, interval, jobControl); - this.nameServiceForwarder = controller.nameServiceForwarder(); - this.db = db; - // Update serialized format - try (Lock lock = db.lockRoutingPolicies()) { - for (var policy : db.readRoutingPolicies().entrySet()) { - db.writeRoutingPolicies(policy.getKey(), policy.getValue()); - } - } - } - - @Override - protected void maintain() { - Map<DeploymentId, List<LoadBalancer>> loadBalancers = findLoadBalancers(); - removeObsoleteEndpointsFromDns(loadBalancers); - storePolicies(loadBalancers); - removeObsoletePolicies(loadBalancers); - registerEndpointsInDns(); - } - - /** Find all exclusive load balancers in this system, grouped by deployment */ - private Map<DeploymentId, List<LoadBalancer>> findLoadBalancers() { - Map<DeploymentId, List<LoadBalancer>> result = new LinkedHashMap<>(); - for (ZoneId zone : controller().zoneRegistry().zones().controllerUpgraded().ids()) { - List<LoadBalancer> loadBalancers = controller().applications().configServer().getLoadBalancers(zone); - for (LoadBalancer loadBalancer : loadBalancers) { - DeploymentId deployment = new DeploymentId(loadBalancer.application(), zone); - result.compute(deployment, (k, existing) -> { - if (existing == null) { - existing = new ArrayList<>(); - } - existing.add(loadBalancer); - return existing; - }); - } - } - return Collections.unmodifiableMap(result); - } - - /** Create global endpoints for all current routing policies */ - private void registerEndpointsInDns() { - try (Lock lock = db.lockRoutingPolicies()) { - Map<RoutingId, List<RoutingPolicy>> routingTable = routingTableFrom(db.readRoutingPolicies()); - - // Create DNS record for each routing ID - for (Map.Entry<RoutingId, List<RoutingPolicy>> route : routingTable.entrySet()) { - Endpoint endpoint = RoutingPolicy.endpointOf(route.getKey().application(), route.getKey().rotation(), - controller().system()); - Set<AliasTarget> targets = route.getValue() - .stream() - .filter(policy -> policy.dnsZone().isPresent()) - .map(policy -> new AliasTarget(policy.canonicalName(), - policy.dnsZone().get(), - policy.zone())) - .collect(Collectors.toSet()); - try { - nameServiceForwarder.createAlias(RecordName.from(endpoint.dnsName()), targets, Priority.normal); - } catch (Exception e) { - log.log(LogLevel.WARNING, "Failed to create or update DNS record for global rotation " + - endpoint.dnsName() + ". Retrying in " + maintenanceInterval(), e); - } - } - } - } - - /** Store routing policies for all load balancers */ - private void storePolicies(Map<DeploymentId, List<LoadBalancer>> loadBalancers) { - for (Map.Entry<DeploymentId, List<LoadBalancer>> entry : loadBalancers.entrySet()) { - ApplicationId application = entry.getKey().applicationId(); - ZoneId zone = entry.getKey().zoneId(); - try (Lock lock = db.lockRoutingPolicies()) { - Set<RoutingPolicy> policies = new LinkedHashSet<>(db.readRoutingPolicies(application)); - for (LoadBalancer loadBalancer : entry.getValue()) { - try { - RoutingPolicy policy = storePolicy(application, zone, loadBalancer); - if (!policies.add(policy)) { - policies.remove(policy); - policies.add(policy); - } - } catch (Exception e) { - log.log(LogLevel.WARNING, "Failed to create or update DNS record for load balancer " + - loadBalancer.hostname() + ". Retrying in " + maintenanceInterval(), - e); - } - } - db.writeRoutingPolicies(application, policies); - } - } - } - - /** Store policy for given load balancer and request a CNAME for it */ - private RoutingPolicy storePolicy(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer) { - RoutingPolicy routingPolicy = new RoutingPolicy(application, loadBalancer.cluster(), zone, - loadBalancer.hostname(), loadBalancer.dnsZone(), - loadBalancer.rotations()); - RecordName name = RecordName.from(routingPolicy.endpointIn(controller().system()).dnsName()); - RecordData data = RecordData.fqdn(loadBalancer.hostname().value()); - nameServiceForwarder.createCname(name, data, Priority.normal); - return routingPolicy; - } - - /** Remove obsolete policies and their CNAME records */ - private void removeObsoletePolicies(Map<DeploymentId, List<LoadBalancer>> loadBalancers) { - try (Lock lock = db.lockRoutingPolicies()) { - var allPolicies = new HashMap<>(db.readRoutingPolicies()); - var removalCandidates = allPolicies.values().stream() - .flatMap(Collection::stream) - .collect(Collectors.toSet()); - var activeLoadBalancers = loadBalancers.values().stream() - .flatMap(Collection::stream) - .map(LoadBalancer::hostname) - .collect(Collectors.toSet()); - // Keep active load balancers by removing them from candidates - removalCandidates.removeIf(policy -> activeLoadBalancers.contains(policy.canonicalName())); - for (var policy : removalCandidates) { - var dnsName = policy.endpointIn(controller().system()).dnsName(); - nameServiceForwarder.removeRecords(Record.Type.CNAME, RecordName.from(dnsName), Priority.normal); - // Remove stale policy from curator - var updatedPolicies = new LinkedHashSet<>(allPolicies.getOrDefault(policy.owner(), Set.of())); - updatedPolicies.remove(policy); - allPolicies.put(policy.owner(), updatedPolicies); - db.writeRoutingPolicies(policy.owner(), updatedPolicies); - } - } - } - - /** Remove DNS for global endpoints not referenced by given load balancers */ - private void removeObsoleteEndpointsFromDns(Map<DeploymentId, List<LoadBalancer>> loadBalancers) { - try (Lock lock = db.lockRoutingPolicies()) { - Set<RoutingId> removalCandidates = routingTableFrom(db.readRoutingPolicies()).keySet(); - Set<RoutingId> activeRoutingIds = routingIdsFrom(loadBalancers); - removalCandidates.removeAll(activeRoutingIds); - for (RoutingId id : removalCandidates) { - Endpoint endpoint = RoutingPolicy.endpointOf(id.application(), id.rotation(), controller().system()); - nameServiceForwarder.removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()), Priority.normal); - } - } - } - - /** Compute routing IDs from given load balancers */ - private static Set<RoutingId> routingIdsFrom(Map<DeploymentId, List<LoadBalancer>> loadBalancers) { - Set<RoutingId> routingIds = new LinkedHashSet<>(); - for (List<LoadBalancer> values : loadBalancers.values()) { - for (LoadBalancer loadBalancer : values) { - for (RotationName rotation : loadBalancer.rotations()) { - routingIds.add(new RoutingId(loadBalancer.application(), rotation)); - } - } - } - return Collections.unmodifiableSet(routingIds); - } - - /** Compute a routing table from given policies */ - private static Map<RoutingId, List<RoutingPolicy>> routingTableFrom(Map<ApplicationId, Set<RoutingPolicy>> routingPolicies) { - var flattenedPolicies = routingPolicies.values().stream().flatMap(Collection::stream).collect(Collectors.toSet()); - var routingTable = new LinkedHashMap<RoutingId, List<RoutingPolicy>>(); - for (var policy : flattenedPolicies) { - for (var rotation : policy.rotations()) { - var id = new RoutingId(policy.owner(), rotation); - routingTable.compute(id, (k, policies) -> { - if (policies == null) { - policies = new ArrayList<>(); - } - policies.add(policy); - return policies; - }); - } - } - return routingTable; - } - -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 59d3f6dbb84..7dcb4aba138 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -294,7 +294,7 @@ public class ControllerTest { "app1--tenant1.global.vespa.yahooapis.com"), tester.configServer().rotationCnames().get(new DeploymentId(application.id(), deployment.zone()))); } - tester.updateDns(); + tester.flushDnsRequests(); assertEquals(3, tester.controllerTester().nameService().records().size()); Optional<Record> record = tester.controllerTester().findCname("app1--tenant1.global.vespa.yahooapis.com"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index 29bec7246ee..83b95ccc8b0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -5,6 +5,7 @@ import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.RegionName; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import java.io.ByteArrayOutputStream; @@ -62,6 +63,10 @@ public class ApplicationPackageBuilder { return this; } + public ApplicationPackageBuilder region(RegionName regionName) { + return region(regionName.value()); + } + public ApplicationPackageBuilder region(String regionName) { environmentBody.append(" <region active='true'>"); environmentBody.append(regionName); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index b6c7e369f07..a093aac430b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -140,8 +140,8 @@ public class DeploymentTester { readyJobTrigger().maintain(); } - /** Dispatch all pending name services requests */ - public void updateDns() { + /** Flush all pending name services requests */ + public void flushDnsRequests() { nameServiceDispatcher.run(); assertTrue("All name service requests dispatched", controller().curator().readNameServiceQueue().requests().isEmpty()); @@ -225,7 +225,7 @@ public class DeploymentTester { assertFalse(applications().require(application.id()).change().hasTargets()); } if (updateDnsAutomatically) { - updateDns(); + flushDnsRequests(); } } 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 e201258c701..913c9a800c1 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 @@ -204,14 +204,16 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer return loadBalancers.getOrDefault(zone, Collections.emptyList()); } + @Override + public List<LoadBalancer> getLoadBalancers(ApplicationId application, ZoneId zone) { + return getLoadBalancers(zone).stream() + .filter(lb -> lb.application().equals(application)) + .collect(Collectors.toUnmodifiableList()); + } + public void addLoadBalancers(ZoneId zone, List<LoadBalancer> loadBalancers) { - this.loadBalancers.compute(zone, (k, existing) -> { - if (existing == null) { - existing = new ArrayList<>(); - } - existing.addAll(loadBalancers); - return existing; - }); + this.loadBalancers.putIfAbsent(zone, new ArrayList<>()); + this.loadBalancers.get(zone).addAll(loadBalancers); } public void removeLoadBalancers(ApplicationId application, ZoneId zone) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java index 13218cc2442..6a3eef5a142 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java @@ -104,7 +104,7 @@ public class DnsMaintainerTest { for (int i = 0; i < ControllerTester.availableRotations; i++) { maintainer.maintain(); } - tester.updateDns(); + tester.flushDnsRequests(); assertFalse("DNS record removed", findCname.apply("app1--tenant1.global.vespa.yahooapis.com").isPresent()); assertFalse("DNS record removed", findCname.apply("app1--tenant1.global.vespa.oath.cloud").isPresent()); assertFalse("DNS record removed", findCname.apply("app1.tenant1.global.vespa.yahooapis.com").isPresent()); @@ -124,7 +124,7 @@ public class DnsMaintainerTest { // One record is removed per run for (int i = 1; i <= staleTotal*2; i++) { maintainer.run(); - tester.updateDns(); + tester.flushDnsRequests(); assertEquals(Math.max(staleTotal - i, 0), records().size()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index b18c39f4042..58f35c0ac05 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -255,7 +255,7 @@ public class MetricsReporterTest { reporter.maintain(); assertEquals("Deployment queues name services requests", 6, metrics.getMetric(MetricsReporter.NAME_SERVICE_REQUESTS_QUEUED).intValue()); - tester.updateDns(); + tester.flushDnsRequests(); reporter.maintain(); assertEquals("Queue consumed", 0, metrics.getMetric(MetricsReporter.NAME_SERVICE_REQUESTS_QUEUED).intValue()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java index 14d5dc4e7c3..1cfb18da851 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java @@ -15,12 +15,9 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; -import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; -import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import org.junit.Test; -import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -37,31 +34,32 @@ import static org.junit.Assert.assertTrue; * @author mortent * @author mpolden */ -public class RoutingPolicyMaintainerTest { +public class RoutingPoliciesTest { private final DeploymentTester tester = new DeploymentTester(); + private final Application app1 = tester.createApplication("app1", "tenant1", 1, 1L); private final Application app2 = tester.createApplication("app2", "tenant1", 1, 1L); - private final RoutingPolicyMaintainer maintainer = new RoutingPolicyMaintainer(tester.controller(), Duration.ofHours(12), - new JobControl(new MockCuratorDb()), - tester.controllerTester().curator()); + private final ZoneId zone1 = ZoneId.from("prod", "us-west-1"); + private final ZoneId zone2 = ZoneId.from("prod", "us-central-1"); + private final ZoneId zone3 = ZoneId.from("prod", "us-east-3"); + private final ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .environment(Environment.prod) - .region("us-west-1") - .region("us-central-1") + .region(zone1.region()) + .region(zone2.region()) .build(); @Test public void maintains_global_routing_policies() { + int buildNumber = 42; int clustersPerZone = 2; - tester.deployCompletely(app1, applicationPackage); - // Cluster is member of 2 global rotations + // Cluster 0 is member of 2 global rotations Map<Integer, Set<RotationName>> rotations = Map.of(0, Set.of(RotationName.from("r0"), RotationName.from("r1"))); - provisionLoadBalancers(app1, clustersPerZone, rotations); + provisionLoadBalancers(clustersPerZone, rotations, app1.id(), zone1, zone2); // Creates alias records for cluster0 - maintain(); + tester.deployCompletely(app1, applicationPackage, ++buildNumber); Supplier<List<Record>> records1 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r0.app1.tenant1.global.vespa.oath.cloud")); Supplier<List<Record>> records2 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r1.app1.tenant1.global.vespa.oath.cloud")); assertEquals(2, records1.get().size()); @@ -76,16 +74,15 @@ public class RoutingPolicyMaintainerTest { // Applications gains a new deployment ApplicationPackage updatedApplicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .region("us-west-1") - .region("us-central-1") - .region("us-east-3") + .region(zone1.region()) + .region(zone2.region()) + .region(zone3.region()) .build(); int numberOfDeployments = 3; - tester.deployCompletely(app1, updatedApplicationPackage, BuildJob.defaultBuildNumber + 1); + provisionLoadBalancers(clustersPerZone, rotations, app1.id(), zone3); + tester.deployCompletely(app1, updatedApplicationPackage, ++buildNumber); // Cluster in new deployment is added to the rotation - provisionLoadBalancers(app1, 2, rotations); - maintain(); assertEquals(numberOfDeployments, records1.get().size()); assertEquals("lb-0--tenant1:app1:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records1.get().get(0).data().asString()); assertEquals("lb-0--tenant1:app1:default--prod.us-east-3/dns-zone-1/prod.us-east-3", records1.get().get(1).data().asString()); @@ -93,16 +90,15 @@ public class RoutingPolicyMaintainerTest { // Another application is deployed Supplier<List<Record>> records3 = () -> tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from("r0.app2.tenant1.global.vespa.oath.cloud")); + provisionLoadBalancers(1, Map.of(0, Set.of(RotationName.from("r0"))), app2.id(), zone1, zone2); tester.deployCompletely(app2, applicationPackage); - provisionLoadBalancers(app2, 1, Map.of(0, Set.of(RotationName.from("r0")))); - maintain(); assertEquals(2, records3.get().size()); assertEquals("lb-0--tenant1:app2:default--prod.us-central-1/dns-zone-1/prod.us-central-1", records3.get().get(0).data().asString()); assertEquals("lb-0--tenant1:app2:default--prod.us-west-1/dns-zone-1/prod.us-west-1", records3.get().get(1).data().asString()); // All rotations for app1 are removed - provisionLoadBalancers(app1, clustersPerZone, Collections.emptyMap()); - maintain(); + provisionLoadBalancers(clustersPerZone, Map.of(), app1.id(), zone1, zone2, zone3); + tester.deployCompletely(app1, updatedApplicationPackage, ++buildNumber); assertEquals(List.of(), records1.get()); Set<RoutingPolicy> policies = tester.controller().curator().readRoutingPolicies(app1.id()); assertEquals(clustersPerZone * numberOfDeployments, policies.size()); @@ -115,11 +111,11 @@ public class RoutingPolicyMaintainerTest { public void maintains_routing_policies_per_zone() { // Deploy application int clustersPerZone = 2; - tester.deployCompletely(app1, applicationPackage); - provisionLoadBalancers(app1, clustersPerZone); + int buildNumber = 42; + provisionLoadBalancers(clustersPerZone, app1.id(), zone1, zone2); + tester.deployCompletely(app1, applicationPackage, ++buildNumber); - // Creates records and policies for all clusters in all zones - maintain(); + // Deployment creates records and policies for all clusters in all zones Set<String> expectedRecords = Set.of( "c0.app1.tenant1.us-west-1.vespa.oath.cloud", "c1.app1.tenant1.us-west-1.vespa.oath.cloud", @@ -129,14 +125,14 @@ public class RoutingPolicyMaintainerTest { assertEquals(expectedRecords, recordNames()); assertEquals(4, policies(app1).size()); - // Next run does nothing - maintain(); + // Next deploy does nothing + tester.deployCompletely(app1, applicationPackage, ++buildNumber); assertEquals(expectedRecords, recordNames()); assertEquals(4, policies(app1).size()); - // Add 1 cluster in each zone - provisionLoadBalancers(app1, clustersPerZone + 1); - maintain(); + // Add 1 cluster in each zone and deploy + provisionLoadBalancers(clustersPerZone + 1, app1.id(), zone1, zone2); + tester.deployCompletely(app1, applicationPackage, ++buildNumber); expectedRecords = Set.of( "c0.app1.tenant1.us-west-1.vespa.oath.cloud", "c1.app1.tenant1.us-west-1.vespa.oath.cloud", @@ -148,10 +144,9 @@ public class RoutingPolicyMaintainerTest { assertEquals(expectedRecords, recordNames()); assertEquals(6, policies(app1).size()); - // Add another application - tester.deployCompletely(app2, applicationPackage); - provisionLoadBalancers(app2, clustersPerZone); - maintain(); + // Deploy another application + provisionLoadBalancers(clustersPerZone, app2.id(), zone1, zone2); + tester.deployCompletely(app2, applicationPackage, ++buildNumber); expectedRecords = Set.of( "c0.app1.tenant1.us-west-1.vespa.oath.cloud", "c1.app1.tenant1.us-west-1.vespa.oath.cloud", @@ -167,9 +162,9 @@ public class RoutingPolicyMaintainerTest { assertEquals(expectedRecords, recordNames()); assertEquals(4, policies(app2).size()); - // Remove cluster from app1 - provisionLoadBalancers(app1, clustersPerZone); - maintain(); + // Deploy removes cluster from app1 + provisionLoadBalancers(clustersPerZone, app1.id(), zone1, zone2); + tester.deployCompletely(app1, applicationPackage, ++buildNumber); expectedRecords = Set.of( "c0.app1.tenant1.us-west-1.vespa.oath.cloud", "c1.app1.tenant1.us-west-1.vespa.oath.cloud", @@ -185,10 +180,10 @@ public class RoutingPolicyMaintainerTest { // Remove app2 completely tester.controller().applications().require(app2.id()).deployments().keySet() .forEach(zone -> { - tester.controller().applications().deactivate(app2.id(), zone); tester.configServer().removeLoadBalancers(app2.id(), zone); + tester.controller().applications().deactivate(app2.id(), zone); }); - maintain(); + tester.flushDnsRequests(); expectedRecords = Set.of( "c0.app1.tenant1.us-west-1.vespa.oath.cloud", "c1.app1.tenant1.us-west-1.vespa.oath.cloud", @@ -200,11 +195,6 @@ public class RoutingPolicyMaintainerTest { assertEquals("Keeps routing policies for " + app1, 4, tester.controller().applications().routingPolicies(app1.id()).size()); } - private void maintain() { - maintainer.run(); - tester.updateDns(); - } - private Set<RoutingPolicy> policies(Application application) { return tester.controller().curator().readRoutingPolicies(application.id()); } @@ -216,22 +206,19 @@ public class RoutingPolicyMaintainerTest { .collect(Collectors.toSet()); } - private void provisionLoadBalancers(Application application, int clustersPerZone, Map<Integer, Set<RotationName>> clusterRotations) { - tester.controller().applications().require(application.id()) - .deployments().keySet() - .forEach(zone -> tester.configServer().removeLoadBalancers(application.id(), zone)); - tester.controller().applications().require(application.id()) - .deployments().keySet() - .forEach(zone -> tester.configServer() - .addLoadBalancers(zone, createLoadBalancers(zone, application.id(), clustersPerZone, clusterRotations))); + private void provisionLoadBalancers(int clustersPerZone, Map<Integer, Set<RotationName>> clusterRotations, ApplicationId application, ZoneId... zones) { + for (ZoneId zone : zones) { + tester.configServer().removeLoadBalancers(application, zone); + tester.configServer().addLoadBalancers(zone, createLoadBalancers(zone, application, clustersPerZone, clusterRotations)); + } } - private void provisionLoadBalancers(Application application, int clustersPerZone) { - provisionLoadBalancers(application, clustersPerZone, Collections.emptyMap()); + private void provisionLoadBalancers(int clustersPerZone, ApplicationId application, ZoneId... zones) { + provisionLoadBalancers(clustersPerZone, Map.of(), application, zones); } private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count, - Map<Integer, Set<RotationName>> clusterRotations) { + Map<Integer, Set<RotationName>> clusterRotations) { List<LoadBalancer> loadBalancers = new ArrayList<>(); for (int i = 0; i < count; i++) { Set<RotationName> rotations = clusterRotations.getOrDefault(i, Collections.emptySet()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json index 6b20adf835e..6218d9d04f0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json @@ -52,9 +52,6 @@ "name": "ResourceMeterMaintainer" }, { - "name": "RoutingPolicyMaintainer" - }, - { "name": "SystemUpgrader" }, { |