Patching and retrieving node reports in host admin

 - A report can be patched to the node repository by using NodeAttributes and
   NodeRepository.updateNodeAttributes().
 - A report can be retrieved as a Jackson class from NodeSpec and its
   NodeReports.

10 files changed, 347 insertions, 13 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java
index 3766402defa..28e812e6ca1 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java
@@ -1,14 +1,24 @@
 // Copyright 2017 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.configserver.noderepository;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 
 import java.time.Instant;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.TreeMap;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+/**
+ * A node in the node repository is modified by setting which attributes to modify in this class,
+ * and then patching the node repository node through {@link NodeRepository#updateNodeAttributes(String, NodeAttributes)}.
+ *
+ * @author Haakon Dybdahl
+ * @author Valerij Fredriksen
+ */
 public class NodeAttributes {
 
     private Optional<Long> restartGeneration = Optional.empty();
@@ -20,6 +30,8 @@ public class NodeAttributes {
     private Optional<String> hardwareDivergence = Optional.empty();
     private Optional<String> hardwareFailureDescription = Optional.empty();
     private Optional<Boolean> wantToDeprovision = Optional.empty();
+    /** The list of reports to patch. A null value is used to remove the report. */
+    private Map<String, JsonNode> reports = new TreeMap<>();
 
     public NodeAttributes() { }
 
@@ -72,6 +84,20 @@ public class NodeAttributes {
         return this;
     }
 
+    public NodeAttributes withReports(Map<String, JsonNode> nodeReports) {
+        this.reports = new TreeMap<>(nodeReports);
+        return this;
+    }
+
+    public NodeAttributes withReport(String reportId, JsonNode jsonNode) {
+        reports.put(reportId, jsonNode);
+        return this;
+    }
+
+    public NodeAttributes withReportRemoved(String reportId) {
+        reports.put(reportId, null);
+        return this;
+    }
 
     public Optional<Long> getRestartGeneration() {
         return restartGeneration;
@@ -109,10 +135,14 @@ public class NodeAttributes {
         return wantToDeprovision;
     }
 
+    public Map<String, JsonNode> getReports() {
+        return reports;
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(restartGeneration, rebootGeneration, dockerImage, vespaVersion, currentOsVersion,
-                currentFirmwareCheck, hardwareDivergence, hardwareFailureDescription, wantToDeprovision);
+                currentFirmwareCheck, hardwareDivergence, hardwareFailureDescription, wantToDeprovision, reports);
     }
 
     @Override
@@ -130,6 +160,7 @@ public class NodeAttributes {
                 && Objects.equals(currentFirmwareCheck, other.currentFirmwareCheck)
                 && Objects.equals(hardwareDivergence, other.hardwareDivergence)
                 && Objects.equals(hardwareFailureDescription, other.hardwareFailureDescription)
+                && Objects.equals(reports, other.reports)
                 && Objects.equals(wantToDeprovision, other.wantToDeprovision);
     }
 
@@ -144,6 +175,7 @@ public class NodeAttributes {
                         currentFirmwareCheck.map(at -> "currentFirmwareCheck=" + at),
                         hardwareDivergence.map(hwDivg -> "hardwareDivergence=" + hwDivg),
                         hardwareFailureDescription.map(hwDesc -> "hardwareFailureDescription=" + hwDesc),
+                        Optional.ofNullable(reports.isEmpty() ? null : "reports=" + reports),
                         wantToDeprovision.map(depr -> "wantToDeprovision=" + depr))
                 .filter(Optional::isPresent)
                 .map(Optional::get)
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
new file mode 100644
index 00000000000..09fe3298619
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeReports.java
@@ -0,0 +1,54 @@
+// 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.configserver.noderepository;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.util.Map;
+import java.util.Optional;
+import java.util.TreeMap;
+
+import static com.yahoo.yolean.Exceptions.uncheck;
+
+/**
+ * API of node reports within node-admin.
+ *
+ * @author hakonhall
+ */
+public class NodeReports {
+    private static final ObjectMapper mapper = new ObjectMapper();
+
+    private final Map<String, JsonNode> reports = new TreeMap<>();
+
+    public NodeReports() { }
+
+    public NodeReports(NodeReports reports) {
+        this.reports.putAll(reports.reports);
+    }
+
+    private NodeReports(Map<String, JsonNode> reports) {
+        this.reports.putAll(reports);
+    }
+
+    public static NodeReports fromMap(Optional<Map<String, JsonNode>> reports) {
+        return reports.map(NodeReports::new).orElseGet(NodeReports::new);
+    }
+
+    public void setReport(String reportId, JsonNode jsonNode) {
+        reports.put(reportId, jsonNode);
+    }
+
+    public <T> Optional<T> getReport(String reportId, Class<T> jacksonClass) {
+        return Optional.ofNullable(reports.get(reportId)).map(r -> uncheck(() -> mapper.treeToValue(r, jacksonClass)));
+    }
+
+    public void removeReport(String reportId) {
+        if (reports.containsKey(reportId)) {
+            reports.put(reportId, null);
+        }
+    }
+
+    public Map<String, JsonNode> getRawMap() {
+        return new TreeMap<>(reports);
+    }
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
index 225929db4bd..c9a1e90935a 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
@@ -1,6 +1,7 @@
 // Copyright 2017 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.configserver.noderepository;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
@@ -55,6 +56,8 @@ public class NodeSpec {
 
     private final Optional<String> hardwareDivergence;
     private final Optional<String> hardwareFailureDescription;
+    private final NodeReports reports;
+
     private final Optional<String> parentHostname;
 
     public NodeSpec(
@@ -87,6 +90,7 @@ public class NodeSpec {
             Set<String> ipAddresses,
             Optional<String> hardwareDivergence,
             Optional<String> hardwareFailureDescription,
+            NodeReports reports,
             Optional<String> parentHostname) {
         this.hostname = Objects.requireNonNull(hostname);
         this.wantedDockerImage = Objects.requireNonNull(wantedDockerImage);
@@ -117,6 +121,7 @@ public class NodeSpec {
         this.ipAddresses = Objects.requireNonNull(ipAddresses);
         this.hardwareDivergence = Objects.requireNonNull(hardwareDivergence);
         this.hardwareFailureDescription = Objects.requireNonNull(hardwareFailureDescription);
+        this.reports = reports;
         this.parentHostname = Objects.requireNonNull(parentHostname);
     }
 
@@ -236,6 +241,8 @@ public class NodeSpec {
         return hardwareFailureDescription;
     }
 
+    public NodeReports getReports() { return reports; }
+
     public Optional<String> getParentHostname() {
         return parentHostname;
     }
@@ -276,6 +283,7 @@ public class NodeSpec {
                 Objects.equals(ipAddresses, that.ipAddresses) &&
                 Objects.equals(hardwareDivergence, that.hardwareDivergence) &&
                 Objects.equals(hardwareFailureDescription, that.hardwareFailureDescription) &&
+                Objects.equals(reports, that.reports) &&
                 Objects.equals(parentHostname, that.parentHostname);
     }
 
@@ -311,6 +319,7 @@ public class NodeSpec {
                 ipAddresses,
                 hardwareDivergence,
                 hardwareFailureDescription,
+                reports,
                 parentHostname);
     }
 
@@ -346,6 +355,7 @@ public class NodeSpec {
                 + " ipAddresses=" + ipAddresses
                 + " hardwareDivergence=" + hardwareDivergence
                 + " hardwareFailureDescription=" + hardwareFailureDescription
+                + " reports=" + reports
                 + " parentHostname=" + parentHostname
                 + " }";
     }
@@ -509,6 +519,7 @@ public class NodeSpec {
         private Set<String> ipAddresses = Collections.emptySet();
         private Optional<String> hardwareDivergence = Optional.empty();
         private Optional<String> hardwareFailureDescription = Optional.empty();
+        private NodeReports reports = new NodeReports();
         private Optional<String> parentHostname = Optional.empty();
 
         public Builder() {}
@@ -527,6 +538,7 @@ public class NodeSpec {
             ipAddresses(node.ipAddresses);
             wantedRebootGeneration(node.wantedRebootGeneration);
             currentRebootGeneration(node.currentRebootGeneration);
+            reports(new NodeReports(node.reports));
 
             node.wantedDockerImage.ifPresent(this::wantedDockerImage);
             node.currentDockerImage.ifPresent(this::currentDockerImage);
@@ -692,6 +704,21 @@ public class NodeSpec {
             return this;
         }
 
+        public Builder reports(NodeReports reports) {
+            this.reports = reports;
+            return this;
+        }
+
+        public Builder report(String reportId, JsonNode report) {
+            this.reports.setReport(reportId, report);
+            return this;
+        }
+
+        public Builder removeReport(String reportId) {
+            reports.removeReport(reportId);
+            return this;
+        }
+
         public Builder parentHostname(String parentHostname) {
             this.parentHostname = Optional.of(parentHostname);
             return this;
@@ -705,6 +732,8 @@ public class NodeSpec {
             attributes.getRestartGeneration().ifPresent(this::currentRestartGeneration);
             attributes.getHardwareFailureDescription().ifPresent(this::hardwareFailureDescription);
             attributes.getWantToDeprovision().ifPresent(this::wantToDeprovision);
+            NodeReports.fromMap(Optional.of(attributes.getReports()));
+
             return this;
         }
 
@@ -816,6 +845,10 @@ public class NodeSpec {
             return hardwareFailureDescription;
         }
 
+        public NodeReports getReports() {
+            return reports;
+        }
+
         public Optional<String> getParentHostname() {
             return parentHostname;
         }
@@ -830,7 +863,7 @@ public class NodeSpec {
                     wantedFirmwareCheck, currentFirmwareCheck,
                     minCpuCores, minMainMemoryAvailableGb, minDiskAvailableGb,
                     fastDisk, bandwidth, ipAddresses, hardwareDivergence, hardwareFailureDescription,
-                    parentHostname);
+                    reports, parentHostname);
         }
 
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
index 46608edf120..d66be008a07 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
@@ -1,12 +1,17 @@
 // Copyright 2017 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.configserver.noderepository;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.base.Strings;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi;
 import com.yahoo.vespa.hosted.node.admin.configserver.HttpException;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.*;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.GetAclResponse;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.GetNodesResponse;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeMessageResponse;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeRepositoryNode;
 import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 import com.yahoo.vespa.hosted.provision.Node;
 
@@ -17,6 +22,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -26,6 +32,7 @@ import java.util.stream.Stream;
  */
 public class RealNodeRepository implements NodeRepository {
     private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(RealNodeRepository.class);
+    private static final ObjectMapper mapper = new ObjectMapper();
 
     private final ConfigServerApi configServerApi;
 
@@ -150,7 +157,6 @@ public class RealNodeRepository implements NodeRepository {
         throw new NodeRepositoryException("Unexpected message " + response.message + " " + response.errorCode);
     }
 
-
     private static NodeSpec createNodeSpec(NodeRepositoryNode node) {
         Objects.requireNonNull(node.type, "Unknown node type");
         NodeType nodeType = NodeType.valueOf(node.type);
@@ -177,6 +183,8 @@ public class RealNodeRepository implements NodeRepository {
                     node.membership.group, node.membership.index, node.membership.retired);
         }
 
+        NodeReports reports = NodeReports.fromMap(Optional.ofNullable(node.reports));
+
         return new NodeSpec(
                 hostName,
                 Optional.ofNullable(node.wantedDockerImage).map(DockerImage::new),
@@ -207,6 +215,7 @@ public class RealNodeRepository implements NodeRepository {
                 node.ipAddresses,
                 Optional.ofNullable(node.hardwareDivergence),
                 Optional.ofNullable(node.hardwareFailureDescription),
+                reports,
                 Optional.ofNullable(node.parentHostname));
     }
 
@@ -222,7 +231,7 @@ public class RealNodeRepository implements NodeRepository {
         return node;
     }
 
-    private static NodeRepositoryNode nodeRepositoryNodeFromNodeAttributes(NodeAttributes nodeAttributes) {
+    public static NodeRepositoryNode nodeRepositoryNodeFromNodeAttributes(NodeAttributes nodeAttributes) {
         NodeRepositoryNode node = new NodeRepositoryNode();
         node.currentDockerImage = nodeAttributes.getDockerImage().map(DockerImage::asString).orElse(null);
         node.currentRestartGeneration = nodeAttributes.getRestartGeneration().orElse(null);
@@ -233,6 +242,10 @@ public class RealNodeRepository implements NodeRepository {
         node.hardwareDivergence = nodeAttributes.getHardwareDivergence().orElse(null);
         node.hardwareFailureDescription = nodeAttributes.getHardwareFailureDescription().orElse(null);
         node.wantToDeprovision = nodeAttributes.getWantToDeprovision().orElse(null);
+
+        Map<String, JsonNode> reports = nodeAttributes.getReports();
+        node.reports = reports == null || reports.isEmpty() ? null : new TreeMap<>(reports);
+
         return node;
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java
index e6e2b675f5c..777d60de17d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java
@@ -4,7 +4,9 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.JsonNode;
 
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -87,10 +89,13 @@ public class NodeRepositoryNode {
     @JsonProperty("allowedToBeDown")
     public Boolean allowedToBeDown;
 
+    @JsonProperty("reports")
+    public Map<String, JsonNode> reports = null;
+
     @Override
     public String toString() {
         return "NodeRepositoryNode{" +
-                "state=" + state +
+                "state='" + state + '\'' +
                 ", hostname='" + hostname + '\'' +
                 ", ipAddresses=" + ipAddresses +
                 ", additionalIpAddresses=" + additionalIpAddresses +
@@ -107,16 +112,16 @@ public class NodeRepositoryNode {
                 ", wantedVespaVersion='" + wantedVespaVersion + '\'' +
                 ", currentOsVersion='" + currentOsVersion + '\'' +
                 ", wantedOsVersion='" + wantedOsVersion + '\'' +
-                ", currentFirmwareCheck='" + currentFirmwareCheck + '\'' +
-                ", wantedFirmwareCheck='" + wantedFirmwareCheck + '\'' +
+                ", currentFirmwareCheck=" + currentFirmwareCheck +
+                ", wantedFirmwareCheck=" + wantedFirmwareCheck +
                 ", failCount=" + failCount +
                 ", fastDisk=" + fastDisk +
                 ", bandwidth=" + bandwidth +
                 ", hardwareFailure=" + hardwareFailure +
                 ", hardwareFailureDescription='" + hardwareFailureDescription + '\'' +
                 ", hardwareDivergence='" + hardwareDivergence + '\'' +
-                ", environment=" + environment +
-                ", type=" + type +
+                ", environment='" + environment + '\'' +
+                ", type='" + type + '\'' +
                 ", wantedDockerImage='" + wantedDockerImage + '\'' +
                 ", currentDockerImage='" + currentDockerImage + '\'' +
                 ", parentHostname='" + parentHostname + '\'' +
@@ -126,6 +131,7 @@ public class NodeRepositoryNode {
                 ", minMainMemoryAvailableGb=" + minMainMemoryAvailableGb +
                 ", minCpuCores=" + minCpuCores +
                 ", allowedToBeDown=" + allowedToBeDown +
+                ", reports=" + reports +
                 '}';
     }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/BaseReport.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/BaseReport.java
new file mode 100644
index 00000000000..c98c5274434
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/BaseReport.java
@@ -0,0 +1,80 @@
+// 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.configserver.reports;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonGetter;
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.util.Objects;
+
+import static com.yahoo.yolean.Exceptions.uncheck;
+
+/**
+ * The most basic form of a node repository report on a node.
+ *
+ * <p>This class can be used directly for simple reports, or can be used as a base class for richer reports.
+ *
+ * <p><strong>Subclass requirements</strong>
+ *
+ * <ol>
+ *     <li>A subclass must maintain the property that {@link ObjectMapper} can map an instance to {@link JsonNode},
+ *     see {@link #toJsonNode()}.</li>
+ *     <li>A subclass must override {@link #updates(BaseReport)} and make sure to return false if
+ *     {@code !super.updates(current)}.</li>
+ * </ol>
+ *
+ * @author hakonhall
+ */
+// @Immutable
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public class BaseReport {
+    /** The time the report was created, in milliseconds since Epoch. */
+    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";
+
+    protected static final ObjectMapper mapper = new ObjectMapper();
+
+    private final Long createdMillis;
+    private final String description;
+
+    @JsonCreator
+    public BaseReport(@JsonProperty(CREATED_FIELD) Long createdMillisOrNull,
+                      @JsonProperty(DESCRIPTION_FIELD) String descriptionOrNull) {
+        this.createdMillis = createdMillisOrNull;
+        this.description = descriptionOrNull;
+
+    }
+
+    @JsonGetter(CREATED_FIELD)
+    public Long getCreatedMillisOrNull() {
+        return createdMillis;
+    }
+
+    @JsonGetter(DESCRIPTION_FIELD)
+    public String getDescriptionOrNull() {
+        return description;
+    }
+
+    /**
+     * Assume {@code this} is a freshly made report, and {@code current} is the report in the node repository:
+     * Return true iff the node repository should be updated.
+     *
+     * <p>The createdMillis field is ignored in this method (unless it is earlier than {@code current}'s?).
+     */
+    public boolean updates(BaseReport current) {
+        if (this == current) return false;
+        if (this.getClass() != current.getClass()) return true;
+        return !Objects.equals(description, current.description);
+    }
+
+    /** Returns {@code this} as a {@link JsonNode}. */
+    public JsonNode toJsonNode() {
+        return uncheck(() -> mapper.valueToTree(this));
+    }
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/package-info.java
new file mode 100644
index 00000000000..e78c4e773d3
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/package-info.java
@@ -0,0 +1,5 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+@ExportPackage
+package com.yahoo.vespa.hosted.node.admin.configserver.reports;
+
+import com.yahoo.osgi.annotation.ExportPackage;
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
new file mode 100644
index 00000000000..c324238d275
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNodeTest.java
@@ -0,0 +1,73 @@
+// 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.configserver.noderepository.bindings;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.yahoo.test.json.JsonTestHelper;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.RealNodeRepository;
+import com.yahoo.vespa.hosted.node.admin.configserver.reports.BaseReport;
+import com.yahoo.vespa.hosted.provision.node.Report;
+import org.junit.Test;
+
+import java.util.HashMap;
+
+import static com.yahoo.yolean.Exceptions.uncheck;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author hakonhall
+ */
+public class NodeRepositoryNodeTest {
+    private static final ObjectMapper mapper = new ObjectMapper();
+    private NodeRepositoryNode node = new NodeRepositoryNode();
+    private NodeAttributes attributes = new NodeAttributes();
+
+
+    /**
+     * Test both how NodeRepositoryNode serialize, and the serialization of an empty NodeRepositoryNode
+     * patched with a NodeAttributes, as they work in tandem:
+     *   NodeAttributes -> NodeRepositoryNode -> JSON.
+     */
+    @Test
+    public void testReportsSerialization() {
+        // Make sure we don't accidentally patch with "reports": null, as that actually means removing all reports.
+        assertEquals(JsonInclude.Include.NON_NULL, NodeRepositoryNode.class.getAnnotation(JsonInclude.class).value());
+
+        // Absent report and unmodified attributes => nothing about reports in JSON
+        node.reports = null;
+        assertNodeAndAttributes("{}");
+
+        // Make sure we're able to patch with a null report value ("reportId": null), as that means removing the report.
+        node.reports = new HashMap<>();
+        node.reports.put("rid", null);
+        attributes.withReportRemoved("rid");
+        assertNodeAndAttributes("{\"reports\": {\"rid\": null}}");
+
+        // Add ridTwo report to node
+        ObjectNode reportJson = mapper.createObjectNode();
+        reportJson.set(Report.CREATED_FIELD, mapper.valueToTree(3));
+        reportJson.set(Report.DESCRIPTION_FIELD, mapper.valueToTree("desc"));
+        node.reports.put("ridTwo", reportJson);
+
+        // Add ridTwo report to attributes
+        BaseReport reportTwo = new BaseReport(3L, "desc");
+        attributes.withReport("ridTwo", reportTwo.toJsonNode());
+
+        // Verify node serializes to expected, as well as attributes patched on node.
+        assertNodeAndAttributes("{\"reports\": {\"rid\": null, \"ridTwo\": {\"createdMillis\": 3, \"description\": \"desc\"}}}");
+    }
+
+    private void assertNodeAndAttributes(String expectedJson) {
+        assertNodeJson(node, expectedJson);
+        assertNodeJson(RealNodeRepository.nodeRepositoryNodeFromNodeAttributes(attributes), expectedJson);
+    }
+
+    private void assertNodeJson(NodeRepositoryNode node, String json) {
+        JsonNode expected = uncheck(() -> mapper.readTree(json));
+        JsonNode actual = uncheck(() -> mapper.valueToTree(node));
+        JsonTestHelper.assertJsonEquals(actual, expected);
+    }
+}
\ No newline at end of file
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/BaseReportTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/BaseReportTest.java
new file mode 100644
index 00000000000..6456a09dd27
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/reports/BaseReportTest.java
@@ -0,0 +1,38 @@
+// 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.configserver.reports;
+
+import com.yahoo.test.json.JsonTestHelper;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author hakonhall
+ */
+public class BaseReportTest {
+    @Test
+    public void testSerialization() {
+        BaseReport report = new BaseReport(1L, "desc");
+        JsonTestHelper.assertJsonEquals(new BaseReport(1L, "desc").toJsonNode(),
+                "{\"createdMillis\": 1, \"description\": \"desc\"}");
+        JsonTestHelper.assertJsonEquals(new BaseReport(null, "desc").toJsonNode(),
+                "{\"description\": \"desc\"}");
+        JsonTestHelper.assertJsonEquals(new BaseReport(1L, null).toJsonNode(),
+                "{\"createdMillis\": 1}");
+        JsonTestHelper.assertJsonEquals(new BaseReport(null, null).toJsonNode(),
+                "{}");
+    }
+
+    @Test
+    public void testShouldUpdate() {
+        BaseReport report = new BaseReport(1L, "desc");
+        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));
+    }
+}
\ No newline at end of file
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java
index e8b423e73ac..5748d4034eb 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java
@@ -1,11 +1,11 @@
 // Copyright 2017 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.integrationTests;
 
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.AddNode;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.provision.Node;
 
 import java.util.Collections;