Simplify CoreCollector, installPath never used

4 files changed, 18 insertions, 79 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java
index 735262a7fbb..b7856b2dd08 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java
@@ -1,4 +1,4 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
 import com.yahoo.collections.Pair;
@@ -12,7 +12,6 @@ import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
@@ -25,7 +24,7 @@ import static com.yahoo.vespa.defaults.Defaults.getDefaults;
  *
  * @author freva
  */
-public class CoreCollector {
+class CoreCollector {
     private static final String GDB_PATH = getDefaults().underVespaHome("bin64/gdb");
     private static final String LZ4_PATH = getDefaults().underVespaHome("bin64/lz4");
     private static final Pattern CORE_GENERATOR_PATH_PATTERN = Pattern.compile("^Core was generated by `(?<path>.*?)'.$");
@@ -36,28 +35,10 @@ public class CoreCollector {
     private static final Logger logger = Logger.getLogger(CoreCollector.class.getName());
     private final ProcessExecuter processExecuter;
 
-    public CoreCollector(ProcessExecuter processExecuter) {
+    CoreCollector(ProcessExecuter processExecuter) {
         this.processExecuter = processExecuter;
     }
 
-    List<String> readInstallState(Path installStatePath) throws IOException {
-        Pair<Integer, String> result = processExecuter.exec(new String[]{"cat", installStatePath.toString()});
-
-        if (result.getFirst() != 0) {
-            throw new RuntimeException("Failed to read install state file at: " + installStatePath + ", result: " + result);
-        }
-        return Arrays.asList(result.getSecond().split("\n"));
-    }
-
-    List<String> readRpmPackages() throws IOException {
-        Pair<Integer, String> result = processExecuter.exec(new String[]{"rpm", "-qa"});
-
-        if (result.getFirst() != 0) {
-            throw new RuntimeException("Failed to read RPM packages " + result);
-        }
-        return Arrays.asList(result.getSecond().split("\n"));
-    }
-
     Path readBinPathFallback(Path coredumpPath) throws IOException {
         String command = GDB_PATH + " -n -batch -core " + coredumpPath + " | grep \'^Core was generated by\'";
         String[] wrappedCommand = new String[] {"/bin/sh", "-c", command};
@@ -107,7 +88,7 @@ public class CoreCollector {
         return Arrays.asList(result.getSecond().split("\n"));
     }
 
-    Map<String, Object> collect(Path coredumpPath, Optional<Path> installStatePath) {
+    Map<String, Object> collect(Path coredumpPath) {
         Map<String, Object> data = new LinkedHashMap<>();
         try {
             coredumpPath = compressCoredump(coredumpPath);
@@ -125,20 +106,6 @@ public class CoreCollector {
             logger.log(Level.WARNING, "Failed to extract backtrace", e);
         }
 
-        installStatePath.ifPresent(installState -> {
-            try {
-                data.put("install_state", readInstallState(installState));
-            } catch (Exception e) {
-                logger.log(Level.WARNING, "Failed to read install state", e);
-            }
-
-            try {
-                data.put("rpm_packages", readRpmPackages());
-            } catch (Exception e) {
-                logger.log(Level.WARNING, "Failed to read RPM packages", e);
-            }
-        });
-
         try {
             deleteDecompressedCoredump(coredumpPath);
         } catch (IOException e) {
@@ -187,7 +154,6 @@ public class CoreCollector {
     }
 
     private boolean diskSpaceAvailable(Path path) throws IOException {
-        // TODO: If running inside container, check against container memory size, not for the enitre host
         String memInfo = new String(Files.readAllBytes(Paths.get("/proc/meminfo")));
         return path.toFile().getFreeSpace() > parseTotalMemorySize(memInfo);
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
index cc2d908a232..3de6427e20d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
@@ -1,4 +1,4 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -43,17 +43,15 @@ public class CoredumpHandler {
     private final Path coredumpsPath;
     private final Path doneCoredumpsPath;
     private final Map<String, Object> nodeAttributes;
-    private final Optional<Path> installStatePath;
     private final String feedEndpoint;
 
     public CoredumpHandler(HttpClient httpClient, CoreCollector coreCollector, Path coredumpsPath, Path doneCoredumpsPath,
-                           Map<String, Object> nodeAttributes, Optional<Path> installStatePath, String feedEndpoint) {
+                           Map<String, Object> nodeAttributes, String feedEndpoint) {
         this.httpClient = httpClient;
         this.coreCollector = coreCollector;
         this.coredumpsPath = coredumpsPath;
         this.doneCoredumpsPath = doneCoredumpsPath;
         this.nodeAttributes = nodeAttributes;
-        this.installStatePath = installStatePath;
         this.feedEndpoint = feedEndpoint;
     }
 
@@ -133,7 +131,7 @@ public class CoredumpHandler {
         if (!Files.exists(metadataPath)) {
             Path coredumpPath = FileHelper.listContentsOfDirectory(coredumpDirectory).stream().findFirst()
                     .orElseThrow(() -> new RuntimeException("No coredump file found in processing directory " + coredumpDirectory));
-            Map<String, Object> metadata = coreCollector.collect(coredumpPath, installStatePath);
+            Map<String, Object> metadata = coreCollector.collect(coredumpPath);
             metadata.putAll(nodeAttributes);
 
             Map<String, Object> fields = new HashMap<>();
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java
index 3899b6669d0..a3116af2aa5 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java
@@ -1,8 +1,7 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
 import com.yahoo.collections.Pair;
-import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 import com.yahoo.system.ProcessExecuter;
 import org.junit.Rule;
 import org.junit.Test;
@@ -18,10 +17,10 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -34,7 +33,7 @@ public class CoreCollectorTest {
     private final ProcessExecuter processExecuter = mock(ProcessExecuter.class);
     private final CoreCollector coreCollector = new CoreCollector(processExecuter);
 
-    private final Path INSTALL_STATE_PATH = Paths.get("/path/to/install.state");
+    private final String GDB_PATH = getDefaults().underVespaHome("bin64/gdb");
     private final Path TEST_CORE_PATH = Paths.get("/tmp/core.1234");
     private final Path TEST_BIN_PATH = Paths.get("/usr/bin/program");
     private final List<String> GDB_BACKTRACE = Arrays.asList("[New Thread 2703]",
@@ -42,16 +41,8 @@ public class CoreCollectorTest {
             "#0  0x00000000004004d8 in main (argv=0x1) at main.c:4", "4\t    printf(argv[3]);",
             "#0  0x00000000004004d8 in main (argv=0x1) at main.c:4");
 
-    private final List<String> INSTALL_STATE = Arrays.asList("package: some_package-0.0.2",
-            "variable 'value'",
-            "ca_file /path/to/ca.pem");
-
-    private final List<String> RPM_PACKAGES = Arrays.asList("some_package-0.0.2",
-            "another_package-1.0.6",
-            "last_pkg-3.10_102");
-
     @Rule
-    public TemporaryFolder folder= new TemporaryFolder();
+    public TemporaryFolder folder = new TemporaryFolder();
 
     private void mockExec(String[] cmd, String output) throws IOException {
         mockExec(cmd, output, "");
@@ -61,8 +52,6 @@ public class CoreCollectorTest {
         when(processExecuter.exec(cmd)).thenReturn(new Pair<>(error.isEmpty() ? 0 : 1, output + error));
     }
 
-    static final String GDB_PATH = getDefaults().underVespaHome("bin64/gdb");
-
     @Test
     public void extractsBinaryPathTest() throws IOException {
         final String[] cmd = {"file", TEST_CORE_PATH.toString()};
@@ -153,26 +142,12 @@ public class CoreCollectorTest {
         mockExec(new String[]{GDB_PATH, "-n", "-ex", "thread apply all bt", "-batch",
                         "/usr/bin/program", "/tmp/core.1234"},
                 String.join("\n", GDB_BACKTRACE));
-        mockExec(new String[]{"cat", INSTALL_STATE_PATH.toString()}, String.join("\n", INSTALL_STATE));
-        mockExec(new String[]{"rpm", "-qa"}, String.join("\n", RPM_PACKAGES));
 
         Map<String, Object> expectedData = new HashMap<>();
         expectedData.put("bin_path", TEST_BIN_PATH.toString());
         expectedData.put("backtrace", new ArrayList<>(GDB_BACKTRACE));
         expectedData.put("backtrace_all_threads", new ArrayList<>(GDB_BACKTRACE));
-        expectedData.put("install_state", new ArrayList<>(INSTALL_STATE));
-        expectedData.put("rpm_packages", new ArrayList<>(RPM_PACKAGES));
-        assertEquals(expectedData, coreCollector.collect(TEST_CORE_PATH, Optional.of(INSTALL_STATE_PATH)));
-    }
-
-    @Test
-    public void collectsPartialIfUnableToDetermineDumpingProgramTest() throws IOException {
-        // We fail to get backtrace and RPM packages, but install state works, make sure it is returned
-        mockExec(new String[]{"cat", INSTALL_STATE_PATH.toString()}, String.join("\n", INSTALL_STATE));
-
-        Map<String, Object> expectedData = new HashMap<>();
-        expectedData.put("install_state", new ArrayList<>(INSTALL_STATE));
-        assertEquals(expectedData, coreCollector.collect(TEST_CORE_PATH, Optional.of(INSTALL_STATE_PATH)));
+        assertEquals(expectedData, coreCollector.collect(TEST_CORE_PATH));
     }
 
     @Test
@@ -185,7 +160,7 @@ public class CoreCollectorTest {
 
         Map<String, Object> expectedData = new HashMap<>();
         expectedData.put("bin_path", TEST_BIN_PATH.toString());
-        assertEquals(expectedData, coreCollector.collect(TEST_CORE_PATH, Optional.empty()));
+        assertEquals(expectedData, coreCollector.collect(TEST_CORE_PATH));
     }
 
     @Test
@@ -219,7 +194,7 @@ public class CoreCollectorTest {
         expectedContentsOfCoredump.forEach(path -> {
             try {
                 path.toFile().createNewFile();
-            } catch (IOException ignored) { ignored.printStackTrace();}
+            } catch (IOException e) { e.printStackTrace();}
         });
         coreCollector.deleteDecompressedCoredump(coredumpPath.resolve(coreDumpFilename));
 
@@ -242,7 +217,7 @@ public class CoreCollectorTest {
         expectedContentsOfCoredump.forEach(path -> {
             try {
                 path.toFile().createNewFile();
-            } catch (IOException ignored) { ignored.printStackTrace();}
+            } catch (IOException e) { e.printStackTrace();}
         });
         coreCollector.deleteDecompressedCoredump(coredumpPath.resolve(coreDumpFilename));
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
index 2ac038e0f21..246ee0ad62b 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
@@ -1,4 +1,4 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
 import org.apache.http.HttpHeaders;
@@ -81,7 +81,7 @@ public class CoredumpHandlerTest {
         donePath = folder.newFolder("done").toPath();
 
         coredumpHandler = new CoredumpHandler(httpClient, coreCollector, crashPath, donePath, attributes,
-                Optional.empty(), feedEndpoint);
+                feedEndpoint);
     }
 
     @Test
@@ -146,7 +146,7 @@ public class CoredumpHandlerTest {
         Path processingPath = coredumpHandler.enqueueCoredumps();
         Path processingCoredumpPath = Files.list(processingPath).findFirst().orElseThrow(() ->
                 new RuntimeException("Expected to find directory with coredump in processing dir"));
-        when(coreCollector.collect(eq(processingCoredumpPath.resolve("core.dump")), any())).thenReturn(metadata);
+        when(coreCollector.collect(eq(processingCoredumpPath.resolve("core.dump")))).thenReturn(metadata);
 
         // Inside 'processing' directory, there should be a new directory containing 'core.dump' file
         String returnedMetadata = coredumpHandler.collectMetadata(processingCoredumpPath, attributes);