Merge pull request #5584 from vespa-engine/hmusum/remove-configserver-code-for-getting-errors-in-logs

Remove code for getting errors in logs from config server

5 files changed, 6 insertions, 165 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 3cef4b9de61..ad52de5eede 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
@@ -26,7 +26,6 @@ import com.yahoo.vespa.config.server.application.ApplicationConvergenceChecker;
 import com.yahoo.vespa.config.server.application.ApplicationSet;
 import com.yahoo.vespa.config.server.application.FileDistributionStatus;
 import com.yahoo.vespa.config.server.application.HttpProxy;
-import com.yahoo.vespa.config.server.application.LogServerLogGrabber;
 import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
 import com.yahoo.vespa.config.server.configchange.RefeedActions;
@@ -80,7 +79,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
 
     private final Tenants tenants;
     private final Optional<Provisioner> hostProvisioner;
-    private final LogServerLogGrabber logServerLogGrabber;
     private final ApplicationConvergenceChecker convergeChecker;
     private final HttpProxy httpProxy;
     private final Clock clock;
@@ -92,11 +90,10 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     @Inject
     public ApplicationRepository(Tenants tenants,
                                  HostProvisionerProvider hostProvisionerProvider,
-                                 LogServerLogGrabber logServerLogGrabber,
                                  ApplicationConvergenceChecker applicationConvergenceChecker,
                                  HttpProxy httpProxy, 
                                  ConfigserverConfig configserverConfig) {
-        this(tenants, hostProvisionerProvider.getHostProvisioner(), logServerLogGrabber,
+        this(tenants, hostProvisionerProvider.getHostProvisioner(),
              applicationConvergenceChecker, httpProxy, configserverConfig, Clock.systemUTC(), new FileDistributionStatus());
     }
 
@@ -104,14 +101,13 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     public ApplicationRepository(Tenants tenants,
                                  Provisioner hostProvisioner,
                                  Clock clock) {
-        this(tenants, Optional.of(hostProvisioner), new LogServerLogGrabber(),
+        this(tenants, Optional.of(hostProvisioner),
              new ApplicationConvergenceChecker(), new HttpProxy(new SimpleHttpFetcher()),
              new ConfigserverConfig(new ConfigserverConfig.Builder()), clock, new FileDistributionStatus());
     }
 
     private ApplicationRepository(Tenants tenants,
                                   Optional<Provisioner> hostProvisioner,
-                                  LogServerLogGrabber logServerLogGrabber,
                                   ApplicationConvergenceChecker applicationConvergenceChecker,
                                   HttpProxy httpProxy,
                                   ConfigserverConfig configserverConfig,
@@ -119,7 +115,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
                                   FileDistributionStatus fileDistributionStatus) {
         this.tenants = tenants;
         this.hostProvisioner = hostProvisioner;
-        this.logServerLogGrabber = logServerLogGrabber;
         this.convergeChecker = applicationConvergenceChecker;
         this.httpProxy = httpProxy;
         this.clock = clock;
@@ -217,11 +212,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return true;
     }
 
-    public String grabLog(Tenant tenant, ApplicationId applicationId) {
-        Application application = getApplication(tenant, applicationId);
-        return logServerLogGrabber.grabLog(application);
-    }
-
     public HttpResponse serviceConvergenceCheck(Tenant tenant, ApplicationId applicationId, String hostname, URI uri) {
         Application application = getApplication(tenant, applicationId);
         return convergeChecker.serviceConvergenceCheck(application, hostname, uri);
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
deleted file mode 100644
index b29953187e5..00000000000
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/LogServerLogGrabber.java
+++ /dev/null
@@ -1,94 +0,0 @@
-// Copyright 2017 Yahoo Holdings. 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.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;
-
-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 returns
- * a log entry once over this API so doing repeated calls will not give the same results.
- *
- * @author dybis
- */
-public class LogServerLogGrabber extends AbstractComponent {
-    private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(LogServerLogGrabber.class.getName());
-
-    public LogServerLogGrabber() {}
-
-    public String grabLog(Application application) {
-        LogServerInfo logServerConnectionInfo = findLogserverConnectionInfo(application);
-        log.log(LogLevel.DEBUG, "Requested error logs, pulling from logserver on " + logServerConnectionInfo);
-        try {
-            return readLog(logServerConnectionInfo.hostName, logServerConnectionInfo.port);
-        } catch (IOException e) {
-            throw new InternalServerException(Exceptions.toMessageString(e));
-        }
-    }
-
-    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)));
-                                        }));
-
-        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");
-
-        return logServerConnectionInfos.get(0);
-    }
-
-    // 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();
-
-        int bufferSize = 4096;
-        int charsRead;
-        do {
-            char[] buffer = new char[bufferSize];
-            charsRead = in.read(buffer);
-            data.append(new String(buffer, 0, charsRead));
-        } while (charsRead == bufferSize);
-        in.close();
-        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/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 7be7bfb47e1..42fdb16c7ca 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
@@ -16,16 +16,12 @@ import com.yahoo.jdisc.application.BindingMatch;
 import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.config.server.http.ContentHandler;
 import com.yahoo.vespa.config.server.http.ContentRequest;
-import com.yahoo.vespa.config.server.http.HttpConfigResponse;
 import com.yahoo.vespa.config.server.http.HttpErrorResponse;
 import com.yahoo.vespa.config.server.http.HttpHandler;
 import com.yahoo.vespa.config.server.http.JSONResponse;
 import com.yahoo.vespa.config.server.http.NotFoundException;
 import com.yahoo.vespa.config.server.tenant.Tenant;
 
-import java.io.IOException;
-import java.io.OutputStream;
-import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 
 /**
@@ -104,12 +100,9 @@ public class ApplicationHandler extends HttpHandler {
     @Override
     public HttpResponse handlePOST(HttpRequest request) {
         ApplicationId applicationId = getApplicationIdFromRequest(request);
-        Tenant tenant = verifyTenantAndApplication(applicationId);
         if (request.getUri().getPath().endsWith("restart"))
             return restart(request, applicationId);
-        if (request.getUri().getPath().endsWith("log"))
-            return grabLog(request, applicationId, tenant);
-        throw new NotFoundException("Illegal POST request '" + request.getUri() + "': Must end by /restart or /log");
+        throw new NotFoundException("Illegal POST request '" + request.getUri() + "': Must end with /restart");
     }
 
     private HttpResponse restart(HttpRequest request, ApplicationId applicationId) {
@@ -120,24 +113,6 @@ public class ApplicationHandler extends HttpHandler {
         return new JSONResponse(Response.Status.OK); // return empty
     }
 
-    private HttpResponse grabLog(HttpRequest request, ApplicationId applicationId, Tenant tenant) {
-        if (getBindingMatch(request).groupCount() != 7)
-            throw new NotFoundException("Illegal POST log request '" + request.getUri() +
-                    "': Must have 6 arguments but had " + ( getBindingMatch(request).groupCount()-1 ) );
-        final String response = applicationRepository.grabLog(tenant, applicationId);
-        return new HttpResponse(200) {
-            @Override
-            public void render(OutputStream outputStream) throws IOException {
-                outputStream.write(response.getBytes(StandardCharsets.UTF_8));
-            }
-
-            @Override
-            public String getContentType() {
-                return HttpConfigResponse.JSON_CONTENT_TYPE;
-            }
-        };
-    }
-
     private HostFilter hostFilterFrom(HttpRequest request) {
         return HostFilter.from(request.getProperty("hostname"),
                                request.getProperty("flavor"),
@@ -157,7 +132,6 @@ public class ApplicationHandler extends HttpHandler {
         return HttpConfigRequests.getBindingMatch(request,
                 // WARNING: UPDATE src/main/resources/configserver-app/services.xml IF YOU MAKE ANY CHANGES TO THESE BINDINGS!
                 "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/content/*",
-                "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/log",
                 "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/filedistributionstatus",
                 "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/restart",
                 "http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/serviceconverge",
diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml
index f5b503e54d8..9419a09ce7e 100644
--- a/configserver/src/main/resources/configserver-app/services.xml
+++ b/configserver/src/main/resources/configserver-app/services.xml
@@ -129,8 +129,6 @@
       <binding>https://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/filedistributionstatus</binding>
       <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/restart</binding>
       <binding>https://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/restart</binding>
-      <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/log</binding>
-      <binding>https://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/log</binding>
       <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/converge</binding>
       <binding>https://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/converge</binding>
       <binding>http://*/application/v2/tenant/*/application/*/environment/*/region/*/instance/*/serviceconverge</binding>
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 ef53baf821d..9a69dfc5f07 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
@@ -22,7 +22,6 @@ import com.yahoo.vespa.config.server.SuperModelGenerationCounter;
 import com.yahoo.vespa.config.server.TestComponentRegistry;
 import com.yahoo.vespa.config.server.application.ApplicationConvergenceChecker;
 import com.yahoo.vespa.config.server.application.HttpProxy;
-import com.yahoo.vespa.config.server.application.LogServerLogGrabber;
 import com.yahoo.vespa.config.server.http.HandlerTest;
 import com.yahoo.vespa.config.server.http.HttpErrorResponse;
 import com.yahoo.vespa.config.server.http.StaticResponse;
@@ -90,8 +89,7 @@ public class ApplicationHandlerTest {
         mockHandler = createMockApplicationHandler(
                 provisioner,
                 new ApplicationConvergenceChecker(stateApiFactory),
-                mockHttpProxy,
-                new MockLogServerLogGrabber());
+                mockHttpProxy);
         listApplicationsHandler = new ListApplicationsHandler(
                 ListApplicationsHandler.testOnlyContext(),
                 tenants, Zone.defaultZone());
@@ -100,14 +98,12 @@ public class ApplicationHandlerTest {
     private ApplicationHandler createMockApplicationHandler(
             Provisioner provisioner,
             ApplicationConvergenceChecker convergeChecker,
-            HttpProxy httpProxy,
-            LogServerLogGrabber logServerLogGrabber) {
+            HttpProxy httpProxy) {
         return new ApplicationHandler(
                 ApplicationHandler.testOnlyContext(),
                 Zone.defaultZone(),
                 new ApplicationRepository(tenants,
                                           HostProvisionerProvider.withProvisioner(provisioner),
-                                          logServerLogGrabber,
                                           convergeChecker,
                                           httpProxy,
                                           new ConfigserverConfig(new ConfigserverConfig.Builder())));
@@ -119,7 +115,6 @@ public class ApplicationHandlerTest {
                 Zone.defaultZone(),
                 new ApplicationRepository(tenants,
                                           HostProvisionerProvider.withProvisioner(provisioner),
-                                          new LogServerLogGrabber(),
                                           new ApplicationConvergenceChecker(stateApiFactory),
                                           new HttpProxy(new SimpleHttpFetcher()),
                                           new ConfigserverConfig(new ConfigserverConfig.Builder())));
@@ -215,14 +210,6 @@ 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, Clock.systemUTC());
-        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();
@@ -249,8 +236,7 @@ public class ApplicationHandlerTest {
         mockHandler = createMockApplicationHandler(
                 provisioner,
                 new ApplicationConvergenceChecker(stateApiFactory),
-                new HttpProxy(new SimpleHttpFetcher()),
-                new LogServerLogGrabber());
+                new HttpProxy(new SimpleHttpFetcher()));
         final ApplicationId applicationId = ApplicationId.defaultId();
         addMockApplication(tenants.getTenant(mytenantName), applicationId, 1, Clock.systemUTC());
         assertApplicationExists(mytenantName, applicationId, Zone.defaultZone());
@@ -405,13 +391,6 @@ 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 HttpResponse fileDistributionStatus(ApplicationId application, Zone zone) {
         String restartUrl = toUrlPath(application, zone, true) + "/filedistributionstatus";
         return mockHandler.handle(HttpRequest.createTestRequest(restartUrl, com.yahoo.jdisc.http.HttpRequest.Method.GET));
@@ -432,10 +411,4 @@ public class ApplicationHandlerTest {
         }
     }
 
-    private static class MockLogServerLogGrabber extends LogServerLogGrabber {
-        @Override
-        protected String readLog(String host, int port) throws IOException {
-            return "log line";
-        }
-    }
 }