Merge pull request #11193 from vespa-engine/hakonhall/define-the-once-node-report-type

Define the ONCE node report type

5 files changed, 137 insertions, 47 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeReports.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeReports.java
index 72c7885d8c1..70ce548916a 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeReports.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeReports.java
@@ -3,11 +3,14 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.reports.BaseReport;
 
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.stream.Collectors;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
 
@@ -43,6 +46,25 @@ public class NodeReports {
         return Optional.ofNullable(reports.get(reportId)).map(r -> uncheck(() -> mapper.treeToValue(r, jacksonClass)));
     }
 
+    /** Gets all reports of the given types and deserialize with the given jacksonClass. */
+    public <T> TreeMap<String, T> getReports(Class<T> jacksonClass, BaseReport.Type... types) {
+        Set<BaseReport.Type> typeSet = Set.of(types);
+
+        return reports.entrySet().stream()
+                .filter(entry -> {
+                    JsonNode reportType = entry.getValue().findValue(BaseReport.TYPE_FIELD);
+                    if (reportType == null || !reportType.isTextual()) return false;
+                    Optional<BaseReport.Type> type = BaseReport.Type.deserialize(reportType.asText());
+                    return type.map(typeSet::contains).orElse(false);
+                })
+                .collect(Collectors.toMap(
+                        entry -> entry.getKey(),
+                        entry -> uncheck(() -> mapper.treeToValue(entry.getValue(), jacksonClass)),
+                        (x,y) -> x, // resolves key collisions - cannot happen.
+                        TreeMap::new
+                ));
+    }
+
     public void removeReport(String reportId) {
         if (reports.containsKey(reportId)) {
             reports.put(reportId, null);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReport.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReport.java
index eac5f7300ef..1da180226b2 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReport.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReport.java
@@ -12,6 +12,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalLong;
+import java.util.stream.Stream;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
 
@@ -23,10 +24,10 @@ import static com.yahoo.yolean.Exceptions.uncheck;
  * <p><strong>Subclass requirements</strong>
  *
  * <ol>
- *     <li>A subclass mush be a Jackson class that can be mapped to {@link JsonNode} with {@link #toJsonNode()},
+ *     <li>A subclass must be a Jackson class that can be mapped to {@link JsonNode} with {@link #toJsonNode()},
  *     and from {@link JsonNode} with {@link #fromJsonNode(JsonNode, Class)}.</li>
- *     <li>A subclass must override {@link #updates(BaseReport)} and make sure to return false if
- *     {@code !super.updates(current)}.</li>
+ *     <li>A subclass must override {@link #updates(BaseReport)} and make sure to return true if
+ *     {@code super.updates(current)}.</li>
  * </ol>
  *
  * @author hakonhall
@@ -51,10 +52,18 @@ public class BaseReport {
     public enum Type {
         /** The default type if none given, or not recognized. */
         UNSPECIFIED,
+        /** A program to be executed once. */
+        ONCE,
         /** The host has a soft failure and should be parked for manual inspection. */
         SOFT_FAIL,
         /** The host has a hard failure and should be given back to siteops. */
-        HARD_FAIL
+        HARD_FAIL;
+
+        public static Optional<Type> deserialize(String typeString) {
+            return Stream.of(Type.values()).filter(type -> type.name().equalsIgnoreCase(typeString)).findAny();
+        }
+
+        public String serialize() { return name(); }
     }
 
     @JsonCreator
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java
index cbc8ffbf1b7..bc88702b0fc 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessException.java
@@ -2,6 +2,8 @@
 
 package com.yahoo.vespa.hosted.node.admin.task.util.process;
 
+import com.yahoo.vespa.hosted.node.admin.task.util.text.SnippetGenerator;
+
 /**
  * Base class for child process related exceptions, with a util to build an error message
  * that includes a large part of the output.
@@ -10,10 +12,7 @@ package com.yahoo.vespa.hosted.node.admin.task.util.process;
  */
 @SuppressWarnings("serial")
 public abstract class ChildProcessException extends RuntimeException {
-    private static final int MAX_OUTPUT_PREFIX = 200;
-    private static final int MAX_OUTPUT_SUFFIX = 200;
-    // Omitting a number of chars less than 10 or less than 10% would be ridiculous.
-    private static final int MAX_OUTPUT_SLACK = Math.max(10, (10 * (MAX_OUTPUT_PREFIX + MAX_OUTPUT_SUFFIX)) / 100);
+    private static final SnippetGenerator snippetGenerator = new SnippetGenerator();
 
     /**
      * An exception with a message of the following format:
@@ -36,44 +35,13 @@ public abstract class ChildProcessException extends RuntimeException {
         super(makeSnippet(problem, commandLine, possiblyHugeOutput), cause);
     }
 
-    private static String makeSnippet(String problem,
-                               String commandLine,
-                               String possiblyHugeOutput) {
-        return makeSnippet(
-                problem,
-                commandLine,
-                possiblyHugeOutput,
-                MAX_OUTPUT_PREFIX,
-                MAX_OUTPUT_SUFFIX,
-                MAX_OUTPUT_SLACK);
-    }
-
-    // Package-private instead of private for testing.
-    static String makeSnippet(String problem,
-                              String commandLine,
-                              String possiblyHugeOutput,
-                              int maxOutputPrefix,
-                              int maxOutputSuffix,
-                              int maxOutputSlack) {
-        StringBuilder stringBuilder = new StringBuilder()
-                .append("Command '")
-                .append(commandLine)
-                .append("' ")
-                .append(problem)
-                .append(": stdout/stderr: '");
-
-        if (possiblyHugeOutput.length() <= maxOutputPrefix + maxOutputSuffix + maxOutputSlack) {
-            stringBuilder.append(possiblyHugeOutput);
-        } else {
-            stringBuilder.append(possiblyHugeOutput, 0, maxOutputPrefix)
-                    .append("... [")
-                    .append(possiblyHugeOutput.length() - maxOutputPrefix - maxOutputSuffix)
-                    .append(" chars omitted] ...")
-                    .append(possiblyHugeOutput.substring(possiblyHugeOutput.length() - maxOutputSuffix));
-        }
-
-        stringBuilder.append("'");
-
-        return stringBuilder.toString();
+    private static String makeSnippet(String problem, String commandLine, String possiblyHugeOutput) {
+        return "Command '" +
+                commandLine +
+                "' " +
+                problem +
+                ": stdout/stderr: '" +
+                snippetGenerator.makeSnippet(possiblyHugeOutput, 500) +
+                "'";
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGenerator.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGenerator.java
new file mode 100644
index 00000000000..7694260d1f7
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGenerator.java
@@ -0,0 +1,32 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.node.admin.task.util.text;
+
+/**
+ * @author hakon
+ */
+public class SnippetGenerator {
+
+    private static final String OMIT_PREFIX = "[...";
+    private static final String OMIT_SUFFIX = " chars omitted]";
+    private static final int ASSUMED_OMIT_TEXT_LENGTH = OMIT_PREFIX.length() + 4 + OMIT_SUFFIX.length();
+
+    /** Returns a snippet of approximate size. */
+    public String makeSnippet(String text, int sizeHint) {
+        if (text.length() <= Math.max(sizeHint, ASSUMED_OMIT_TEXT_LENGTH)) return text;
+
+        int maxSuffixLength = Math.max(0, (sizeHint - ASSUMED_OMIT_TEXT_LENGTH) / 2);
+        int maxPrefixLength = Math.max(0, sizeHint - ASSUMED_OMIT_TEXT_LENGTH - maxSuffixLength);
+        String sizeString = Integer.toString(text.length() - maxPrefixLength - maxSuffixLength);
+
+        // It would be silly to return a snippet when the full text is barely longer.
+        // Note: Say ASSUMED_OMIT_TEXT_LENGTH=23: text will be returned whenever sizeHint<23 and text.length()<28.
+        int snippetLength = maxPrefixLength + OMIT_PREFIX.length() + sizeString.length() + OMIT_SUFFIX.length() + maxSuffixLength;
+        if (text.length() <= 1.05 * snippetLength + 5) return text;
+
+        return text.substring(0, maxPrefixLength) +
+                OMIT_PREFIX +
+                sizeString +
+                OMIT_SUFFIX +
+                text.substring(text.length() - maxSuffixLength);
+    }
+}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGeneratorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGeneratorTest.java
new file mode 100644
index 00000000000..8a67052a286
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/text/SnippetGeneratorTest.java
@@ -0,0 +1,59 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.node.admin.task.util.text;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author hakon
+ */
+public class SnippetGeneratorTest {
+    private final SnippetGenerator generator = new SnippetGenerator();
+
+    private void assertSnippet(String text, int sizeHint, String expectedSnippet) {
+        assertEquals(expectedSnippet, generator.makeSnippet(text, sizeHint));
+    }
+
+    @Test
+    public void prefixSnippetForReallySmallSizeHint() {
+        assertSnippet(
+                "This is a long text that should be snippeted", 0,
+                "[...44 chars omitted]");
+
+        assertSnippet(
+                "This is a long text that should be snippeted", 1,
+                "[...44 chars omitted]");
+    }
+
+    @Test
+    public void snippet() {
+        assertSnippet(
+                "This is a long text that should be snippeted", 23,
+                "[...44 chars omitted]");
+
+        assertSnippet(
+                "This is a long text that should be snippeted", 24,
+                "T[...43 chars omitted]");
+
+        assertSnippet(
+                "This is a long text that should be snippeted", 30,
+                "This[...37 chars omitted]ted");
+
+    }
+
+    @Test
+    public void noShorteningNeeded() {
+        assertSnippet(
+                "This is a long text that should be snippeted", 39,
+                "This is [...28 chars omitted]nippeted");
+
+        assertSnippet(
+                "This is a long text that should be snippeted", 40,
+                "This is a long text that should be snippeted");
+
+        assertSnippet(
+                "This is a long text that should be snippeted", 50,
+                "This is a long text that should be snippeted");
+    }
+}
\ No newline at end of file