Refactor endpoint verification during deployments

7 files changed, 156 insertions, 48 deletions

diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java b/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java
new file mode 100644
index 00000000000..d05e26cf5a5
--- /dev/null
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java
@@ -0,0 +1,114 @@
+package com.yahoo.config.provision;
+
+import ai.vespa.http.DomainName;
+
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.directory.InitialDirContext;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * @author jonmv
+ */
+public class EndpointsChecker {
+
+    public record Endpoint(ClusterSpec.Id clusterName,
+                    DomainName dnsName,
+                    Optional<InetAddress> ipAddress,
+                    Optional<DomainName> canonicalName,
+                    boolean isPublic) { }
+
+    public record UnavailabilityCause(String message) { }
+
+    public interface HostNameResolver { Optional<InetAddress> resolve(DomainName hostName); }
+
+    public interface CNameResolver { Optional<DomainName> resolve(DomainName hostName); }
+
+    private EndpointsChecker() { }
+
+    public static Optional<UnavailabilityCause> endpointsAvailable(List<Endpoint> zoneEndpoints) {
+        return endpointsAvailable(zoneEndpoints, EndpointsChecker::resolveHostName, EndpointsChecker::resolveCname);
+    }
+
+    public static Optional<UnavailabilityCause> endpointsAvailable(List<Endpoint> zoneEndpoints,
+                                                                   HostNameResolver hostNameResolver,
+                                                                   CNameResolver cNameResolver) {
+        if (zoneEndpoints.isEmpty())
+            return Optional.of(new UnavailabilityCause("Endpoints not yet ready."));
+
+        for (Endpoint endpoint : zoneEndpoints) {
+            Optional<InetAddress> resolvedIpAddress = hostNameResolver.resolve(endpoint.dnsName());
+            if (resolvedIpAddress.isEmpty())
+                return Optional.of(new UnavailabilityCause("DNS lookup yielded no IP address for '" + endpoint.dnsName() + "'."));
+
+            if (resolvedIpAddress.equals(endpoint.ipAddress())) // We expect a certain IP address, and that's what we got, so we're good.
+                continue;
+
+            if (endpoint.ipAddress().isPresent()) // We expect a certain IP address, but that's not what we got.
+                return Optional.of(new UnavailabilityCause("IP address of '" + endpoint.dnsName() + "' (" +
+                                                           resolvedIpAddress.get().getHostAddress() + ") and load balancer " +
+                                                           "' (" + endpoint.ipAddress().get().getHostAddress() + ") are not equal"));
+
+            if (endpoint.canonicalName().isEmpty()) // We have no expected IP address, and no canonical name, so there's nothing more to check.
+                continue;
+
+            Optional<DomainName> cNameValue = cNameResolver.resolve(endpoint.dnsName());
+            if (cNameValue.filter(endpoint.canonicalName().get()::equals).isEmpty()) {
+                return Optional.of(new UnavailabilityCause("CNAME '" + endpoint.dnsName() + "' points at " +
+                                                           cNameValue.map(name -> "'" + name + "'").orElse("nothing") +
+                                                           " but should point at load balancer " +
+                                                           endpoint.canonicalName().map(name -> "'" + name + "'").orElse("nothing")));
+            }
+
+            Optional<InetAddress> loadBalancerAddress = hostNameResolver.resolve(endpoint.canonicalName().get());
+            if ( ! loadBalancerAddress.equals(resolvedIpAddress)) {
+                return Optional.of(new UnavailabilityCause("IP address of CNAME '" + endpoint.dnsName() + "' (" +
+                                                           resolvedIpAddress.get().getHostAddress() + ") and load balancer '" +
+                                                           endpoint.canonicalName().get() + "' (" +
+                                                           loadBalancerAddress.map(InetAddress::getHostAddress).orElse("empty") + ") are not equal"));
+            }
+        }
+        return Optional.empty();
+    }
+
+    /** Returns the IP address of the given host name, if any. */
+    static Optional<InetAddress> resolveHostName(DomainName hostname) {
+        try {
+            return Optional.of(InetAddress.getByName(hostname.value()));
+        }
+        catch (UnknownHostException ignored) {
+            return Optional.empty();
+        }
+    }
+
+    /** Returns the host name of the given CNAME, if any. */
+    static Optional<DomainName> resolveCname(DomainName endpoint) {
+        try {
+            InitialDirContext ctx = new InitialDirContext();
+            try {
+                Attributes attrs = ctx.getAttributes("dns:/" + endpoint.value(), new String[]{ "CNAME" });
+                for (Attribute attribute : Collections.list(attrs.getAll())) {
+                    Enumeration<?> vals = attribute.getAll();
+                    if (vals.hasMoreElements()) {
+                        String hostname = vals.nextElement().toString();
+                        return Optional.of(hostname.substring(0, hostname.length() - 1)).map(DomainName::of);
+                    }
+                }
+            }
+            finally {
+                ctx.close();
+            }
+        }
+        catch (NamingException e) {
+            throw new RuntimeException(e);
+        }
+        return Optional.empty();
+    }
+
+}
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 b4d9dd49880..edc5faefe65 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
@@ -2,6 +2,8 @@
 package com.yahoo.vespa.hosted.controller.api.integration.deployment;
 
 import ai.vespa.http.DomainName;
+import com.yahoo.config.provision.EndpointsChecker.Endpoint;
+import com.yahoo.config.provision.EndpointsChecker.UnavailabilityCause;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
@@ -33,11 +35,7 @@ public interface TesterCloud {
     /** Returns whether the test container is ready to serve */
     boolean testerReady(DeploymentId deploymentId);
 
-    /** Returns the IP address of the given host name, if any. */
-    Optional<InetAddress> resolveHostName(DomainName hostname);
-
-    /** Returns the host name of the given CNAME, if any. */
-    Optional<DomainName> resolveCname(DomainName hostName);
+    Optional<UnavailabilityCause> verifyEndpoints(List<Endpoint> endpoints);
 
     /** Returns the test report as JSON if available */
     Optional<TestReport> getTestReport(DeploymentId deploymentId);
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 939c74fe61d..7a819ecc1db 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
@@ -3,6 +3,9 @@ package com.yahoo.vespa.hosted.controller.api.integration.stubs;
 
 import ai.vespa.http.DomainName;
 import com.google.common.net.InetAddresses;
+import com.yahoo.config.provision.EndpointsChecker;
+import com.yahoo.config.provision.EndpointsChecker.Endpoint;
+import com.yahoo.config.provision.EndpointsChecker.UnavailabilityCause;
 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.TestReport;
@@ -59,15 +62,18 @@ public class MockTesterCloud implements TesterCloud {
     }
 
     @Override
-    public Optional<InetAddress> resolveHostName(DomainName hostname) {
+    public Optional<UnavailabilityCause> verifyEndpoints(List<Endpoint> endpoints) {
+        return EndpointsChecker.endpointsAvailable(endpoints, this::resolveHostName, this::resolveCname);
+    }
+
+    private Optional<InetAddress> resolveHostName(DomainName hostname) {
         return nameService.findRecords(Record.Type.A, RecordName.from(hostname.value())).stream()
                 .findFirst()
                 .map(record -> InetAddresses.forString(record.data().asString()))
                 .or(() -> Optional.of(InetAddresses.forString("1.2.3.4")));
     }
 
-    @Override
-    public Optional<DomainName> resolveCname(DomainName hostName) {
+    private Optional<DomainName> resolveCname(DomainName hostName) {
         return nameService.findRecords(Record.Type.CNAME, RecordName.from(hostName.value())).stream()
                           .findFirst()
                           .map(record -> DomainName.of(record.data().asString().substring(0, record.data().asString().length() - 1)));
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 7da61f9bc63..8ab0a7ce56d 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
@@ -9,6 +9,7 @@ import com.yahoo.config.application.api.Notifications;
 import com.yahoo.config.application.api.Notifications.When;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.EndpointsChecker;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.RoutingMethod;
@@ -45,6 +46,7 @@ import com.yahoo.yolean.Exceptions;
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.io.UncheckedIOException;
+import java.net.InetAddress;
 import java.security.cert.CertificateExpiredException;
 import java.security.cert.CertificateNotYetValidException;
 import java.security.cert.X509Certificate;
@@ -87,6 +89,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.Step.deployReal;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.deployTester;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.installTester;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.report;
+import static com.yahoo.yolean.Exceptions.uncheck;
 import static java.util.Objects.requireNonNull;
 import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.INFO;
@@ -504,45 +507,24 @@ public class InternalStepRunner implements StepRunner {
     private boolean endpointsAvailable(ApplicationId id, ZoneId zone, DualLogger logger) {
         DeploymentId deployment = new DeploymentId(id, zone);
         Map<ZoneId, List<Endpoint>> endpoints = controller.routing().readTestRunnerEndpointsOf(Set.of(deployment));
-        if ( ! endpoints.containsKey(zone)) {
-            logger.log("Endpoints not yet ready.");
+        DeploymentRoutingContext context = controller.routing().of(deployment);
+        boolean resolveEndpoints = context.routingMethod() == RoutingMethod.exclusive;
+        var unavailableCause = controller.serviceRegistry().testerCloud().verifyEndpoints(
+                endpoints.getOrDefault(zone, List.of())
+                         .stream()
+                         .map(endpoint -> {
+                             ClusterSpec.Id cluster = ClusterSpec.Id.from(endpoint.name());
+                             RoutingPolicy policy = context.routingPolicy(cluster).get();
+                             return new EndpointsChecker.Endpoint(cluster,
+                                                                  DomainName.of(endpoint.dnsName()),
+                                                                  policy.ipAddress().filter(__ -> resolveEndpoints).map(uncheck(InetAddress::getByName)),
+                                                                  policy.canonicalName().filter(__ -> resolveEndpoints),
+                                                                  policy.isPublic());
+                         }).toList());
+        if (unavailableCause.isPresent()) {
+            logger.log(unavailableCause.get().message());
             return false;
         }
-        for (var endpoint : endpoints.get(zone)) {
-            DomainName endpointName = DomainName.of(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;
-            }
-            DeploymentRoutingContext context = controller.routing().of(deployment);
-            if (context.routingMethod() == RoutingMethod.exclusive)  {
-                RoutingPolicy policy = context.routingPolicy(ClusterSpec.Id.from(endpoint.name()))
-                                              .orElseThrow(() -> new IllegalStateException(endpoint + " has no matching policy"));
-                if (policy.ipAddress().isPresent()) {
-                    if (ipAddress.equals(policy.ipAddress().map(InetAddresses::forString))) continue;
-                    logger.log(INFO, "IP address of '" + endpointName + "' (" +
-                            ipAddress.map(InetAddresses::toAddrString).get() + ") and load balancer "
-                            + "' (" + policy.ipAddress().orElseThrow() + ") are not equal");
-                    return false;
-                }
-
-                var cNameValue = controller.jobController().cloud().resolveCname(endpointName);
-                if ( ! cNameValue.map(policy.canonicalName().get()::equals).orElse(false)) {
-                    logger.log(INFO, "CNAME '" + endpointName + "' points at " +
-                                     cNameValue.map(name -> "'" + name + "'").orElse("nothing") +
-                                     " but should point at load balancer '" + policy.canonicalName() + "'");
-                    return false;
-                }
-                var loadBalancerAddress = controller.jobController().cloud().resolveHostName(policy.canonicalName().get());
-                if ( ! loadBalancerAddress.equals(ipAddress)) {
-                    logger.log(INFO, "IP address of CNAME '" + endpointName + "' (" + ipAddress.get() + ") and load balancer '" +
-                                     policy.canonicalName().get() + "' (" + loadBalancerAddress.orElse(null) + ") are not equal");
-                    return false;
-                }
-            }
-        }
-
         logEndpoints(endpoints, logger);
         return true;
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java
index acf3ba99a1a..38ecff452c8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java
@@ -7,7 +7,6 @@ import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.RoutingMethod;
 import com.yahoo.text.Text;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
-import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.application.Endpoint;
 import com.yahoo.vespa.hosted.controller.application.Endpoint.Port;
 import com.yahoo.vespa.hosted.controller.application.EndpointId;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
index 03322d7c962..1efafb81685 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
@@ -364,8 +364,8 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
         ControllerTester wrapped = new ControllerTester(tester);
         wrapped.upgradeSystem(Version.fromString("7.1"));
         new DeploymentTester(wrapped).newDeploymentContext(ApplicationId.from(tenantName, applicationName, InstanceName.defaultName()))
-                .submit()
-                .deploy();
+                                     .submit()
+                                     .deploy();
 
         tester.assertResponse(request("/application/v4/tenant/scoober", GET).roles(Role.reader(tenantName)),
                 (response) -> assertFalse(response.getBodyAsString().contains("archiveAccessRole")),
diff --git a/vespajlib/src/main/java/com/yahoo/yolean/Exceptions.java b/vespajlib/src/main/java/com/yahoo/yolean/Exceptions.java
index 89b4e76368b..4f3f048eb0c 100644
--- a/vespajlib/src/main/java/com/yahoo/yolean/Exceptions.java
+++ b/vespajlib/src/main/java/com/yahoo/yolean/Exceptions.java
@@ -4,6 +4,7 @@ package com.yahoo.yolean;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.util.Optional;
+import java.util.function.Function;
 
 /**
  * Helper methods for handling exceptions
@@ -129,6 +130,14 @@ public class Exceptions {
     @FunctionalInterface public interface RunnableThrowingInterruptedException { void run() throws InterruptedException; }
 
     /**
+     * Wraps any IOException thrown from a function in an UncheckedIOException.
+     */
+    public static <T, R> Function<T, R> uncheck(FunctionThrowingIOException<T, R> function) {
+        return t -> uncheck(() -> function.map(t));
+    }
+    @FunctionalInterface public interface FunctionThrowingIOException<T, R> { R map(T t) throws IOException; }
+
+    /**
      * Wraps any IOException thrown from a supplier in an UncheckedIOException.
      */
     public static <T> T uncheck(SupplierThrowingIOException<T> supplier) {