Validate CNAMEs and IP lookup after deployment

8 files changed, 77 insertions, 35 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java
index b76af2b2d51..f09340ce470 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterCloud.java
@@ -1,11 +1,14 @@
 // 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.deployment;
 
+import com.yahoo.config.provision.HostName;
+import com.yahoo.config.provision.zone.RoutingMethod;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
 
 import java.net.URI;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * Allows running some predefined tests -- typically remotely.
@@ -29,14 +32,11 @@ public interface TesterCloud {
     /** Returns whether the test container is ready to serve */
     boolean testerReady(DeploymentId deploymentId);
 
-    /** Returns whether the given URL is registered in DNS. */
-    boolean exists(URI endpointUrl);
+    /** Returns the IP address of the given host name, if any. */
+    Optional<String> resolveHostName(HostName hostname);
 
-    /**
-     * Returns whether the given URL is registered in DNS. Always returns true,
-     * as endpoints are not use in this case
-     */
-    default boolean exists(DeploymentId deploymentId) { return true; }
+    /** Returns the host name of the given CNAME, if any. */
+    Optional<HostName> resolveCName(HostName hostName);
 
     enum Status {
 
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java
index 5f315ec1456..f5594e60202 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockTesterCloud.java
@@ -1,13 +1,19 @@
 // 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.stubs;
 
+import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
+import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Status.NOT_STARTED;
@@ -15,10 +21,14 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.Teste
 
 public class MockTesterCloud implements TesterCloud {
 
+    private final NameService cNames;
+
     private List<LogEntry> log = new ArrayList<>();
     private Status status = NOT_STARTED;
     private byte[] config;
 
+    public MockTesterCloud(NameService cNames) {
+        this.cNames = cNames;
     }
 
     @Override
@@ -46,13 +56,15 @@ public class MockTesterCloud implements TesterCloud {
     }
 
     @Override
-    public boolean exists(URI endpointUrl) {
-        return true;
+    public Optional<String> resolveHostName(HostName hostname) {
+        return Optional.of("1.2.3.4");
     }
 
     @Override
-    public boolean exists(DeploymentId deploymentId) {
-        return true;
+    public Optional<HostName> resolveCName(HostName hostName) {
+        return cNames.findRecords(Record.Type.CNAME, RecordName.from(hostName.value())).stream()
+                     .findFirst()
+                     .map(record -> HostName.from(record.data().asString().substring(0, record.data().asString().length() - 1)));
     }
 
     public void add(LogEntry entry) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
index 4637de9a32a..e84cbe3fc07 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
@@ -37,6 +37,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeMap;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 /**
@@ -121,13 +122,11 @@ public class RoutingController {
     }
 
     /** Returns all non-global endpoints and corresponding cluster IDs for given deployments, grouped by their zone */
-    public Map<ZoneId, Map<URI, ClusterSpec.Id>> zoneEndpointsOf(Collection<DeploymentId> deployments) {
-        var endpoints = new TreeMap<ZoneId, Map<URI, ClusterSpec.Id>>(Comparator.comparing(ZoneId::value));
+    public Map<ZoneId, List<Endpoint>> zoneEndpointsOf(Collection<DeploymentId> deployments) {
+        var endpoints = new TreeMap<ZoneId, List<Endpoint>>(Comparator.comparing(ZoneId::value));
         for (var deployment : deployments) {
-            var zoneEndpoints = endpointsOf(deployment).scope(Endpoint.Scope.zone).asList().stream()
-                                                       .collect(Collectors.toUnmodifiableMap(Endpoint::url,
-                                                                                             endpoint -> ClusterSpec.Id.from(endpoint.name())));
-            if (!zoneEndpoints.isEmpty()) {
+            var zoneEndpoints = endpointsOf(deployment).scope(Endpoint.Scope.zone).asList();
+            if  ( ! zoneEndpoints.isEmpty()) {
                 endpoints.put(deployment.zoneId(), zoneEndpoints);
             }
         }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index 6bfbaf24775..729ceaeff34 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -53,7 +53,6 @@ import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.io.UncheckedIOException;
 import java.math.BigInteger;
-import java.net.URI;
 import java.security.KeyPair;
 import java.security.cert.CertificateExpiredException;
 import java.security.cert.CertificateNotYetValidException;
@@ -472,11 +471,41 @@ public class InternalStepRunner implements StepRunner {
             logger.log("Endpoints not yet ready.");
             return false;
         }
-        for (var endpoint : endpoints.get(zone).keySet())
-            if ( ! controller.jobController().cloud().exists(endpoint)) {
-                logger.log(INFO, "DNS lookup yielded no IP address for '" + endpoint + "'.");
+        var policies = controller.routing().policies().get(new DeploymentId(id, zone));
+        for (var endpoint : endpoints.get(zone)) {
+            HostName endpointName = HostName.from(endpoint.dnsName());
+            var ipAddress = controller.jobController().cloud().resolveHostName(endpointName);
+            if (ipAddress.isEmpty()) {
+                logger.log(INFO, "DNS lookup yielded no IP address for '" + endpointName + "'.");
                 return false;
             }
+            if (endpoint.routingMethod() == RoutingMethod.exclusive)  {
+                var policy = policies.get(new RoutingPolicyId(id, ClusterSpec.Id.from(endpoint.name()), zone));
+                if (policy == null)
+                    throw new IllegalStateException(endpoint + " has no matching policy in " + policies);
+
+                var cNameValue = controller.jobController().cloud().resolveCName(endpointName);
+                if (cNameValue.isEmpty()) {
+                    logger.log(INFO, "CNAME '" + endpointName + "' does not yet point to anything");
+                    return false;
+                }
+                if ( ! cNameValue.get().equals(policy.canonicalName())) {
+                    logger.log(INFO, "CNAME '" + endpointName + "' doesn't point to expected host name '" + policy.canonicalName() + "'");
+                    return false;
+                }
+                var loadBalancerAddress = controller.jobController().cloud().resolveHostName(policy.canonicalName());
+                if (loadBalancerAddress.isEmpty()) {
+                    logger.log(INFO, "DNS lookup yielded no IP address for load balancer '" + policy.canonicalName() + "'");
+                    return false;
+                }
+                // Verify that the JVMs internal DNS cache has seen the update. Both names should resolve to the same IP address.
+                if ( ! loadBalancerAddress.equals(ipAddress)) {
+                    logger.log(INFO, "IP address of CNAME '" + endpointName + "' (" + ipAddress.get() + ") and load balancer '" +
+                                     policy.canonicalName() + "' (" + loadBalancerAddress.get() + ") are not equal");
+                    return false;
+                }
+            }
+        }
 
         logEndpoints(endpoints, logger);
         return true;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index cee492276af..775eb2a4d75 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -164,7 +164,9 @@ public final class ControllerTester {
 
     public AthenzDbMock athenzDb() { return athenzDb; }
 
-    public MemoryNameService nameService() { return serviceRegistry.nameServiceMock(); }
+    public MemoryNameService nameService() {
+        return serviceRegistry.nameService();
+    }
 
     public ZoneRegistryMock zoneRegistry() { return serviceRegistry.zoneRegistry(); }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
index 5eae68adf5a..36ab6a9aa60 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
@@ -309,9 +309,6 @@ public class DeploymentContext {
                     throw new AssertionError("Job '" + run.id() + "' was run twice");
 
         assertFalse("Change should have no targets, but was " + instance().change(), instance().change().hasTargets());
-        if (!deferDnsUpdates) {
-            flushDnsUpdates();
-        }
         return this;
     }
 
@@ -411,6 +408,8 @@ public class DeploymentContext {
         var id = newRun(JobType.systemTest);
         var testZone = JobType.systemTest.zone(tester.controller().system());
         runner.run();
+        if ( ! deferDnsUpdates)
+            flushDnsUpdates();
         configServer().convergeServices(instanceId, testZone);
         configServer().convergeServices(testerId.id(), testZone);
         runner.run();
@@ -438,6 +437,9 @@ public class DeploymentContext {
         // First step is always a deployment.
         runner.advance(currentRun(job));
 
+        if ( ! deferDnsUpdates)
+            flushDnsUpdates();
+
         if (job.type().isTest())
             doInstallTester(job);
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index fb0817f8320..44fd677a4d3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -260,11 +260,13 @@ public class InternalStepRunnerTest {
         tester.controllerTester().zoneRegistry().exclusiveRoutingIn(ZoneApiMock.from(systemTestZone), ZoneApiMock.from(stagingZone));
         app.newRun(JobType.systemTest);
         tester.runner().run();
-        tester.configServer().convergeServices(app.instanceId(), JobType.systemTest.zone(system()));
-        tester.configServer().convergeServices(app.testerId().id(), JobType.systemTest.zone(system()));
         assertEquals(unfinished, tester.jobs().last(app.instanceId(), JobType.systemTest).get().stepStatuses().get(Step.installReal));
         assertEquals(unfinished, tester.jobs().last(app.instanceId(), JobType.systemTest).get().stepStatuses().get(Step.installTester));
-        tester.runner().run();
+
+        app.flushDnsUpdates();
+        tester.configServer().convergeServices(app.instanceId(), JobType.systemTest.zone(system()));
+        tester.configServer().convergeServices(app.testerId().id(), JobType.systemTest.zone(system()));
+        tester.runner().run();;
         assertEquals(succeeded, tester.jobs().last(app.instanceId(), JobType.systemTest).get().stepStatuses().get(Step.installReal));
         assertEquals(succeeded, tester.jobs().last(app.instanceId(), JobType.systemTest).get().stepStatuses().get(Step.installTester));
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
index 69f4e0672bf..8863651e1b5 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
@@ -42,7 +42,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
     private final ConfigServerMock configServerMock;
     private final MemoryNameService memoryNameService = new MemoryNameService();
     private final MemoryGlobalRoutingService memoryGlobalRoutingService = new MemoryGlobalRoutingService();
-    private final RoutingGeneratorMock routingGeneratorMock;
+    private final RoutingGeneratorMock routingGeneratorMock = new RoutingGeneratorMock();
     private final MockMailer mockMailer = new MockMailer();
     private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock();
     private final MockMeteringClient mockMeteringClient = new MockMeteringClient();
@@ -55,7 +55,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
     private final MockBilling mockBilling = new MockBilling();
     private final MockAwsEventFetcher mockAwsEventFetcher = new MockAwsEventFetcher();
     private final ArtifactRepositoryMock artifactRepositoryMock = new ArtifactRepositoryMock();
-    private final MockTesterCloud mockTesterCloud = new MockTesterCloud();
+    private final MockTesterCloud mockTesterCloud;
     private final ApplicationStoreMock applicationStoreMock = new ApplicationStoreMock();
     private final MockRunDataStore mockRunDataStore = new MockRunDataStore();
     private final MockTenantCost mockTenantCost = new MockTenantCost();
@@ -64,7 +64,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
     public ServiceRegistryMock(SystemName system) {
         this.zoneRegistryMock = new ZoneRegistryMock(system);
         this.configServerMock = new ConfigServerMock(zoneRegistryMock);
-        this.routingGeneratorMock = new RoutingGeneratorMock();
+        this.mockTesterCloud = new MockTesterCloud(nameService());
     }
 
     @Inject
@@ -193,10 +193,6 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
         return configServerMock;
     }
 
-    public MemoryNameService nameServiceMock() {
-        return memoryNameService;
-    }
-
     public MemoryGlobalRoutingService globalRoutingServiceMock() {
         return memoryGlobalRoutingService;
     }