Merge pull request #11812 from vespa-engine/hmusum/cleanup-log-fetching

Hmusum/cleanup log fetching

3 files changed, 42 insertions, 30 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 43b78212827..35c41711a6e 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
@@ -759,12 +759,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     private String getLogServerURI(ApplicationId applicationId, Optional<String> hostname) {
         // Allow to get logs from a given hostname if the application is under the hosted-vespa tenant.
         // We make no validation that the hostname is actually allocated to the given application since
-        // most applications under hosted-vespa are not known to the model and its OK for a user to get
+        // most applications under hosted-vespa are not known to the model and it's OK for a user to get
         // logs for any host if they are authorized for the hosted-vespa tenant.
         if (hostname.isPresent()) {
             if (HOSTED_VESPA_TENANT.equals(applicationId.tenant()))
                 return "http://" + hostname.get() + ":8080/logs";
-            else throw new IllegalArgumentException("Only hostname paramater unsupported for application " + applicationId);
+            else
+                throw new IllegalArgumentException("Using hostname parameter when getting logs is not supported for application "
+                                                   + applicationId);
         }
 
         Application application = getApplication(applicationId);
@@ -773,7 +775,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         HostInfo logServerHostInfo = hostInfos.stream()
                 .filter(host -> host.getServices().stream()
                         .anyMatch(serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver")))
-                .findFirst().orElseThrow(() -> new IllegalArgumentException("Could not find HostInfo for LogServer"));
+                .findFirst().orElseThrow(() -> new IllegalArgumentException("Could not find host info for logserver"));
 
         ServiceInfo serviceInfo = logServerHostInfo.getServices().stream().filter(service -> List.of(LOGSERVER_CONTAINER.serviceName, CONTAINER.serviceName).contains(service.getServiceType()))
                 .findFirst().orElseThrow(() -> new IllegalArgumentException("No container running on logserver host"));
@@ -782,11 +784,10 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     }
 
     private int servicePort(ServiceInfo serviceInfo) {
-        int port = serviceInfo.getPorts().stream()
+        return serviceInfo.getPorts().stream()
                 .filter(portInfo -> portInfo.getTags().stream().anyMatch(tag -> tag.equalsIgnoreCase("http")))
                 .findFirst().orElseThrow(() -> new IllegalArgumentException("Could not find HTTP port"))
                 .getPort();
-        return port;
     }
 
     public Slime createDeployLog() {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java
index 0f5642525d3..6549f359b29 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/LogRetriever.java
@@ -4,13 +4,10 @@ package com.yahoo.vespa.config.server.http;
 import ai.vespa.util.http.VespaHttpClientBuilder;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.yolean.Exceptions;
-import org.apache.http.Header;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
 
 import java.io.IOException;
-import java.io.OutputStream;
-import java.util.Optional;
 import java.util.logging.Logger;
 
 /**
@@ -31,26 +28,4 @@ public class LogRetriever {
         }
     }
 
-    private static class ProxyResponse extends HttpResponse {
-
-        private final org.apache.http.HttpResponse clientResponse;
-
-        private ProxyResponse(org.apache.http.HttpResponse clientResponse) {
-            super(clientResponse.getStatusLine().getStatusCode());
-            this.clientResponse = clientResponse;
-        }
-
-        @Override
-        public String getContentType() {
-            return Optional.ofNullable(clientResponse.getFirstHeader("Content-Type"))
-                    .map(Header::getValue)
-                    .orElseGet(super::getContentType);
-        }
-
-        @Override
-        public void render(OutputStream outputStream) throws IOException {
-            clientResponse.getEntity().writeTo(outputStream);
-        }
-    }
-
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ProxyResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ProxyResponse.java
new file mode 100644
index 00000000000..4c310dd8a0d
--- /dev/null
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ProxyResponse.java
@@ -0,0 +1,36 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.config.server.http;
+
+import com.yahoo.container.jdisc.HttpResponse;
+import org.apache.http.Header;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.Optional;
+
+/**
+ * Proxies response back to client, keeps Content-Type header if it is present
+ *
+ * @author Ola Aunrønning
+ */
+class ProxyResponse extends HttpResponse {
+
+    private final org.apache.http.HttpResponse clientResponse;
+
+    ProxyResponse(org.apache.http.HttpResponse clientResponse) {
+        super(clientResponse.getStatusLine().getStatusCode());
+        this.clientResponse = clientResponse;
+    }
+
+    @Override
+    public String getContentType() {
+        return Optional.ofNullable(clientResponse.getFirstHeader("Content-Type"))
+                .map(Header::getValue)
+                .orElseGet(super::getContentType);
+    }
+
+    @Override
+    public void render(OutputStream outputStream) throws IOException {
+        clientResponse.getEntity().writeTo(outputStream);
+    }
+}