Use model instead of config for getting logserver connection info

* Add unit test
* Minor refactorings

3 files changed, 70 insertions, 51 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java
index 4928663c3b0..e71fd3f31fc 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java
@@ -177,8 +177,7 @@ public class Application implements ModelResult {
         return resolveConfig(req, new UncompressedConfigResponseFactory());
     }
 
-    // TODO: Remove 'throws IOException'
-    public <CONFIGTYPE extends ConfigInstance> CONFIGTYPE getConfig(Class<CONFIGTYPE> configClass, String configId) throws IOException {
+    public <CONFIGTYPE extends ConfigInstance> CONFIGTYPE getConfig(Class<CONFIGTYPE> configClass, String configId) {
         ConfigKey<CONFIGTYPE> key = new ConfigKey<>(configClass, configId);
         ConfigPayload payload = model.getConfig(key, (ConfigDefinition)null, null);
         return payload.toInstance(configClass, configId);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/LogServerLogGrabber.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/LogServerLogGrabber.java
index ba8fdc00ccf..810588bbb8d 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/LogServerLogGrabber.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/LogServerLogGrabber.java
@@ -1,8 +1,9 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.application;
 
-import com.yahoo.cloud.config.ModelConfig;
 import com.yahoo.component.AbstractComponent;
+import com.yahoo.config.model.api.PortInfo;
+import com.yahoo.config.model.api.ServiceInfo;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.config.server.http.InternalServerException;
 import com.yahoo.yolean.Exceptions;
@@ -11,11 +12,13 @@ import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.net.Socket;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Optional;
 
 /**
- * Fetches log entries from logserver with level errors and fatal. The logserver only return
- * a log entry once over this API so doing repeated call will not give the same results.
+ * Fetches log entries from logserver with level errors and fatal. The logserver only returns
+ * a log entry once over this API so doing repeated calls will not give the same results.
  *
  * @author dybis
  */
@@ -24,60 +27,34 @@ public class LogServerLogGrabber extends AbstractComponent {
 
     public LogServerLogGrabber() {}
 
-    private Optional<Integer> getErrorLogPort(ModelConfig.Hosts.Services service) {
-        return service.ports().stream()
-            .filter(port -> port.tags().toLowerCase().contains("last-errors-holder"))
-            .map(ModelConfig.Hosts.Services.Ports::number)
-            .findFirst();
-    }
-
-    private class LogServerConnectionInfo {
-        String hostName;
-        int port;
-    }
-
     public String grabLog(Application application) {
-
-        // TODO: Use model to get values (see how it's done in ApplicationConvergenceChecker)
-        final ModelConfig config;
+        LogServerInfo logServerConnectionInfo = findLogserverConnectionInfo(application);
+        log.log(LogLevel.DEBUG, "Requested error logs, pulling from logserver on " + logServerConnectionInfo);
         try {
-            config = application.getConfig(ModelConfig.class, "");
+            return readLog(logServerConnectionInfo.hostName, logServerConnectionInfo.port);
         } catch (IOException e) {
-            throw new RuntimeException(e);
+            throw new InternalServerException(Exceptions.toMessageString(e));
         }
+    }
 
-        final LogServerConnectionInfo logServerConnectionInfo = new LogServerConnectionInfo();
+    private LogServerInfo findLogserverConnectionInfo(Application application) {
+        List<LogServerInfo> logServerConnectionInfos = new ArrayList<>();
+        application.getModel().getHosts()
+                   .forEach(host -> host.getServices().stream()
+                                        .filter(service -> service.getServiceType().equals("logserver"))
+                                        .forEach(logService -> {
+                                            Optional<Integer> logPort = getErrorLogPort(logService);
+                                            logPort.ifPresent(port -> logServerConnectionInfos.add(new LogServerInfo(host.getHostname(), port)));
+                                        }));
 
-        config.hosts().stream()
-                .forEach(host -> host.services().stream()
-                        .filter(service -> service.type().equals("logserver"))
-                        .forEach(logService -> {
-                            Optional<Integer> logPort = getErrorLogPort(logService);
-                            if (logPort.isPresent()) {
-                                if (logServerConnectionInfo.hostName != null) {
-                                    throw new RuntimeException("Found several log server ports");
-                                }
-                                logServerConnectionInfo.hostName = host.name();
-                                logServerConnectionInfo.port = logPort.get();
-                            }
-                        }));
+        if (logServerConnectionInfos.size() > 1) throw new RuntimeException("Found several log server ports");
+        if (logServerConnectionInfos.size() == 0) throw new InternalServerException("Did not find any log server in config model");
 
-        if (logServerConnectionInfo.hostName == null) {
-            throw new InternalServerException("Did not find any log server in config model");
-        }
-        log.log(LogLevel.DEBUG, "Requested error logs, pulling from logserver on " + logServerConnectionInfo.hostName + " "
-                + logServerConnectionInfo.port);
-        final String response;
-        try {
-            response = readLog(logServerConnectionInfo.hostName, logServerConnectionInfo.port);
-            log.log(LogLevel.DEBUG, "Requested error logs was " + response.length() + " characters");
-        } catch (IOException e) {
-            throw new InternalServerException(Exceptions.toMessageString(e));
-        }
-        return response;
+        return logServerConnectionInfos.get(0);
     }
 
-    private String readLog(String host, int port) throws IOException {
+    // Protected to be able to test
+    protected String readLog(String host, int port) throws IOException {
         Socket socket = new Socket(host, port);
         BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
         StringBuilder data = new StringBuilder();
@@ -93,4 +70,25 @@ public class LogServerLogGrabber extends AbstractComponent {
         socket.close();
         return data.toString();
     }
+
+    private Optional<Integer> getErrorLogPort(ServiceInfo service) {
+        return service.getPorts().stream()
+                      .filter(port -> port.getTags().contains("last-errors-holder"))
+                      .map(PortInfo::getPort)
+                      .findFirst();
+    }
+
+    private class LogServerInfo {
+        String hostName;
+        int port;
+
+        LogServerInfo(String hostName, int port) {
+            this.hostName = hostName;
+            this.port = port;
+        }
+
+        public String toString() {
+            return hostName + ":" + port;
+        }
+    }
 }
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 b091ec29b75..c34cb565878 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
@@ -95,7 +95,7 @@ public class ApplicationHandlerTest {
                 provisioner,
                 new ApplicationConvergenceChecker(stateApiFactory),
                 mockHttpProxy,
-                new LogServerLogGrabber());
+                new MockLogServerLogGrabber());
         listApplicationsHandler = new ListApplicationsHandler(
                 Runnable::run, AccessLog.voidAccessLog(), tenants, Zone.defaultZone());
     }
@@ -219,6 +219,14 @@ public class ApplicationHandlerTest {
     }
 
     @Test
+    public void testGrabLog() throws Exception {
+        long sessionId = 1;
+        ApplicationId application = new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(mytenantName).build();
+        addMockApplication(tenants.getTenant(mytenantName), application, sessionId);
+        assertEquals("log line", grabLog(application, Zone.defaultZone()));
+    }
+
+    @Test
     public void testClusterControllerStatus() throws Exception {
         long sessionId = 1;
         ApplicationId application = new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(mytenantName).build();
@@ -386,6 +394,13 @@ public class ApplicationHandlerTest {
         HandlerTest.assertHttpStatusCodeAndMessage(response, 200, "");
     }
 
+    private String grabLog(ApplicationId application, Zone zone) throws IOException {
+        String restartUrl = toUrlPath(application, zone, true) + "/log";
+        HttpResponse response = mockHandler.handle(HttpRequest.createTestRequest(restartUrl, com.yahoo.jdisc.http.HttpRequest.Method.POST));
+        HandlerTest.assertHttpStatusCodeAndMessage(response, 200, "");
+        return SessionHandlerTest.getRenderedString(response);
+    }
+
     private static class MockStateApiFactory implements ApplicationConvergenceChecker.StateApiFactory {
         public boolean createdApi = false;
         @Override
@@ -400,4 +415,11 @@ public class ApplicationHandlerTest {
             };
         }
     }
+
+    private static class MockLogServerLogGrabber extends LogServerLogGrabber {
+        @Override
+        protected String readLog(String host, int port) throws IOException {
+            return "log line";
+        }
+    }
 }