Merge pull request #18553 from vespa-engine/freva/status-storage-distributor

Status page for storagenode and distributor

6 files changed, 37 insertions, 20 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index 6249e3bfff8..56fd6a64305 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -547,15 +547,27 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         }
     }
 
-    public HttpResponse clusterControllerStatusPage(ApplicationId applicationId, String hostName, String pathSuffix) {
+    public HttpResponse serviceStatusPage(ApplicationId applicationId, String hostName, String serviceName, String pathSuffix) {
         // WARNING: pathSuffix may be given by the external user. Make sure no security issues arise...
         // We should be OK here, because at most, pathSuffix may change the parent path, but cannot otherwise
         // change the hostname and port. Exposing other paths on the cluster controller should be fine.
         // TODO: It would be nice to have a simple check to verify pathSuffix doesn't contain /../ components.
-        String relativePath = "clustercontroller-status/" + pathSuffix;
+        String pathPrefix;
+        switch (serviceName) {
+            case "container-clustercontroller": {
+                pathPrefix = "clustercontroller-status/v1/";
+                break;
+            }
+            case "distributor":
+            case "storagenode": {
+                pathPrefix = "";
+                break;
+            }
+            default:
+                throw new NotFoundException("No status page for service: " + serviceName);
+        }
 
-        return httpProxy.get(getApplication(applicationId), hostName,
-                             CLUSTERCONTROLLER_CONTAINER.serviceName, relativePath);
+        return httpProxy.get(getApplication(applicationId), hostName, serviceName, pathPrefix + pathSuffix);
     }
 
     public Map<String, ClusterReindexing> getClusterReindexingStatus(ApplicationId applicationId) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
index e56fb0b1bcc..ec7b6616bb6 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
@@ -68,13 +68,13 @@ public class ApplicationHandler extends HttpHandler {
 
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}")) return getApplicationResponse(ApplicationId.from(path.get("tenant"), path.get("application"), InstanceName.defaultName().value()));
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}")) return getApplicationResponse(applicationId(path));
-        if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/clustercontroller/{hostname}/status/{*}")) return clusterControllerStatusPage(applicationId(path), path.get("hostname"), path.getRest());
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/content/{*}")) return content(applicationId(path), path.getRest(), request);
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/filedistributionstatus")) return filedistributionStatus(applicationId(path), request);
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/logs")) return logs(applicationId(path), request);
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/metrics/deployment")) return deploymentMetrics(applicationId(path));
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/metrics/proton")) return protonMetrics(applicationId(path));
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/reindexing")) return getReindexingStatus(applicationId(path));
+        if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/service/{service}/{hostname}/status/{*}")) return serviceStatusPage(applicationId(path), path.get("service"), path.get("hostname"), path.getRest());
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/serviceconverge")) return listServiceConverge(applicationId(path), request);
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/serviceconverge/{hostAndPort}")) return checkServiceConverge(applicationId(path), path.get("hostAndPort"), request);
         if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/suspended")) return isSuspended(applicationId(path));
@@ -115,8 +115,8 @@ public class ApplicationHandler extends HttpHandler {
                 getTimeoutFromRequest(request), getVespaVersionFromRequest(request));
     }
 
-    private HttpResponse clusterControllerStatusPage(ApplicationId applicationId, String hostname, String pathSuffix) {
-        return applicationRepository.clusterControllerStatusPage(applicationId, hostname, pathSuffix);
+    private HttpResponse serviceStatusPage(ApplicationId applicationId, String service, String hostname, String pathSuffix) {
+        return applicationRepository.serviceStatusPage(applicationId, hostname, service, pathSuffix);
     }
 
     private HttpResponse content(ApplicationId applicationId, String contentPath, HttpRequest request) {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
index dd7604264f8..46d94ec476b 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
@@ -60,7 +60,6 @@ import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.stream.Stream;
 
-import static com.yahoo.config.model.api.container.ContainerServiceType.CLUSTERCONTROLLER_CONTAINER;
 import static com.yahoo.container.jdisc.HttpRequest.createTestRequest;
 import static com.yahoo.jdisc.http.HttpRequest.Method.DELETE;
 import static com.yahoo.jdisc.http.HttpRequest.Method.GET;
@@ -72,9 +71,8 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 /**
  * @author hmusum
@@ -343,10 +341,9 @@ public class ApplicationHandlerTest {
     }
 
     @Test
-    public void testClusterControllerStatus() throws Exception {
+    public void testServiceStatus() throws Exception {
         applicationRepository.deploy(testApp, prepareParams(applicationId));
         String host = "foo.yahoo.com";
-        String url = toUrlPath(applicationId, Zone.defaultZone(), true) + "/clustercontroller/" + host + "/status/v1/clusterName1";
         HttpProxy mockHttpProxy = mock(HttpProxy.class);
         ApplicationRepository applicationRepository = new ApplicationRepository.Builder()
                 .withTenantRepository(tenantRepository)
@@ -356,11 +353,19 @@ public class ApplicationHandlerTest {
                 .withHttpProxy(mockHttpProxy)
                 .build();
         ApplicationHandler mockHandler = createApplicationHandler(applicationRepository);
-        when(mockHttpProxy.get(any(), eq(host), eq(CLUSTERCONTROLLER_CONTAINER.serviceName), eq("clustercontroller-status/v1/clusterName1")))
-                .thenReturn(new StaticResponse(200, "text/html", "<html>...</html>"));
+        doAnswer(invoc -> new StaticResponse(200, "text/html", "<html>" +
+                "host=" + invoc.getArgument(1, String.class) + "," +
+                "service=" + invoc.getArgument(2, String.class) + "," +
+                "path=" + invoc.getArgument(3, String.class) + "</html>")).when(mockHttpProxy).get(any(), any(), any(), any());
 
-        HttpResponse response = mockHandler.handle(createTestRequest(url, GET));
-        assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>...</html>");
+        HttpResponse response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/container-clustercontroller/" + host + "/status/some/path/clusterName1", GET));
+        assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=container-clustercontroller,path=clustercontroller-status/v1/some/path/clusterName1</html>");
+
+        response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/distributor/" + host + "/status/something", GET));
+        assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=distributor,path=something</html>");
+
+        response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/fake-service/" + host + "/status/something", GET));
+        assertHttpStatusCodeAndMessage(response, 404, "{\"error-code\":\"NOT_FOUND\",\"message\":\"No status page for service: fake-service\"}");
     }
 
     @Test
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
index 81c4d7be483..8d9f20a7cee 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java
@@ -55,7 +55,7 @@ public interface ConfigServer {
 
     Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, String restPath);
 
-    String getClusterControllerStatus(DeploymentId deployment, String node, String subPath);
+    String getServiceStatusPage(DeploymentId deployment, String serviceName, String node, String subPath);
 
     /**
      * Gets the Vespa logs of the given deployment.
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 21b0fb12a5c..a0e5934fbc5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -1678,9 +1678,9 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
     private HttpResponse service(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String restPath, HttpRequest request) {
         DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region));
 
-        if ("container-clustercontroller".equals((serviceName)) && restPath.contains("/status/")) {
+        if (restPath.contains("/status/")) {
             String[] parts = restPath.split("/status/");
-            String result = controller.serviceRegistry().configServer().getClusterControllerStatus(deploymentId, parts[0], parts[1]);
+            String result = controller.serviceRegistry().configServer().getServiceStatusPage(deploymentId, serviceName, parts[0], parts[1]);
             return new HtmlResponse(result);
         }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 407ab2432a5..92c8cbc4889 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -532,7 +532,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     }
 
     @Override
-    public String getClusterControllerStatus(DeploymentId deployment, String node, String subPath) {
+    public String getServiceStatusPage(DeploymentId deployment, String serviceName, String node, String subPath) {
         return "<h1>OK</h1>";
     }