Type field on node reports

3 files changed, 89 insertions, 23 deletions

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 42804b69b17..5c0e69dadb3 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
@@ -11,6 +11,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 
 import java.util.Objects;
 import java.util.Optional;
+import java.util.OptionalLong;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
 
@@ -28,9 +29,10 @@ import static com.yahoo.yolean.Exceptions.uncheck;
  *     {@code !super.updates(current)}.</li>
  * </ol>
  *
+ * <p>NOT immutable to allow e.g. type to be overridden before serialization.</p>
+ *
  * @author hakonhall
  */
-// @Immutable
 @JsonIgnoreProperties(ignoreUnknown = true)
 @JsonInclude(JsonInclude.Include.NON_NULL)
 public class BaseReport {
@@ -38,28 +40,60 @@ public class BaseReport {
     public static final String CREATED_FIELD = "createdMillis";
     /** The description of the error (implies wanting to fail out node). */
     public static final String DESCRIPTION_FIELD = "description";
+    /** The type of report, see {@link Type} enum. */
+    public static final String TYPE_FIELD = "type";
 
     protected static final ObjectMapper mapper = new ObjectMapper();
 
-    private final Long createdMillis;
-    private final String description;
+    private final OptionalLong createdMillis;
+    private final Optional<String> description;
+
+    private Type type;
+
+    public enum Type {
+        /** The default type if none given, or not recognized. */
+        UNSPECIFIED,
+        /** The host has a soft failure and should parked for manual inspection. */
+        SOFT_FAIL,
+        /** The host has a hard failure and should be given back to siteops. */
+        HARD_FAIL
+    }
 
     @JsonCreator
     public BaseReport(@JsonProperty(CREATED_FIELD) Long createdMillisOrNull,
-                      @JsonProperty(DESCRIPTION_FIELD) String descriptionOrNull) {
-        this.createdMillis = createdMillisOrNull;
-        this.description = descriptionOrNull;
+                      @JsonProperty(DESCRIPTION_FIELD) String descriptionOrNull,
+                      @JsonProperty(TYPE_FIELD) Type typeOrNull) {
+        this.createdMillis = createdMillisOrNull == null ? OptionalLong.empty() : OptionalLong.of(createdMillisOrNull);
+        this.description = Optional.ofNullable(descriptionOrNull);
+        this.type = typeOrNull == null ? Type.UNSPECIFIED : typeOrNull;
+    }
 
+    public BaseReport(Long createdMillisOrNull, String descriptionOrNull) {
+        this(createdMillisOrNull, descriptionOrNull, Type.UNSPECIFIED);
     }
 
     @JsonGetter(CREATED_FIELD)
     public final Long getCreatedMillisOrNull() {
-        return createdMillis;
+        return createdMillis.isPresent() ? createdMillis.getAsLong() : null;
     }
 
     @JsonGetter(DESCRIPTION_FIELD)
     public final String getDescriptionOrNull() {
-        return description;
+        return description.orElse(null);
+    }
+
+    /** null is returned on UNSPECIFIED to avoid noisy reports. */
+    @JsonGetter(TYPE_FIELD)
+    public final Type getTypeOrNull() {
+        return type == Type.UNSPECIFIED ? null : type;
+    }
+
+    public Type getType() {
+        return type;
+    }
+
+    public void setType(Type typeOrNull) {
+        this.type = typeOrNull;
     }
 
     /**
@@ -71,7 +105,8 @@ public class BaseReport {
     public boolean updates(BaseReport current) {
         if (this == current) return false;
         if (this.getClass() != current.getClass()) return true;
-        return !Objects.equals(description, current.description);
+        return !Objects.equals(description, current.description) ||
+                !Objects.equals(type, current.type);
     }
 
     /** A variant of {@link #updates(BaseReport)} handling possibly absent reports, whether new or old. */
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNodeTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNodeTest.java
index ded6d4fbf61..e1e7ea240b0 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNodeTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNodeTest.java
@@ -53,7 +53,7 @@ public class NodeRepositoryNodeTest {
         node.reports.put("ridTwo", reportJson);
 
         // Add ridTwo report to attributes
-        BaseReport reportTwo = new BaseReport(3L, "desc");
+        BaseReport reportTwo = new BaseReport(3L, "desc", null);
         attributes.withReport("ridTwo", reportTwo.toJsonNode());
 
         // Verify node serializes to expected, as well as attributes patched on node.
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReportTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReportTest.java
index 561733d7d76..4d37f40a2e9 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReportTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/reports/BaseReportTest.java
@@ -4,6 +4,8 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository.reports;
 import com.yahoo.test.json.JsonTestHelper;
 import org.junit.Test;
 
+import static com.yahoo.vespa.hosted.node.admin.configserver.noderepository.reports.BaseReport.Type.UNSPECIFIED;
+import static com.yahoo.vespa.hosted.node.admin.configserver.noderepository.reports.BaseReport.Type.SOFT_FAIL;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -13,35 +15,64 @@ import static org.junit.Assert.assertTrue;
  */
 public class BaseReportTest {
     private static final String JSON_1 = "{\"createdMillis\": 1, \"description\": \"desc\"}";
+    private static final String JSON_2 = "{\"createdMillis\": 1, \"description\": \"desc\", \"type\": \"SOFT_FAIL\"}";
+
     @Test
-    public void testSerialization() {
-        JsonTestHelper.assertJsonEquals(new BaseReport(1L, "desc").toJsonNode(),
+    public void testSerialization1() {
+        JsonTestHelper.assertJsonEquals(new BaseReport(1L, "desc", SOFT_FAIL).toJsonNode(),
+                JSON_2);
+        JsonTestHelper.assertJsonEquals(new BaseReport(null, "desc", SOFT_FAIL).toJsonNode(),
+                "{\"description\": \"desc\", \"type\": \"SOFT_FAIL\"}");
+        JsonTestHelper.assertJsonEquals(new BaseReport(1L, null, SOFT_FAIL).toJsonNode(),
+                "{\"createdMillis\": 1, \"type\": \"SOFT_FAIL\"}");
+        JsonTestHelper.assertJsonEquals(new BaseReport(null, null, SOFT_FAIL).toJsonNode(),
+                "{\"type\": \"SOFT_FAIL\"}");
+
+        JsonTestHelper.assertJsonEquals(new BaseReport(1L, "desc", null).toJsonNode(),
                 JSON_1);
-        JsonTestHelper.assertJsonEquals(new BaseReport(null, "desc").toJsonNode(),
+        JsonTestHelper.assertJsonEquals(new BaseReport(null, "desc", null).toJsonNode(),
                 "{\"description\": \"desc\"}");
-        JsonTestHelper.assertJsonEquals(new BaseReport(1L, null).toJsonNode(),
+        JsonTestHelper.assertJsonEquals(new BaseReport(1L, null, null).toJsonNode(),
                 "{\"createdMillis\": 1}");
-        JsonTestHelper.assertJsonEquals(new BaseReport(null, null).toJsonNode(),
+        JsonTestHelper.assertJsonEquals(new BaseReport(null, null, null).toJsonNode(),
                 "{}");
     }
 
     @Test
     public void testShouldUpdate() {
-        BaseReport report = new BaseReport(1L, "desc");
+        BaseReport report = new BaseReport(1L, "desc", SOFT_FAIL);
         assertFalse(report.updates(report));
-        assertFalse(new BaseReport(1L, "desc").updates(report));
-        assertFalse(new BaseReport(2L, "desc").updates(report));
-        assertFalse(new BaseReport(null, "desc").updates(report));
 
-        assertTrue(new BaseReport(1L, "desc 2").updates(report));
-        assertTrue(new BaseReport(1L, null).updates(report));
+        // createdMillis is ignored
+        assertFalse(new BaseReport(1L, "desc", SOFT_FAIL).updates(report));
+        assertFalse(new BaseReport(2L, "desc", SOFT_FAIL).updates(report));
+        assertFalse(new BaseReport(null, "desc", SOFT_FAIL).updates(report));
+
+        // description is not ignored
+        assertTrue(new BaseReport(1L, "desc 2", SOFT_FAIL).updates(report));
+        assertTrue(new BaseReport(1L, null, SOFT_FAIL).updates(report));
+
+        // type is not ignored
+        assertTrue(new BaseReport(1L, "desc", null).updates(report));
+        assertTrue(new BaseReport(1L, "desc", BaseReport.Type.HARD_FAIL).updates(report));
     }
 
     @Test
     public void testJsonSerialization() {
-        BaseReport report = BaseReport.fromJson(JSON_1);
+        BaseReport report = BaseReport.fromJson(JSON_2);
         assertEquals(1L, (long) report.getCreatedMillisOrNull());
         assertEquals("desc", report.getDescriptionOrNull());
-        JsonTestHelper.assertJsonEquals(report.toJson(), JSON_1);
+        assertEquals(SOFT_FAIL, report.getTypeOrNull());
+        JsonTestHelper.assertJsonEquals(report.toJson(), JSON_2);
+    }
+
+    @Test
+    public void settingInformationalIgnoresType() {
+        BaseReport report = BaseReport.fromJson(JSON_2);
+        assertEquals(BaseReport.Type.SOFT_FAIL, report.getTypeOrNull());
+        report.setType(UNSPECIFIED);
+        JsonTestHelper.assertJsonEquals(JSON_1, report.toJson());
+        report.setType(null);
+        JsonTestHelper.assertJsonEquals(JSON_1, report.toJson());
     }
 }
\ No newline at end of file