Do not use serializable NodeRepositoryNode internally in controller

4 files changed, 267 insertions, 245 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java
index 5f46b949844..25357ad7f98 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java
@@ -1,7 +1,6 @@
 // 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.controller.api.integration.configserver;
 
-import com.fasterxml.jackson.databind.JsonNode;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.DockerImage;
@@ -9,26 +8,29 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeHistory;
 
 import java.time.Instant;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.UUID;
 
 /**
  * A node in hosted Vespa.
  *
+ * This is immutable and all fields are guaranteed to be non-null. This should never leak any wire format types or
+ * types from third-party libraries.
+ *
+ * Use {@link Node#builder()} or {@link Node#builder(Node)} to create instances of this.
+ *
  * @author mpolden
  * @author jonmv
  */
 public class Node {
 
+    private final String id;
     private final HostName hostname;
     private final Optional<HostName> parentHostname;
     private final State state;
@@ -50,204 +52,278 @@ public class Node {
     private final long rebootGeneration;
     private final long wantedRebootGeneration;
     private final int cost;
+    private final int failCount;
     private final String flavor;
     private final String clusterId;
     private final ClusterType clusterType;
+    private final String group;
     private final boolean retired;
     private final boolean wantToRetire;
     private final boolean wantToDeprovision;
     private final boolean wantToRebuild;
     private final Optional<TenantName> reservedTo;
     private final Optional<ApplicationId> exclusiveTo;
-    private final Map<String, JsonNode> reports;
-    private final List<NodeHistory> history;
+    private final Map<String, String> reports;
+    private final List<Event> history;
+    private final Set<String> ipAddresses;
     private final Set<String> additionalIpAddresses;
-    private final String openStackId;
+    private final Set<String> additionalHostnames;
     private final Optional<String> switchHostname;
     private final Optional<String> modelName;
-
-    public Node(HostName hostname, Optional<HostName> parentHostname, State state, NodeType type, NodeResources resources, Optional<ApplicationId> owner,
-                Version currentVersion, Version wantedVersion, Version currentOsVersion, Version wantedOsVersion,
-                Optional<Instant> currentFirmwareCheck, Optional<Instant> wantedFirmwareCheck, ServiceState serviceState,
-                Optional<Instant> suspendedSince, long restartGeneration, long wantedRestartGeneration, long rebootGeneration, long wantedRebootGeneration,
-                int cost, String flavor, String clusterId, ClusterType clusterType, boolean retired, boolean wantToRetire, boolean wantToDeprovision,
-                boolean wantToRebuild, Optional<TenantName> reservedTo, Optional<ApplicationId> exclusiveTo,
-                DockerImage wantedDockerImage, DockerImage currentDockerImage, Map<String, JsonNode> reports, List<NodeHistory> history,
-                Set<String> additionalIpAddresses, String openStackId, Optional<String> switchHostname, Optional<String> modelName) {
-        this.hostname = hostname;
-        this.parentHostname = parentHostname;
-        this.state = state;
-        this.type = type;
-        this.resources = resources;
-        this.owner = owner;
-        this.currentVersion = currentVersion;
-        this.wantedVersion = wantedVersion;
-        this.currentOsVersion = currentOsVersion;
-        this.wantedOsVersion = wantedOsVersion;
-        this.currentFirmwareCheck = currentFirmwareCheck;
-        this.wantedFirmwareCheck = wantedFirmwareCheck;
-        this.serviceState = serviceState;
-        this.suspendedSince = suspendedSince;
+    private final Environment environment;
+
+    private Node(String id, HostName hostname, Optional<HostName> parentHostname, State state, NodeType type,
+                 NodeResources resources, Optional<ApplicationId> owner, Version currentVersion, Version wantedVersion,
+                 Version currentOsVersion, Version wantedOsVersion, Optional<Instant> currentFirmwareCheck,
+                 Optional<Instant> wantedFirmwareCheck, ServiceState serviceState, Optional<Instant> suspendedSince,
+                 long restartGeneration, long wantedRestartGeneration, long rebootGeneration,
+                 long wantedRebootGeneration, int cost, int failCount, String flavor, String clusterId,
+                 ClusterType clusterType, String group, boolean retired, boolean wantToRetire, boolean wantToDeprovision,
+                 boolean wantToRebuild, Optional<TenantName> reservedTo, Optional<ApplicationId> exclusiveTo,
+                 DockerImage wantedDockerImage, DockerImage currentDockerImage, Map<String, String> reports,
+                 List<Event> history, Set<String> ipAddresses, Set<String> additionalIpAddresses,
+                 Set<String> additionalHostnames, Optional<String> switchHostname,
+                 Optional<String> modelName, Environment environment) {
+        this.id = Objects.requireNonNull(id, "id must be non-null");
+        this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null");
+        this.parentHostname = Objects.requireNonNull(parentHostname, "parentHostname must be non-null");
+        this.state = Objects.requireNonNull(state, "state must be non-null");
+        this.type = Objects.requireNonNull(type, "type must be non-null");
+        this.resources = Objects.requireNonNull(resources, "resources must be non-null");
+        this.owner = Objects.requireNonNull(owner, "owner must be non-null");
+        this.currentVersion = Objects.requireNonNull(currentVersion, "currentVersion must be non-null");
+        this.wantedVersion = Objects.requireNonNull(wantedVersion, "wantedVersion must be non-null");
+        this.currentOsVersion = Objects.requireNonNull(currentOsVersion, "currentOsVersion must be non-null");
+        this.wantedOsVersion = Objects.requireNonNull(wantedOsVersion, "wantedOsVersion must be non-null");
+        this.currentFirmwareCheck = Objects.requireNonNull(currentFirmwareCheck, "currentFirmwareCheck must be non-null");
+        this.wantedFirmwareCheck = Objects.requireNonNull(wantedFirmwareCheck, "wantedFirmwareCheck must be non-null");
+        this.serviceState = Objects.requireNonNull(serviceState, "serviceState must be non-null");
+        this.suspendedSince = Objects.requireNonNull(suspendedSince, "suspendedSince must be non-null");
         this.restartGeneration = restartGeneration;
         this.wantedRestartGeneration = wantedRestartGeneration;
         this.rebootGeneration = rebootGeneration;
         this.wantedRebootGeneration = wantedRebootGeneration;
         this.cost = cost;
-        this.flavor = flavor;
-        this.clusterId = clusterId;
-        this.clusterType = clusterType;
+        this.failCount = failCount;
+        this.flavor = Objects.requireNonNull(flavor, "flavor must be non-null");
+        this.clusterId = Objects.requireNonNull(clusterId, "clusterId must be non-null");
+        this.clusterType = Objects.requireNonNull(clusterType, "clusterType must be non-null");
         this.retired = retired;
+        this.group = Objects.requireNonNull(group, "group must be non-null");
         this.wantToRetire = wantToRetire;
         this.wantToDeprovision = wantToDeprovision;
-        this.reservedTo = reservedTo;
-        this.exclusiveTo = exclusiveTo;
-        this.wantedDockerImage = wantedDockerImage;
-        this.currentDockerImage = currentDockerImage;
+        this.reservedTo = Objects.requireNonNull(reservedTo, "reservedTo must be non-null");
+        this.exclusiveTo = Objects.requireNonNull(exclusiveTo, "exclusiveTo must be non-null");
+        this.wantedDockerImage = Objects.requireNonNull(wantedDockerImage, "wantedDockerImage must be non-null");
+        this.currentDockerImage = Objects.requireNonNull(currentDockerImage, "currentDockerImage must be non-null");
         this.wantToRebuild = wantToRebuild;
-        this.reports = reports;
-        this.history = history;
-        this.openStackId = openStackId;
-        this.additionalIpAddresses = additionalIpAddresses;
-        this.switchHostname = switchHostname;
-        this.modelName = modelName;
+        this.reports = Map.copyOf(Objects.requireNonNull(reports, "reports must be non-null"));
+        this.history = List.copyOf(Objects.requireNonNull(history, "history must be non-null"));
+        this.ipAddresses = Set.copyOf(Objects.requireNonNull(ipAddresses, "ipAddresses must be non-null"));
+        this.additionalIpAddresses = Set.copyOf(Objects.requireNonNull(additionalIpAddresses, "additionalIpAddresses must be non-null"));
+        this.additionalHostnames = Set.copyOf(Objects.requireNonNull(additionalHostnames, "additionalHostnames must be non-null"));
+        this.switchHostname = Objects.requireNonNull(switchHostname, "switchHostname must be non-null");
+        this.modelName = Objects.requireNonNull(modelName, "modelName must be non-null");
+        this.environment = Objects.requireNonNull(environment, "environment must be non-ull");
     }
 
+    /** The cloud provider's unique ID for this */
+    public String id() {
+        return id;
+    }
+
+    /** The hostname of this */
     public HostName hostname() {
         return hostname;
     }
 
+    /** The parent hostname of this, if any */
     public Optional<HostName> parentHostname() {
         return parentHostname;
     }
 
+    /** Current state of this */
     public State state() { return state; }
 
+    /** The node type of this */
     public NodeType type() {
         return type;
     }
 
+    /** Resources, such as CPU and memory, of this */
     public NodeResources resources() {
         return resources;
     }
 
+    /** The application owning this, if any */
     public Optional<ApplicationId> owner() {
         return owner;
     }
 
+    /** The Vespa version this is currently running */
     public Version currentVersion() {
         return currentVersion;
     }
 
+    /** The wanted Vespa version */
     public Version wantedVersion() {
         return wantedVersion;
     }
 
+    /** The OS version this is currently running */
     public Version currentOsVersion() {
         return currentOsVersion;
     }
 
+    /** The wanted OS version */
     public Version wantedOsVersion() {
         return wantedOsVersion;
     }
 
+    /** The container image of this is currently running */
     public DockerImage currentDockerImage() {
         return currentDockerImage;
     }
 
+    /** The wanted Docker image */
     public DockerImage wantedDockerImage() {
         return wantedDockerImage;
     }
 
+    /** The last time this checked for a firmware update */
     public Optional<Instant> currentFirmwareCheck() {
         return currentFirmwareCheck;
     }
 
+    /** The wanted time this should check for a firmware update */
     public Optional<Instant> wantedFirmwareCheck() {
         return wantedFirmwareCheck;
     }
 
+    /** The current service state of this */
     public ServiceState serviceState() {
         return serviceState;
     }
 
+    /** The most recent time this suspended, if any */
     public Optional<Instant> suspendedSince() {
         return suspendedSince;
     }
 
+    /** The current restart generation */
     public long restartGeneration() {
         return restartGeneration;
     }
 
+    /** The wanted restart generation */
     public long wantedRestartGeneration() {
         return wantedRestartGeneration;
     }
 
+    /** The current reboot generation */
     public long rebootGeneration() {
         return rebootGeneration;
     }
 
+    /** The wanted reboot generation */
     public long wantedRebootGeneration() {
         return wantedRebootGeneration;
     }
 
+    /** A number representing the cost of this */
     public int cost() {
         return cost;
     }
 
+    /** How many times this has failed */
+    public int failCount() {
+        return failCount;
+    }
+
+    /** The flavor of this */
     public String flavor() {
         return flavor;
     }
 
+    /** The cluster ID of this, empty string if unallocated */
     public String clusterId() {
         return clusterId;
     }
 
+    /** The cluster type of this */
     public ClusterType clusterType() {
         return clusterType;
     }
 
+    /** Whether this is retired */
     public boolean retired() {
         return retired;
     }
 
+    /** The group of this node, empty string if unallocated */
+    public String group() {
+        return group;
+    }
+
+    /** Whether this node has been requested to retire */
     public boolean wantToRetire() {
         return wantToRetire;
     }
 
+    /** Whether this node has been requested to deprovision */
     public boolean wantToDeprovision() {
         return wantToDeprovision;
     }
 
+    /** Whether this node has been requested to rebuild */
     public boolean wantToRebuild() {
         return wantToRebuild;
     }
 
+    /** The tenant this has been reserved to, if any */
     public Optional<TenantName> reservedTo() { return reservedTo; }
 
+    /** The application this has been provisioned exclusively for, if any */
     public Optional<ApplicationId> exclusiveTo() { return exclusiveTo; }
 
-    public Map<String, JsonNode> reports() {
+    /** Returns the reports of this node. Key is the report ID. Value is untyped, but is typically a JSON string */
+    public Map<String, String> reports() {
         return reports;
     }
 
-    public List<NodeHistory> history() {
+    /** History of events affecting this */
+    public List<Event> history() {
         return history;
     }
 
+    /** IP addresses of this */
+    public Set<String> ipAddresses() {
+        return ipAddresses;
+    }
+
+    /** Additional IP addresses available on this, usable by child nodes */
     public Set<String> additionalIpAddresses() {
         return additionalIpAddresses;
     }
 
-    public String openStackId() {
-        return openStackId;
+    /** Additional hostnames available on this, usable by child nodes */
+    public Set<String> additionalHostnames() {
+        return additionalHostnames;
     }
 
+    /** Hostname of the switch this is connected to, if any */
     public Optional<String> switchHostname() {
         return switchHostname;
     }
 
+    /** The server model of this, if any */
     public Optional<String> modelName() { return modelName; }
 
+    /** The environment this runs in */
+    public Environment environment() {
+        return environment;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -293,48 +369,106 @@ public class Node {
         combined,
         unknown
     }
+    
+    /** Known nope environments */
+    public enum Environment {
+        bareMetal,
+        virtualMachine,
+        dockerContainer,
+    }
+
+    /** A node event */
+    public static class Event {
+
+        private final Instant at;
+        private final String agent;
+        private final String name;
+
+        public Event(Instant at, String agent, String name) {
+            this.at = Objects.requireNonNull(at);
+            this.agent = Objects.requireNonNull(agent);
+            this.name = Objects.requireNonNull(name);
+        }
+
+        /** The time this occurred */
+        public Instant at() {
+            return at;
+        }
+
+        /** The agent responsible for this */
+        public String agent() {
+            return agent;
+        }
+
+        /** Name of the event */
+        public String name() {
+            return name;
+        }
+
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
 
+    public static Builder builder(Node node) {
+        return new Builder(node);
+    }
+
+    /**
+     * Builder for a {@link Node}.
+     *
+     * The appropriate builder method must be called for any field that does not have a default value.
+     */
     public static class Builder {
+
         private HostName hostname;
+
+        private String id = UUID.randomUUID().toString();
         private Optional<HostName> parentHostname = Optional.empty();
-        private State state;
-        private NodeType type;
-        private NodeResources resources;
+        private State state = State.active;
+        private NodeType type = NodeType.host;
+        private NodeResources resources = NodeResources.unspecified();
         private Optional<ApplicationId> owner = Optional.empty();
-        private Version currentVersion;
-        private Version wantedVersion;
-        private Version currentOsVersion;
-        private Version wantedOsVersion;
-        private DockerImage currentDockerImage;
-        private DockerImage wantedDockerImage;
+        private Version currentVersion = Version.emptyVersion;
+        private Version wantedVersion = Version.emptyVersion;
+        private Version currentOsVersion = Version.emptyVersion;
+        private Version wantedOsVersion = Version.emptyVersion;
+        private DockerImage currentDockerImage = DockerImage.EMPTY;
+        private DockerImage wantedDockerImage = DockerImage.EMPTY;
         private Optional<Instant> currentFirmwareCheck = Optional.empty();
         private Optional<Instant> wantedFirmwareCheck = Optional.empty();
-        private ServiceState serviceState;
+        private ServiceState serviceState = ServiceState.unknown;
         private Optional<Instant> suspendedSince = Optional.empty();
-        private long restartGeneration;
-        private long wantedRestartGeneration;
-        private long rebootGeneration;
-        private long wantedRebootGeneration;
-        private int cost;
-        private String flavor;
-        private String clusterId;
-        private ClusterType clusterType;
-        private boolean retired;
-        private boolean wantToRetire;
-        private boolean wantToDeprovision;
-        private boolean wantToRebuild;
+        private long restartGeneration = 0;
+        private long wantedRestartGeneration = 0;
+        private long rebootGeneration = 0;
+        private long wantedRebootGeneration = 0;
+        private int cost = 0;
+        private int failCount = 0;
+        private String flavor = "default";
+        private String clusterId = "";
+        private ClusterType clusterType = ClusterType.unknown;
+        private String group = "";
+        private boolean retired = false;
+        private boolean wantToRetire = false;
+        private boolean wantToDeprovision = false;
+        private boolean wantToRebuild = false;
         private Optional<TenantName> reservedTo = Optional.empty();
         private Optional<ApplicationId> exclusiveTo = Optional.empty();
-        private Map<String, JsonNode> reports = new HashMap<>();
-        private List<NodeHistory> history = new ArrayList<>();
-        private Set<String> additionalIpAddresses = new HashSet<>();
-        private String openStackId;
+        private Map<String, String> reports = Map.of();
+        private List<Event> history = List.of();
+        private Set<String> ipAddresses = Set.of();
+        private Set<String> additionalIpAddresses = Set.of();
+        private Set<String> additionalHostnames = Set.of();
         private Optional<String> switchHostname = Optional.empty();
         private Optional<String> modelName = Optional.empty();
+        private Environment environment = Environment.bareMetal;
 
-        public Builder() { }
+        private Builder() {}
 
-        public Builder(Node node) {
+        private Builder(Node node) {
+            this.id = node.id;
             this.hostname = node.hostname;
             this.parentHostname = node.parentHostname;
             this.state = node.state;
@@ -347,18 +481,20 @@ public class Node {
             this.wantedOsVersion = node.wantedOsVersion;
             this.currentDockerImage = node.currentDockerImage;
             this.wantedDockerImage = node.wantedDockerImage;
-            this.currentFirmwareCheck = node.currentFirmwareCheck;
-            this.wantedFirmwareCheck = node.wantedFirmwareCheck;
             this.serviceState = node.serviceState;
             this.suspendedSince = node.suspendedSince;
+            this.currentFirmwareCheck = node.currentFirmwareCheck;
+            this.wantedFirmwareCheck = node.wantedFirmwareCheck;
             this.restartGeneration = node.restartGeneration;
             this.wantedRestartGeneration = node.wantedRestartGeneration;
             this.rebootGeneration = node.rebootGeneration;
             this.wantedRebootGeneration = node.wantedRebootGeneration;
             this.cost = node.cost;
+            this.failCount = node.failCount;
             this.flavor = node.flavor;
             this.clusterId = node.clusterId;
             this.clusterType = node.clusterType;
+            this.group = node.group;
             this.retired = node.retired;
             this.wantToRetire = node.wantToRetire;
             this.wantToDeprovision = node.wantToDeprovision;
@@ -367,10 +503,21 @@ public class Node {
             this.exclusiveTo = node.exclusiveTo;
             this.reports = node.reports;
             this.history = node.history;
+            this.ipAddresses = node.ipAddresses;
             this.additionalIpAddresses = node.additionalIpAddresses;
-            this.openStackId = node.openStackId;
+            this.additionalHostnames = node.additionalHostnames;
             this.switchHostname = node.switchHostname;
             this.modelName = node.modelName;
+            this.environment = node.environment;
+        }
+
+        public Builder id(String id) {
+            this.id = id;
+            return this;
+        }
+
+        public Builder hostname(String hostname) {
+            return hostname(HostName.from(hostname));
         }
 
         public Builder hostname(HostName hostname) {
@@ -378,6 +525,10 @@ public class Node {
             return this;
         }
 
+        public Builder parentHostname(String parentHostname) {
+            return parentHostname(HostName.from(parentHostname));
+        }
+
         public Builder parentHostname(HostName parentHostname) {
             this.parentHostname = Optional.ofNullable(parentHostname);
             return this;
@@ -478,6 +629,11 @@ public class Node {
             return this;
         }
 
+        public Builder failCount(int failCount) {
+            this.failCount = failCount;
+            return this;
+        }
+
         public Builder flavor(String flavor) {
             this.flavor = flavor;
             return this;
@@ -493,6 +649,11 @@ public class Node {
             return this;
         }
 
+        public Builder group(String group) {
+            this.group = group;
+            return this;
+        }
+
         public Builder retired(boolean retired) {
             this.retired = retired;
             return this;
@@ -523,18 +684,23 @@ public class Node {
             return this;
         }
 
-        public Builder history(List<NodeHistory> history) {
+        public Builder history(List<Event> history) {
             this.history = history;
             return this;
         }
 
+        public Builder ipAddresses(Set<String> ipAdresses) {
+            this.ipAddresses = ipAdresses;
+            return this;
+        }
+
         public Builder additionalIpAddresses(Set<String> additionalIpAddresses) {
             this.additionalIpAddresses = additionalIpAddresses;
             return this;
         }
 
-        public Builder openStackId(String openStackId) {
-            this.openStackId = openStackId;
+        public Builder additionalHostnames(Set<String> additionalHostnames) {
+            this.additionalHostnames = additionalHostnames;
             return this;
         }
 
@@ -548,18 +714,26 @@ public class Node {
             return this;
         }
 
-        public Builder reports(Map<String, JsonNode> reports) {
+        public Builder reports(Map<String, String> reports) {
             this.reports = reports;
             return this;
         }
 
+        public Builder environment(Environment environment) {
+            this.environment = environment;
+            return this;
+        }
+
         public Node build() {
-            return new Node(hostname, parentHostname, state, type, resources, owner, currentVersion, wantedVersion,
+            return new Node(id, hostname, parentHostname, state, type, resources, owner, currentVersion, wantedVersion,
                             currentOsVersion, wantedOsVersion, currentFirmwareCheck, wantedFirmwareCheck, serviceState,
-                            suspendedSince, restartGeneration, wantedRestartGeneration, rebootGeneration, wantedRebootGeneration,
-                            cost, flavor, clusterId, clusterType, retired, wantToRetire, wantToDeprovision, wantToRebuild, reservedTo, exclusiveTo,
-                            wantedDockerImage, currentDockerImage, reports, history, additionalIpAddresses, openStackId, switchHostname, modelName);
+                            suspendedSince, restartGeneration, wantedRestartGeneration, rebootGeneration,
+                            wantedRebootGeneration, cost, failCount, flavor, clusterId, clusterType, group, retired,
+                            wantToRetire, wantToDeprovision, wantToRebuild, reservedTo, exclusiveTo, wantedDockerImage,
+                            currentDockerImage, reports, history, ipAddresses, additionalIpAddresses,
+                            additionalHostnames, switchHostname, modelName, environment);
         }
 
     }
+
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java
index ac4ff0a80a0..3c16eac06c7 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java
@@ -3,21 +3,15 @@ package com.yahoo.vespa.hosted.controller.api.integration.configserver;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.HostName;
-import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeList;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeMembership;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.OrchestratorStatus;
 
 import java.net.URI;
 import java.time.Duration;
-import java.time.Instant;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -26,7 +20,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
- * A minimal interface to the node repository, providing only the operations used by the controller.
+ * Node repository interface intended for use by the controller.
  *
  * @author mpolden
  */
@@ -38,10 +32,7 @@ public interface NodeRepository {
 
     void setState(ZoneId zone, NodeState nodeState, String hostname);
 
-    NodeRepositoryNode getNode(ZoneId zone, String hostname);
-
-    // TODO: Migrate any callers to list() and remove this method
-    NodeList listNodes(ZoneId zone);
+    Node getNode(ZoneId zone, String hostname);
 
     /** List all nodes in given zone */
     List<Node> list(ZoneId zone, boolean includeDeprovisioned);
@@ -96,145 +87,4 @@ public interface NodeRepository {
     /** Checks whether the zone has the spare capacity to remove the given hosts */
     boolean isReplaceable(ZoneId zoneId, List<HostName> hostNames);
 
-    static Node toNode(NodeRepositoryNode node) {
-        var application = Optional.ofNullable(node.getOwner())
-                                  .map(owner -> ApplicationId.from(owner.getTenant(), owner.getApplication(),
-                                                                   owner.getInstance()));
-        var parentHostname = Optional.ofNullable(node.getParentHostname()).map(HostName::from);
-        var resources = new NodeResources(
-                toDouble(node.getResources().getVcpu()),
-                toDouble(node.getResources().getMemoryGb()),
-                toDouble(node.getResources().getDiskGb()),
-                toDouble(node.getResources().getBandwidthGbps()),
-                diskSpeedFromString(node.getResources().getDiskSpeed()),
-                storageTypeFromString(node.getResources().getStorageType()));
-        return new Node(HostName.from(node.getHostname()),
-                        parentHostname,
-                        fromJacksonState(node.getState()),
-                        fromJacksonType(node.getType()),
-                        resources,
-                        application,
-                        versionFrom(node.getVespaVersion()),
-                        versionFrom(node.getWantedVespaVersion()),
-                        versionFrom(node.getCurrentOsVersion()),
-                        versionFrom(node.getWantedOsVersion()),
-                        Optional.ofNullable(node.getCurrentFirmwareCheck()).map(Instant::ofEpochMilli),
-                        Optional.ofNullable(node.getWantedFirmwareCheck()).map(Instant::ofEpochMilli),
-                        toServiceState(node.getOrchestratorStatus()),
-                        Optional.ofNullable(node.suspendedSinceMillis()).map(Instant::ofEpochMilli),
-                        toInt(node.getCurrentRestartGeneration()),
-                        toInt(node.getRestartGeneration()),
-                        toInt(node.getCurrentRebootGeneration()),
-                        toInt(node.getRebootGeneration()),
-                        toInt(node.getCost()),
-                        node.getFlavor(),
-                        clusterIdOf(node.getMembership()),
-                        clusterTypeOf(node.getMembership()),
-                        Optional.ofNullable(node.getMembership()).map(NodeMembership::getRetired).orElse(false),
-                        node.getWantToRetire(),
-                        node.getWantToDeprovision(),
-                        node.getWantToRebuild(), Optional.ofNullable(node.getReservedTo()).map(TenantName::from),
-                        Optional.ofNullable(node.getExclusiveTo()).map(ApplicationId::fromSerializedForm),
-                        dockerImageFrom(node.getWantedDockerImage()),
-                        dockerImageFrom(node.getCurrentDockerImage()),
-                        node.getReports(),
-                        node.getHistory(),
-                        node.getAdditionalIpAddresses(),
-                        node.getOpenStackId(),
-                        Optional.ofNullable(node.getSwitchHostname()),
-                        Optional.ofNullable(node.getModelName()));
-    }
-
-    private static String clusterIdOf(NodeMembership nodeMembership) {
-        return nodeMembership == null ? "" : nodeMembership.clusterid;
-    }
-
-    private static Node.ClusterType clusterTypeOf(NodeMembership nodeMembership) {
-        if (nodeMembership == null) return Node.ClusterType.unknown;
-        switch (nodeMembership.clustertype) {
-            case "admin": return Node.ClusterType.admin;
-            case "content": return Node.ClusterType.content;
-            case "container": return Node.ClusterType.container;
-            case "combined": return Node.ClusterType.combined;
-        }
-        return Node.ClusterType.unknown;
-    }
-
-    // Convert Jackson type to config.provision type
-    private static NodeType fromJacksonType(com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeType nodeType) {
-        switch (nodeType) {
-            case tenant: return NodeType.tenant;
-            case host: return NodeType.host;
-            case proxy: return NodeType.proxy;
-            case proxyhost: return NodeType.proxyhost;
-            case config: return NodeType.config;
-            case confighost: return NodeType.confighost;
-            case controller: return NodeType.controller;
-            case controllerhost: return NodeType.controllerhost;
-            default: throw new IllegalArgumentException("Unknown type: " + nodeType);
-        }
-    }
-
-    private static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State fromJacksonState(NodeState state) {
-        switch (state) {
-            case provisioned: return Node.State.provisioned;
-            case ready: return Node.State.ready;
-            case reserved: return Node.State.reserved;
-            case active: return Node.State.active;
-            case inactive: return Node.State.inactive;
-            case dirty: return Node.State.dirty;
-            case failed: return Node.State.failed;
-            case parked: return Node.State.parked;
-            case breakfixed: return Node.State.breakfixed;
-            case deprovisioned: return Node.State.deprovisioned;
-        }
-        return Node.State.unknown;
-    }
-
-    private static NodeResources.DiskSpeed diskSpeedFromString(String diskSpeed) {
-        if (diskSpeed == null) return NodeResources.DiskSpeed.getDefault();
-        switch (diskSpeed) {
-            case "fast": return NodeResources.DiskSpeed.fast;
-            case "slow": return NodeResources.DiskSpeed.slow;
-            case "any": return NodeResources.DiskSpeed.any;
-            default: throw new IllegalArgumentException("Unknown disk speed '" + diskSpeed + "'");
-        }
-    }
-
-    private static NodeResources.StorageType storageTypeFromString(String storageType) {
-        if (storageType == null) return NodeResources.StorageType.getDefault();
-        switch (storageType) {
-            case "remote": return NodeResources.StorageType.remote;
-            case "local": return NodeResources.StorageType.local;
-            case "any": return NodeResources.StorageType.any;
-            default: throw new IllegalArgumentException("Unknown storage type '" + storageType + "'");
-        }
-    }
-
-    private static Node.ServiceState toServiceState(OrchestratorStatus orchestratorStatus) {
-        switch (orchestratorStatus) {
-            case ALLOWED_TO_BE_DOWN: return Node.ServiceState.allowedDown;
-            case PERMANENTLY_DOWN: return Node.ServiceState.permanentlyDown;
-            case NO_REMARKS: return Node.ServiceState.expectedUp;
-        }
-
-        return Node.ServiceState.unknown;
-    }
-
-    private static double toDouble(Double d) {
-        return d == null ? 0 : d;
-    }
-
-    private static int toInt(Integer i) {
-        return i == null ? 0 : i;
-    }
-
-    private static Version versionFrom(String s) {
-        return s == null ? Version.emptyVersion : Version.fromString(s);
-    }
-
-    private static DockerImage dockerImageFrom(String s) {
-        return s == null ? DockerImage.EMPTY : DockerImage.fromString(s);
-    }
-
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/repair/RepairTicketReport.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/repair/RepairTicketReport.java
index c2425fe0f72..97c6222e77d 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/repair/RepairTicketReport.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/repair/RepairTicketReport.java
@@ -1,7 +1,6 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.api.integration.repair;
 
-import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.JsonNode;
@@ -53,8 +52,8 @@ public class RepairTicketReport {
         return REPORT_ID;
     }
 
-    public static RepairTicketReport fromJsonNode(JsonNode node) {
-        return uncheck(() -> objectMapper.treeToValue(node, RepairTicketReport.class));
+    public static RepairTicketReport fromJsonNode(String jsonReport) {
+        return uncheck(() -> objectMapper.readValue(jsonReport, RepairTicketReport.class));
     }
 
     public JsonNode toJsonNode() {
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VCMRReport.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VCMRReport.java
index a3c0af95053..8ebfde9f475 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VCMRReport.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VCMRReport.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.vcmr;
 
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
@@ -69,12 +68,12 @@ public class VCMRReport {
     /**
      * Serialization functions - mapped to {@link Node#reports()}
      */
-    public static VCMRReport fromReports(Map<String, JsonNode> reports) {
+    public static VCMRReport fromReports(Map<String, String> reports) {
         var serialized = reports.get(REPORT_ID);
         if (serialized == null)
             return new VCMRReport();
 
-        return uncheck(() -> objectMapper.treeToValue(serialized, VCMRReport.class));
+        return uncheck(() -> objectMapper.readValue(serialized, VCMRReport.class));
     }
 
     /**