diff options
5 files changed, 65 insertions, 26 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java index e55bbaf6acc..80a62d94f2e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java @@ -14,7 +14,8 @@ import java.util.Optional; import java.util.Set; /** - * Represents the DNS routing policy for a load balancer. + * Represents the DNS routing policy for a load balancer. A routing policy is uniquely identified by its owner, cluster + * and zone. * * @author mortent * @author mpolden @@ -90,13 +91,15 @@ public class RoutingPolicy { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - RoutingPolicy policy = (RoutingPolicy) o; - return canonicalName.equals(policy.canonicalName); + RoutingPolicy that = (RoutingPolicy) o; + return owner.equals(that.owner) && + cluster.equals(that.cluster) && + zone.equals(that.zone); } @Override public int hashCode() { - return Objects.hash(canonicalName); + return Objects.hash(owner, cluster, zone); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java index 9c70f734baf..ff88805f957 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CostReportMaintainer.java @@ -26,7 +26,7 @@ public class CostReportMaintainer extends Maintainer { public CostReportMaintainer(Controller controller, Duration interval, JobControl jobControl, CostReportConsumer costReportConsumer) { - super(controller, interval, jobControl, "CostReportMaintainer", EnumSet.of(SystemName.main)); + super(controller, interval, jobControl, null, EnumSet.of(SystemName.main)); this.consumer = costReportConsumer; this.nodeRepository = controller.serviceRegistry().configServer().nodeRepository(); this.clock = controller.clock(); 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 a82479fd0d8..603757e099d 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 @@ -82,8 +82,8 @@ public class RoutingPolicies { deploymentSpec); try (var lock = db.lockRoutingPolicies()) { removeObsoleteEndpointsFromDns(lbs, lock); - var writtenPolicies = storePoliciesOf(lbs, lock); - removeObsoletePolicies(lbs, writtenPolicies, lock); + storePoliciesOf(lbs, lock); + removeObsoletePolicies(lbs, lock); registerEndpointsInDns(lbs, lock); } } @@ -108,7 +108,7 @@ public class RoutingPolicies { } /** Store routing policies for given route. Returns the persisted policies. */ - private Set<RoutingPolicy> storePoliciesOf(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { + private void storePoliciesOf(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { var policies = new LinkedHashSet<>(get(loadBalancers.application)); for (LoadBalancer loadBalancer : loadBalancers.list) { var endpointIds = loadBalancers.endpointIdsOf(loadBalancer); @@ -120,7 +120,6 @@ public class RoutingPolicies { } } db.writeRoutingPolicies(loadBalancers.application, policies); - return policies; } /** Create a policy for given load balancer and register a CNAME for it */ @@ -135,13 +134,8 @@ public class RoutingPolicies { } /** Remove obsolete policies for given route and their CNAME records */ - private void removeObsoletePolicies(AllocatedLoadBalancers loadBalancers, Set<RoutingPolicy> writtenPolicies, - @SuppressWarnings("unused") Lock lock) { + private void removeObsoletePolicies(AllocatedLoadBalancers loadBalancers, @SuppressWarnings("unused") Lock lock) { var allPolicies = new LinkedHashSet<>(get(loadBalancers.application)); - if (!writtenPolicies.equals(allPolicies)) { - LOGGER.log(LogLevel.ERROR, String.format("Stale read! This should not happen. Wrote policies %s, but read %s", - writtenPolicies, allPolicies)); - } var removalCandidates = new HashSet<>(allPolicies); var activeLoadBalancers = loadBalancers.list.stream() .map(LoadBalancer::hostname) @@ -153,17 +147,11 @@ public class RoutingPolicies { LOGGER.log(LogLevel.WARNING, "Removing " + removalCandidates + ". Active load balancers " + activeLoadBalancers); } - removalCandidates.forEach(allPolicies::remove); - - // Find CNAMEs in use - var cnamesUsed = allPolicies.stream() - .map(policy -> policy.endpointIn(controller.system()).dnsName()) - .collect(Collectors.toSet()); for (var policy : removalCandidates) { var dnsName = policy.endpointIn(controller.system()).dnsName(); - if ( ! cnamesUsed.contains(dnsName)) - controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(dnsName), Priority.normal); + controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(dnsName), Priority.normal); + allPolicies.remove(policy); } db.writeRoutingPolicies(loadBalancers.application, allPolicies); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java index 11d6c19b570..04c65480dd6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java @@ -10,10 +10,10 @@ import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceAlloca import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; -import java.text.SimpleDateFormat; import java.time.Clock; +import java.time.ZoneId; +import java.time.format.DateTimeFormatter; import java.util.Comparator; -import java.util.Date; import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; @@ -34,7 +34,7 @@ public class CostCalculator { Map<Property, ResourceAllocation> fixedAllocations, CloudName cloudName) { - var date = new SimpleDateFormat("yyyy-MM-dd").format(Date.from(clock.instant())); + var date = DateTimeFormatter.ofPattern("yyyy-MM-dd").withZone(ZoneId.of("UTC")).format(clock.instant()); // Group properties by tenant name Map<TenantName, Property> propertyByTenantName = controller.tenants().asList().stream() 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 f78873bae09..1bb20296bd2 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 @@ -307,6 +307,47 @@ public class RoutingPoliciesTest { assertEquals(1, tester.policiesOf(devInstance).size()); assertEquals(Set.of("c0.user.app1.tenant1.us-east-1.dev.vespa.oath.cloud"), tester.recordNames()); } + + @Test + public void reprovisioning_load_balancer_preserves_cname_record() { + var tester = new RoutingPoliciesTester(); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); + + // Initial load balancer is provisioned + tester.provisionLoadBalancers(1, context.instanceId(), zone1); + var applicationPackage = new ApplicationPackageBuilder() + .region(zone1.region()) + .build(); + + // Application is deployed + context.submit(applicationPackage).deploy(); + var expectedRecords = Set.of( + "c0.app1.tenant1.us-west-1.vespa.oath.cloud" + ); + assertEquals(expectedRecords, tester.recordNames()); + assertEquals(1, tester.policiesOf(context.instanceId()).size()); + + // Application is removed and the load balancer is deprovisioned + tester.controllerTester().controller().applications().deactivate(context.instanceId(), zone1); + tester.controllerTester().configServer().removeLoadBalancers(context.instanceId(), zone1); + + // Load balancer for the same application is provisioned again, but with a different hostname + var newHostname = HostName.from("new-hostname"); + var loadBalancer = new LoadBalancer("LB-0-Z-" + zone1.value(), + context.instanceId(), + ClusterSpec.Id.from("c0"), + newHostname, + LoadBalancer.State.active, + Optional.of("dns-zone-1")); + tester.controllerTester().configServer().addLoadBalancers(zone1, List.of(loadBalancer)); + + // Application redeployment preserves DNS record + context.submit(applicationPackage).deploy(); + assertEquals(expectedRecords, tester.recordNames()); + assertEquals(1, tester.policiesOf(context.instanceId()).size()); + assertEquals("CNAME points to current load blancer", newHostname.value() + ".", + tester.cnameDataOf(expectedRecords.iterator().next()).get(0)); + } private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, int count) { List<LoadBalancer> loadBalancers = new ArrayList<>(); @@ -368,6 +409,13 @@ public class RoutingPoliciesTest { .collect(Collectors.toList()); } + private List<String> cnameDataOf(String name) { + return tester.controllerTester().nameService().findRecords(Record.Type.CNAME, RecordName.from(name)).stream() + .map(Record::data) + .map(RecordData::asString) + .collect(Collectors.toList()); + } + } } |