diff options
6 files changed, 135 insertions, 66 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java index 70aaf5de112..d8959abf37b 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java @@ -13,6 +13,8 @@ import java.util.Set; import java.util.stream.Collectors; /** + * Represents the resources allocated to a deployment at specific point in time. + * * @author olaa */ public class ResourceSnapshot { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java index b8692233b2d..4f36e713ba4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java @@ -69,7 +69,7 @@ public class NameServiceForwarder { forward(new RemoveRecords(type, data), priority); } - private void forward(NameServiceRequest request, NameServiceQueue.Priority priority) { + protected void forward(NameServiceRequest request, NameServiceQueue.Priority priority) { try (Lock lock = db.lockNameServiceQueue()) { NameServiceQueue queue = db.readNameServiceQueue(); var queued = queue.requests().size(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java index d57d6ff0976..76a186a2f6b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository; import com.yahoo.vespa.hosted.controller.api.integration.resource.MeteringClient; import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshot; +import com.yahoo.yolean.Exceptions; import java.time.Clock; import java.time.Duration; @@ -18,10 +19,11 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.logging.Level; import java.util.stream.Collectors; /** - * Creates a ResourceSnapshot per application, which is then passed on to a MeteringClient + * Creates a {@link ResourceSnapshot} per application, which is then passed on to a MeteringClient * * @author olaa */ @@ -49,7 +51,15 @@ public class ResourceMeterMaintainer extends ControllerMaintainer { @Override protected void maintain() { + try { + collectResourceSnapshots(); + } catch (Exception e) { + log.log(Level.WARNING, "Failed to collect resource snapshots. Retrying in " + interval() + ". Error: " + + Exceptions.toMessageString(e)); + } + } + private void collectResourceSnapshots() { Collection<ResourceSnapshot> resourceSnapshots = getAllResourceSnapshots(); meteringClient.consume(resourceSnapshots); @@ -104,4 +114,5 @@ public class ResourceMeterMaintainer extends ControllerMaintainer { private boolean isNodeStateMeterable(Node node) { return METERABLE_NODE_STATES.contains(node.state()); } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index e5a99c2e69d..d6f0ff4d85f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -24,6 +24,7 @@ import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; +import com.yahoo.vespa.hosted.controller.dns.NameServiceRequest; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.util.ArrayList; @@ -166,7 +167,7 @@ public class RoutingPolicies { // TODO(mpolden): Remove and inline call to updateGlobalDnsOf when feature flag disappears private void updateGlobalDnsOf(ApplicationId application, Collection<RoutingPolicy> routingPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Lock lock) { - if (weightedDnsPerRegion.with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm()).value()) { + if (useWeightedDnsPerRegion(application)) { updateGlobalDnsOf(routingPolicies, inactiveZones, lock); } else { legacyUpdateGlobalDnsOf(routingPolicies, inactiveZones, lock); @@ -245,7 +246,7 @@ public class RoutingPolicies { var policyId = new RoutingPolicyId(loadBalancer.application(), loadBalancer.cluster(), allocation.deployment.zoneId()); var existingPolicy = policies.get(policyId); var newPolicy = new RoutingPolicy(policyId, loadBalancer.hostname(), loadBalancer.dnsZone(), - allocation.endpointIdsOf(loadBalancer), + allocation.endpointIdsOf(loadBalancer, useWeightedDnsPerRegion(loadBalancer.application())), new Status(isActive(loadBalancer), GlobalRouting.DEFAULT_STATUS)); // Preserve global routing status for existing policy if (existingPolicy != null) { @@ -262,15 +263,15 @@ public class RoutingPolicies { var name = RecordName.from(policy.endpointIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry()) .dnsName()); var data = RecordData.fqdn(policy.canonicalName().value()); - NameUpdater nameUpdater = nameUpdaterIn(policy.id().zone()); + NameServiceForwarder forwarder = nameServiceForwarderIn(policy.id().zone()); if (policy.id().owner().equals(SystemApplication.configServer.id())) { // TODO(mpolden): Remove this after transition is complete. Before automatic provisioning of config server // load balancers, the DNS records for the config server LB were of type A. It's not possible // to change the type of an existing record, we therefore remove the A record before creating // a CNAME. - nameUpdater.removeRecords(Record.Type.A, name); + forwarder.removeRecords(Record.Type.A, name, Priority.normal); } - nameUpdater.createCname(name, data); + forwarder.createCname(name, data, Priority.normal); } /** Remove policies and zone DNS records unreferenced by given load balancers */ @@ -284,7 +285,9 @@ public class RoutingPolicies { !policy.id().zone().equals(allocation.deployment.zoneId())) continue; var dnsName = policy.endpointIn(controller.system(), RoutingMethod.exclusive, controller.zoneRegistry()).dnsName(); - nameUpdaterIn(allocation.deployment.zoneId()).removeRecords(Record.Type.CNAME, RecordName.from(dnsName)); + nameServiceForwarderIn(allocation.deployment.zoneId()).removeRecords(Record.Type.CNAME, + RecordName.from(dnsName), + Priority.normal); newPolicies.remove(policy.id()); } db.writeRoutingPolicies(allocation.deployment.applicationId(), newPolicies); @@ -300,16 +303,17 @@ public class RoutingPolicies { var endpoints = controller.routing().endpointsOf(id.application()) .not().requiresRotation() .named(id.endpointId()); - var nameUpdater = nameUpdaterIn(allocation.deployment.zoneId()); - endpoints.forEach(endpoint -> nameUpdater.removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()))); + var forwarder = nameServiceForwarderIn(allocation.deployment.zoneId()); + endpoints.forEach(endpoint -> forwarder.removeRecords(Record.Type.ALIAS, RecordName.from(endpoint.dnsName()), + Priority.normal)); } } /** Compute routing IDs from given load balancers */ - private static Set<RoutingId> routingIdsFrom(LoadBalancerAllocation allocation) { + private Set<RoutingId> routingIdsFrom(LoadBalancerAllocation allocation) { Set<RoutingId> routingIds = new LinkedHashSet<>(); for (var loadBalancer : allocation.loadBalancers) { - for (var endpointId : allocation.endpointIdsOf(loadBalancer)) { + for (var endpointId : allocation.endpointIdsOf(loadBalancer, useWeightedDnsPerRegion(loadBalancer.application()))) { routingIds.add(new RoutingId(loadBalancer.application(), endpointId)); } } @@ -349,6 +353,10 @@ public class RoutingPolicies { return false; } + private boolean useWeightedDnsPerRegion(ApplicationId application) { + return weightedDnsPerRegion.with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm()).value(); + } + /** Represents records for a region-wide endpoint */ private static class RegionEndpoint { @@ -410,7 +418,7 @@ public class RoutingPolicies { } /** Compute all endpoint IDs for given load balancer */ - private Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer) { + private Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer, boolean useWeightedDnsPerRegion) { if (!deployment.zoneId().environment().isProduction()) { // Only production deployments have configurable endpoints return Set.of(); } @@ -418,12 +426,16 @@ public class RoutingPolicies { if (instanceSpec.isEmpty()) { return Set.of(); } + if (useWeightedDnsPerRegion && instanceSpec.get().globalServiceId().filter(id -> id.equals(loadBalancer.cluster().value())).isPresent()) { + // Legacy assignment always has the default endpoint Id + return Set.of(EndpointId.defaultId()); + } return instanceSpec.get().endpoints().stream() .filter(endpoint -> endpoint.containerId().equals(loadBalancer.cluster().value())) .filter(endpoint -> endpoint.regions().contains(deployment.zoneId().region())) .map(com.yahoo.config.application.api.Endpoint::endpointId) .map(EndpointId::of) - .collect(Collectors.toSet()); + .collect(Collectors.toUnmodifiableSet()); } } @@ -440,52 +452,24 @@ public class RoutingPolicies { } /** Returns the name updater to use for given zone */ - private NameUpdater nameUpdaterIn(ZoneId zone) { + private NameServiceForwarder nameServiceForwarderIn(ZoneId zone) { if (controller.zoneRegistry().routingMethods(zone).contains(RoutingMethod.exclusive)) { - return new NameUpdater(controller.nameServiceForwarder()); + return controller.nameServiceForwarder(); } - return new DiscardingNameUpdater(); + return new NameServiceDiscarder(controller.curator()); } - /** A name updater that passes name service operations to the next handler */ - private static class NameUpdater { - - private final NameServiceForwarder forwarder; - - public NameUpdater(NameServiceForwarder forwarder) { - this.forwarder = forwarder; - } + /** A {@link NameServiceForwarder} that does nothing. Used in zones where no explicit DNS updates are needed */ + private static class NameServiceDiscarder extends NameServiceForwarder { - public void removeRecords(Record.Type type, RecordName name) { - forwarder.removeRecords(type, name, Priority.normal); + public NameServiceDiscarder(CuratorDb db) { + super(db); } - public void createAlias(RecordName name, Set<AliasTarget> targets) { - forwarder.createAlias(name, targets, Priority.normal); - } - - public void createCname(RecordName name, RecordData data) { - forwarder.createCname(name, data, Priority.normal); - } - - } - - /** A name updater that does nothing */ - private static class DiscardingNameUpdater extends NameUpdater { - - private DiscardingNameUpdater() { - super(null); - } - - @Override - public void removeRecords(Record.Type type, RecordName name) {} - - @Override - public void createAlias(RecordName name, Set<AliasTarget> targets) {} - @Override - public void createCname(RecordName name, RecordData data) {} - + protected void forward(NameServiceRequest request, Priority priority) { + // Ignored + } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index 3ee4c6961a4..7064c6e1362 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -176,8 +176,8 @@ public class RoutingPoliciesTest { // Weight of inactive zone is set to zero tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, ImmutableMap.of(zone1, 1L, - zone3, 1L, - zone4, 0L)); + zone3, 1L, + zone4, 0L)); // Other zone in shared region is set out. Entire record group for the region is removed as all zones in the // region are out (weight sum = 0) @@ -198,6 +198,27 @@ public class RoutingPoliciesTest { } @Test + public void global_routing_policies_legacy_global_service_id() { + var tester = new RoutingPoliciesTester(); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); + int clustersPerZone = 2; + int numberOfDeployments = 2; + var applicationPackage = applicationPackageBuilder() + .region(zone1.region()) + .region(zone2.region()) + .globalServiceId("c0") + .build(); + tester.provisionLoadBalancers(clustersPerZone, context.instanceId(), zone1, zone2); + + // Creates alias records + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + tester.assertTargets(context.instanceId(), EndpointId.defaultId(), 0, zone1, zone2); + assertEquals("Routing policy count is equal to cluster count", + numberOfDeployments * clustersPerZone, + tester.policiesOf(context.instance().id()).size()); + } + + @Test public void zone_routing_policies() { zone_routing_policies(false); zone_routing_policies(true); @@ -294,8 +315,19 @@ public class RoutingPoliciesTest { } @Test + public void zone_routing_policies_without_dns_update() { + var tester = new RoutingPoliciesTester(new DeploymentTester(), false); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); + tester.provisionLoadBalancers(1, context.instanceId(), true, zone1, zone2); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); + assertEquals(0, tester.controllerTester().controller().curator().readNameServiceQueue().requests().size()); + assertEquals(Set.of(), tester.recordNames()); + assertEquals(2, tester.policiesOf(context.instanceId()).size()); + } + + @Test public void global_routing_policies_in_rotationless_system() { - var tester = new RoutingPoliciesTester(new DeploymentTester(new ControllerTester(new RotationsConfig.Builder().build()))); + var tester = new RoutingPoliciesTester(new DeploymentTester(new ControllerTester(new RotationsConfig.Builder().build())), true); var context = tester.newDeploymentContext("tenant1", "app1", "default"); tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); @@ -692,7 +724,7 @@ public class RoutingPoliciesTest { } public RoutingPoliciesTester(SystemName system) { - this(new DeploymentTester(new ControllerTester(new ServiceRegistryMock(system)))); + this(new DeploymentTester(new ControllerTester(new ServiceRegistryMock(system))), true); } public RoutingPolicies routingPolicies() { @@ -707,14 +739,15 @@ public class RoutingPoliciesTest { return tester.controllerTester(); } - public RoutingPoliciesTester(DeploymentTester tester) { + public RoutingPoliciesTester(DeploymentTester tester, boolean exclusiveRouting) { this.tester = tester; List<ZoneApi> zones = new ArrayList<>(tester.controllerTester().zoneRegistry().zones().all().zones()); zones.add(ZoneApiMock.from(zone3)); zones.add(ZoneApiMock.from(zone4)); - tester.controllerTester().zoneRegistry() - .setZones(zones) - .exclusiveRoutingIn(zones); + tester.controllerTester().zoneRegistry().setZones(zones); + if (exclusiveRouting) { + tester.controllerTester().zoneRegistry().exclusiveRoutingIn(zones); + } tester.controllerTester().configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(), SystemApplication.all()); ((InMemoryFlagSource) tester.controllerTester().controller().flagSource()).withBooleanFlag(Flags.WEIGHTED_DNS_PER_REGION.id(), true); diff --git a/functions.cmake b/functions.cmake index 984ec165718..864fc010faa 100644 --- a/functions.cmake +++ b/functions.cmake @@ -150,6 +150,10 @@ function(vespa_generate_config TARGET RELATIVE_CONFIG_DEF_PATH) MAIN_DEPENDENCY ${CONFIG_DEF_PATH} ) + if (TARGET ${TARGET}_object) + # Generated config is in implicit object library + set(TARGET "${TARGET}_object") + endif() # Add generated to sources for target target_sources(${TARGET} PRIVATE ${CONFIG_H_PATH} ${CONFIG_CPP_PATH}) @@ -164,6 +168,19 @@ function(vespa_generate_config TARGET RELATIVE_CONFIG_DEF_PATH) vespa_add_source_target("configgen_${TARGET}_${CONFIG_NAME}" DEPENDS ${CONFIG_H_PATH} ${CONFIG_CPP_PATH}) endfunction() +macro(__split_sources_list) + unset(SOURCE_FILES) + unset(NON_TARGET_SOURCE_FILES) + unset(TARGET_SOURCE_FILES) + if(ARG_SOURCES) + set(SOURCE_FILES ${ARG_SOURCES}) + set(TARGET_SOURCE_FILES ${ARG_SOURCES}) + set(NON_TARGET_SOURCE_FILES ${ARG_SOURCES}) + list(FILTER TARGET_SOURCE_FILES INCLUDE REGEX "TARGET_OBJECTS:") + list(FILTER NON_TARGET_SOURCE_FILES EXCLUDE REGEX "TARGET_OBJECTS:") + endif() +endmacro() + function(vespa_add_library TARGET) cmake_parse_arguments(ARG "STATIC;OBJECT;INTERFACE;TEST" @@ -172,13 +189,12 @@ function(vespa_add_library TARGET) ${ARGN}) __check_target_parameters() - - if(ARG_SOURCES) - set(SOURCE_FILES ${ARG_SOURCES}) - else() + __split_sources_list() + if(NOT ARG_SOURCES) # In the case where no source files are given, we include an empty source file to suppress a warning from CMake # This way, config-only libraries will not generate lots of build warnings set(SOURCE_FILES "${CMAKE_SOURCE_DIR}/empty.cpp") + set(NON_TARGET_SOURCE_FILES ${SOURCE_FILES}) endif() if(ARG_OBJECT) @@ -191,7 +207,13 @@ function(vespa_add_library TARGET) set(SOURCE_FILES) endif() - add_library(${TARGET} ${LINKAGE} ${LIBRARY_TYPE} ${SOURCE_FILES}) + if (ARG_OBJECT OR ARG_INTERFACE OR TARGET ${TARGET}_object OR NOT NON_TARGET_SOURCE_FILES) + unset(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY) + add_library(${TARGET} ${LINKAGE} ${LIBRARY_TYPE} ${SOURCE_FILES}) + else() + set(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY True) + add_library(${TARGET} ${LINKAGE} ${LIBRARY_TYPE} $<TARGET_OBJECTS:${TARGET}_object> ${TARGET_SOURCE_FILES}) + endif() __add_dependencies_to_target() __handle_test_targets() @@ -207,6 +229,11 @@ function(vespa_add_library TARGET) __add_target_to_module(${TARGET}) __export_include_directories(${TARGET}) + if(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY) + unset(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY) + vespa_add_library(${TARGET}_object OBJECT SOURCES ${NON_TARGET_SOURCE_FILES} DEPENDS ${ARG_DEPENDS}) + add_dependencies(${TARGET} ${TARGET}_object) + endif() endfunction() function(__install_header_files) @@ -237,7 +264,14 @@ function(vespa_add_executable TARGET) ${ARGN}) __check_target_parameters() - add_executable(${TARGET} ${ARG_SOURCES}) + __split_sources_list() + if(TARGET ${TARGET}_object OR NOT NON_TARGET_SOURCE_FILES) + unset(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY) + add_executable(${TARGET} ${ARG_SOURCES}) + else() + set(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY True) + add_executable(${TARGET} $<TARGET_OBJECTS:${TARGET}_object> ${TARGET_SOURCE_FILES}) + endif() __add_dependencies_to_target() __handle_test_targets() @@ -252,6 +286,11 @@ function(vespa_add_executable TARGET) __add_target_to_module(${TARGET}) __export_include_directories(${TARGET}) + if(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY) + unset(VESPA_ADD_IMPLICIT_OBJECT_LIBRARY) + vespa_add_library(${TARGET}_object OBJECT SOURCES ${NON_TARGET_SOURCE_FILES} DEPENDS ${ARG_DEPENDS}) + add_dependencies(${TARGET} ${TARGET}_object) + endif() endfunction() macro(vespa_define_module) |