Move container status.html check as well

8 files changed, 60 insertions, 73 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
index d05e26cf5a5..94988293d07 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/EndpointsChecker.java
@@ -1,6 +1,7 @@
 package com.yahoo.config.provision;
 
 import ai.vespa.http.DomainName;
+import ai.vespa.http.HttpURL;
 
 import javax.naming.NamingException;
 import javax.naming.directory.Attribute;
@@ -19,62 +20,75 @@ import java.util.Optional;
 public class EndpointsChecker {
 
     public record Endpoint(ClusterSpec.Id clusterName,
-                    DomainName dnsName,
-                    Optional<InetAddress> ipAddress,
-                    Optional<DomainName> canonicalName,
-                    boolean isPublic) { }
+                           HttpURL url,
+                           Optional<InetAddress> ipAddress,
+                           Optional<DomainName> canonicalName,
+                           boolean isPublic) { }
 
-    public record UnavailabilityCause(String message) { }
+    public enum Status { available, endpointsUnavailable, containersUnhealthy }
+
+    public record Availability(Status status, String message) { }
 
     public interface HostNameResolver { Optional<InetAddress> resolve(DomainName hostName); }
 
     public interface CNameResolver { Optional<DomainName> resolve(DomainName hostName); }
 
+    public interface ContainerHealthChecker { boolean healthy(Endpoint endpoint); }
+
     private EndpointsChecker() { }
 
-    public static Optional<UnavailabilityCause> endpointsAvailable(List<Endpoint> zoneEndpoints) {
-        return endpointsAvailable(zoneEndpoints, EndpointsChecker::resolveHostName, EndpointsChecker::resolveCname);
+    public static Availability endpointsAvailable(List<Endpoint> zoneEndpoints, ContainerHealthChecker containerHealthChecker) {
+        return endpointsAvailable(zoneEndpoints, EndpointsChecker::resolveHostName, EndpointsChecker::resolveCname, containerHealthChecker);
     }
 
-    public static Optional<UnavailabilityCause> endpointsAvailable(List<Endpoint> zoneEndpoints,
-                                                                   HostNameResolver hostNameResolver,
-                                                                   CNameResolver cNameResolver) {
+    public static Availability endpointsAvailable(List<Endpoint> zoneEndpoints,
+                                                  HostNameResolver hostNameResolver,
+                                                  CNameResolver cNameResolver,
+                                                  ContainerHealthChecker containerHealthChecker) {
         if (zoneEndpoints.isEmpty())
-            return Optional.of(new UnavailabilityCause("Endpoints not yet ready."));
+            return new Availability(Status.endpointsUnavailable, "Endpoints not yet ready.");
 
         for (Endpoint endpoint : zoneEndpoints) {
-            Optional<InetAddress> resolvedIpAddress = hostNameResolver.resolve(endpoint.dnsName());
+            Optional<InetAddress> resolvedIpAddress = hostNameResolver.resolve(endpoint.url().domain());
             if (resolvedIpAddress.isEmpty())
-                return Optional.of(new UnavailabilityCause("DNS lookup yielded no IP address for '" + endpoint.dnsName() + "'."));
+                return new Availability(Status.endpointsUnavailable, "DNS lookup yielded no IP address for '" + endpoint.url().domain() + "'.");
 
             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"));
+                return new Availability(Status.endpointsUnavailable,
+                                        "IP address of '" + endpoint.url().domain() + "' (" +
+                                        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());
+            Optional<DomainName> cNameValue = cNameResolver.resolve(endpoint.url().domain());
             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")));
+                return new Availability(Status.endpointsUnavailable,
+                                        "CNAME '" + endpoint.url().domain() + "' 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 new Availability(Status.endpointsUnavailable,
+                                        "IP address of CNAME '" + endpoint.url().domain() + "' (" +
+                                        resolvedIpAddress.get().getHostAddress() + ") and load balancer '" +
+                                        endpoint.canonicalName().get() + "' (" +
+                                        loadBalancerAddress.map(InetAddress::getHostAddress).orElse("empty") + ") are not equal");
             }
         }
-        return Optional.empty();
+
+        for (Endpoint endpoint : zoneEndpoints)
+            if ( ! containerHealthChecker.healthy(endpoint))
+                return new Availability(Status.containersUnhealthy, "Failed to get enough healthy responses from " + endpoint.url());
+
+        return new Availability(Status.available, "Endpoints are ready");
     }
 
     /** Returns the IP address of the given host name, if any. */
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java
index 0aa86ab211a..22236281a93 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java
@@ -27,6 +27,7 @@ import java.util.List;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 public class HttpProxy {
+
     private final HttpFetcher fetcher;
 
     @Inject public HttpProxy(NodeHostnameVerifier verifier) { this(new SimpleHttpFetcher(Duration.ofSeconds(30), verifier)); }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java
index 25d87b5c940..9821cbe9c15 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java
@@ -19,6 +19,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 public class SimpleHttpFetcher implements HttpFetcher {
+
     private static final Logger logger = Logger.getLogger(SimpleHttpFetcher.class.getName());
 
     private final CloseableHttpClient client;
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 edc5faefe65..9ad06a3311f 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,14 +1,11 @@
 // Copyright Yahoo. 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 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.config.provision.EndpointsChecker.Availability;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
 
-import java.net.InetAddress;
 import java.net.URI;
 import java.util.List;
 import java.util.Optional;
@@ -29,13 +26,10 @@ public interface TesterCloud {
     /** Returns the current status of the tester. */
     Status getStatus(DeploymentId deploymentId);
 
-    /** Returns whether the container is ready to serve. */
-    boolean ready(URI endpointUrl);
-
     /** Returns whether the test container is ready to serve */
     boolean testerReady(DeploymentId deploymentId);
 
-    Optional<UnavailabilityCause> verifyEndpoints(List<Endpoint> endpoints);
+    Availability 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 7a819ecc1db..e9a7c8bad33 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
@@ -5,7 +5,7 @@ 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.config.provision.EndpointsChecker.Availability;
 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;
@@ -19,7 +19,6 @@ 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;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Status.RUNNING;
@@ -52,18 +51,13 @@ public class MockTesterCloud implements TesterCloud {
     public Status getStatus(DeploymentId deploymentId) { return status; }
 
     @Override
-    public boolean ready(URI testerUrl) {
-        return true;
-    }
-
-    @Override
     public boolean testerReady(DeploymentId deploymentId) {
         return true;
     }
 
     @Override
-    public Optional<UnavailabilityCause> verifyEndpoints(List<Endpoint> endpoints) {
-        return EndpointsChecker.endpointsAvailable(endpoints, this::resolveHostName, this::resolveCname);
+    public Availability verifyEndpoints(List<Endpoint> endpoints) {
+        return EndpointsChecker.endpointsAvailable(endpoints, this::resolveHostName, this::resolveCname, __ -> true);
     }
 
     private Optional<InetAddress> resolveHostName(DomainName hostname) {
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 8ab0a7ce56d..d7118c4b3c2 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
@@ -2,7 +2,7 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
 import ai.vespa.http.DomainName;
-import com.google.common.net.InetAddresses;
+import ai.vespa.http.HttpURL;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.Notifications;
@@ -10,6 +10,8 @@ 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.EndpointsChecker.Availability;
+import com.yahoo.config.provision.EndpointsChecker.Status;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.RoutingMethod;
@@ -353,13 +355,13 @@ public class InternalStepRunner implements StepRunner {
         }
         if (summary.converged()) {
             controller.jobController().locked(id, lockedRun -> lockedRun.withSummary(null));
-            if (endpointsAvailable(id.application(), id.type().zone(), logger)) {
-                if (containersAreUp(id.application(), id.type().zone(), logger)) {
+            Availability availability = endpointsAvailable(id.application(), id.type().zone(), logger);
+            if (availability.status() == Status.available) {
                     logger.log("Installation succeeded!");
                     return Optional.of(running);
-                }
             }
-            else if (timedOut(id, deployment.get(), timeouts.endpoint())) {
+            logger.log(availability.message());
+            if (availability.status() == Status.endpointsUnavailable && timedOut(id, deployment.get(), timeouts.endpoint())) {
                 logger.log(WARNING, "Endpoints failed to show up within " + timeouts.endpoint().toMinutes() + " minutes!");
                 return Optional.of(error);
             }
@@ -478,21 +480,6 @@ public class InternalStepRunner implements StepRunner {
         return Optional.empty();
     }
 
-    /** Returns true iff all calls to endpoint in the deployment give 100 consecutive 200 OK responses on /status.html. */
-    private boolean containersAreUp(ApplicationId id, ZoneId zoneId, DualLogger logger) {
-        var endpoints = controller.routing().readTestRunnerEndpointsOf(Set.of(new DeploymentId(id, zoneId)));
-        if ( ! endpoints.containsKey(zoneId))
-            return false;
-
-        return endpoints.get(zoneId).parallelStream().allMatch(endpoint -> {
-            boolean ready = controller.jobController().cloud().ready(endpoint.url());
-            if (!ready) {
-                logger.log("Failed to get 100 consecutive OKs from " + endpoint);
-            }
-            return ready;
-        });
-    }
-
     /** Returns true iff all containers in the tester deployment give 100 consecutive 200 OK responses on /status.html. */
     private boolean testerContainersAreUp(ApplicationId id, ZoneId zoneId, DualLogger logger) {
         DeploymentId deploymentId = new DeploymentId(id, zoneId);
@@ -504,29 +491,24 @@ public class InternalStepRunner implements StepRunner {
         }
     }
 
-    private boolean endpointsAvailable(ApplicationId id, ZoneId zone, DualLogger logger) {
+    private Availability endpointsAvailable(ApplicationId id, ZoneId zone, DualLogger logger) {
         DeploymentId deployment = new DeploymentId(id, zone);
         Map<ZoneId, List<Endpoint>> endpoints = controller.routing().readTestRunnerEndpointsOf(Set.of(deployment));
+        logEndpoints(endpoints, logger);
         DeploymentRoutingContext context = controller.routing().of(deployment);
         boolean resolveEndpoints = context.routingMethod() == RoutingMethod.exclusive;
-        var unavailableCause = controller.serviceRegistry().testerCloud().verifyEndpoints(
+        return 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()),
+                                                                  HttpURL.from(endpoint.url()),
                                                                   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;
-        }
-        logEndpoints(endpoints, logger);
-        return true;
     }
 
     private void logEndpoints(Map<ZoneId, List<Endpoint>> zoneEndpoints, DualLogger logger) {
diff --git a/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java b/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java
index 4f2bdfb213e..c5ebafb2425 100644
--- a/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java
+++ b/http-utils/src/main/java/ai/vespa/util/http/hc5/VespaHttpClientBuilder.java
@@ -31,6 +31,7 @@ import static com.yahoo.security.tls.TransportSecurityUtils.isTransportSecurityE
  * @author jonmv
  */
 public class VespaHttpClientBuilder {
+
     private HttpClientConnectionManagerFactory connectionManagerFactory = PoolingHttpClientConnectionManager::new;
     private HostnameVerifier hostnameVerifier = new NoopHostnameVerifier();
     private boolean rewriteHttpToHttps = true;
diff --git a/vespajlib/src/main/java/ai/vespa/http/HttpURL.java b/vespajlib/src/main/java/ai/vespa/http/HttpURL.java
index 9641ea2a8fd..ba1a8e08740 100644
--- a/vespajlib/src/main/java/ai/vespa/http/HttpURL.java
+++ b/vespajlib/src/main/java/ai/vespa/http/HttpURL.java
@@ -239,7 +239,7 @@ public class HttpURL {
             return parse(raw, HttpURL::requirePathSegment);
         }
 
-        /** Parses the given raw, normalized path string; this ignores whether the path is absolute or relative.) */
+        /** Parses the given raw, normalized path string; this ignores whether the path is absolute or relative. */
         public static Path parse(String raw, Consumer<String> validator) {
             Path path = new Path(null, 0, raw.endsWith("/"), segmentValidator(validator));
             if (raw.startsWith("/")) raw = raw.substring(1);