Merge pull request #18622 from vespa-engine/mpolden/controller-node-cleanup-2

Remove Jackson types from NodeRepository interface

12 files changed, 193 insertions, 187 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 9b63f5630c9..3e380b77a86 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
@@ -53,7 +53,7 @@ public class Node {
     private final long wantedRebootGeneration;
     private final int cost;
     private final int failCount;
-    private final String flavor;
+    private final Optional<String> flavor;
     private final String clusterId;
     private final ClusterType clusterType;
     private final String group;
@@ -77,7 +77,7 @@ public class Node {
                  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,
+                 long wantedRebootGeneration, int cost, int failCount, Optional<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,
@@ -241,7 +241,7 @@ public class Node {
     }
 
     /** The flavor of this */
-    public String flavor() {
+    public Optional<String> flavor() {
         return flavor;
     }
 
@@ -458,7 +458,7 @@ public class Node {
         private long wantedRebootGeneration = 0;
         private int cost = 0;
         private int failCount = 0;
-        private String flavor = "default";
+        private Optional<String> flavor = Optional.empty();
         private String clusterId = "";
         private ClusterType clusterType = ClusterType.unknown;
         private String group = "";
@@ -647,7 +647,7 @@ public class Node {
         }
 
         public Builder flavor(String flavor) {
-            this.flavor = flavor;
+            this.flavor = Optional.of(flavor);
             return this;
         }
 
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 3c16eac06c7..60b24ad8c0b 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
@@ -7,12 +7,9 @@ import com.yahoo.config.provision.HostName;
 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.NodeRepositoryNode;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState;
 
 import java.net.URI;
 import java.time.Duration;
-import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -26,12 +23,16 @@ import java.util.stream.Collectors;
  */
 public interface NodeRepository {
 
-    void addNodes(ZoneId zone, Collection<NodeRepositoryNode> nodes);
+    /** Add new nodes to the node repository */
+    void addNodes(ZoneId zone, List<Node> nodes);
 
+    /** Delete node */
     void deleteNode(ZoneId zone, String hostname);
 
-    void setState(ZoneId zone, NodeState nodeState, String hostname);
+    /** Move node to given state */
+    void setState(ZoneId zone, Node.State state, String hostname);
 
+    /** Get node from zone */
     Node getNode(ZoneId zone, String hostname);
 
     /** List all nodes in given zone */
@@ -50,17 +51,23 @@ public interface NodeRepository {
                                       .collect(Collectors.toList());
     }
 
+    /** Get node repository's view of given application */
     Application getApplication(ZoneId zone, ApplicationId application);
 
+    /** Update application */
     void patchApplication(ZoneId zone, ApplicationId application,
                           double currentReadShare, double maxReadShare);
 
+    /** Get node statistics such as cost and load from given zone */
     NodeRepoStats getStats(ZoneId zone);
 
+    /** Get all archive URLs found in zone */
     Map<TenantName, URI> getArchiveUris(ZoneId zone);
 
+    /** Update archive URL for given tenant */
     void setArchiveUri(ZoneId zone, TenantName tenantName, URI archiveUri);
 
+    /** Remove archive URL for given tenant */
     void removeArchiveUri(ZoneId zone, TenantName tenantName);
 
     /** Upgrade all nodes of given type to a new version */
@@ -78,13 +85,22 @@ public interface NodeRepository {
     /** Cancels firmware checks on all hosts in the given zone. */
     void cancelFirmwareCheck(ZoneId zone);
 
-    void retireAndDeprovision(ZoneId zoneId, String hostName);
+    /** Retire given node */
+    void retire(ZoneId zone, String hostname, boolean wantToRetire, boolean wantToDeprovision);
 
-    void patchNode(ZoneId zoneId, String hostName, NodeRepositoryNode node);
+    /** Update reports for given node. A key with null value clears that report */
+    void updateReports(ZoneId zone, String hostname, Map<String, String> reports);
 
-    void reboot(ZoneId zoneId, String hostName);
+    /** Update hardware model */
+    void updateModel(ZoneId zone, String hostname, String modelName);
+
+    /** Update switch hostname */
+    void updateSwitchHostname(ZoneId zone, String hostname, String switchHostname);
+
+    /** Schedule reboot of given node */
+    void reboot(ZoneId zone, String hostname);
 
     /** Checks whether the zone has the spare capacity to remove the given hosts */
-    boolean isReplaceable(ZoneId zoneId, List<HostName> hostNames);
+    boolean isReplaceable(ZoneId zone, List<HostName> hostnames);
 
 }
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 8ebfde9f475..fdd382e9007 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.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
@@ -18,29 +17,30 @@ import java.util.Set;
 import static com.yahoo.yolean.Exceptions.uncheck;
 
 /**
- * @author olaa
  *
  * Node repository report containing list of upcoming VCMRs impacting a node
+ *
+ * @author olaa
  */
 @JsonIgnoreProperties(ignoreUnknown = true)
-public class VCMRReport {
+public class VcmrReport {
 
     private static final String REPORT_ID = "vcmr";
     private static final ObjectMapper objectMapper = new ObjectMapper()
             .registerModule(new JavaTimeModule());
 
     @JsonProperty("upcoming")
-    private Set<VCMR> vcmrs;
+    private Set<Vcmr> vcmrs;
 
-    public VCMRReport() {
+    public VcmrReport() {
         this(new HashSet<>());
     }
 
-    public VCMRReport(Set<VCMR> vcmrs) {
+    public VcmrReport(Set<Vcmr> vcmrs) {
         this.vcmrs = vcmrs;
     }
 
-    public Set<VCMR> getVcmrs() {
+    public Set<Vcmr> getVcmrs() {
         return vcmrs;
     }
 
@@ -48,7 +48,7 @@ public class VCMRReport {
      * @return true if list of VCMRs is changed
      */
     public boolean addVcmr(String id, ZonedDateTime plannedStartTime, ZonedDateTime plannedEndtime) {
-        var vcmr = new VCMR(id, plannedStartTime, plannedEndtime);
+        var vcmr = new Vcmr(id, plannedStartTime, plannedEndtime);
         if (vcmrs.contains(vcmr))
             return false;
 
@@ -68,23 +68,23 @@ public class VCMRReport {
     /**
      * Serialization functions - mapped to {@link Node#reports()}
      */
-    public static VCMRReport fromReports(Map<String, String> reports) {
+    public static VcmrReport fromReports(Map<String, String> reports) {
         var serialized = reports.get(REPORT_ID);
         if (serialized == null)
-            return new VCMRReport();
+            return new VcmrReport();
 
-        return uncheck(() -> objectMapper.readValue(serialized, VCMRReport.class));
+        return uncheck(() -> objectMapper.readValue(serialized, VcmrReport.class));
     }
 
     /**
      * Set report to 'null' if list is empty - clearing the report
      * See NodePatcher in node-repository
      */
-    public Map<String, JsonNode> toNodeReports() {
-        Map<String, JsonNode> reports = new HashMap<>();
-        JsonNode jsonNode = vcmrs.isEmpty() ?
-                null : uncheck(() -> objectMapper.valueToTree(this));
-        reports.put(REPORT_ID, jsonNode);
+    public Map<String, String> toNodeReports() {
+        Map<String, String> reports = new HashMap<>();
+        String json = vcmrs.isEmpty() ?
+                null : uncheck(() -> objectMapper.valueToTree(this).toString());
+        reports.put(REPORT_ID, json);
         return reports;
     }
 
@@ -93,15 +93,15 @@ public class VCMRReport {
         return "VCMRReport{" + vcmrs + "}";
     }
 
-    public static class VCMR {
+    public static class Vcmr {
 
         private String id;
         private ZonedDateTime plannedStartTime;
         private ZonedDateTime plannedEndTime;
 
-        VCMR(@JsonProperty("id") String id,
-                          @JsonProperty("plannedStartTime") ZonedDateTime plannedStartTime,
-                          @JsonProperty("plannedEndTime") ZonedDateTime plannedEndTime) {
+        Vcmr(@JsonProperty("id") String id,
+             @JsonProperty("plannedStartTime") ZonedDateTime plannedStartTime,
+             @JsonProperty("plannedEndTime") ZonedDateTime plannedEndTime) {
             this.id = id;
             this.plannedStartTime = plannedStartTime;
             this.plannedEndTime = plannedEndTime;
@@ -123,7 +123,7 @@ public class VCMRReport {
         public boolean equals(Object o) {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
-            VCMR vcmr = (VCMR) o;
+            Vcmr vcmr = (Vcmr) o;
             return Objects.equals(id, vcmr.id) &&
                     Objects.equals(plannedStartTime, vcmr.plannedStartTime) &&
                     Objects.equals(plannedEndTime, vcmr.plannedEndTime);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
index 3c5495a6bfe..a8089555ffc 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
@@ -59,7 +59,7 @@ public class CloudEventReporter extends ControllerMaintainer {
                 if (!affects(node, event)) continue;
                 log.info("Retiring and deprovisioning " + node.hostname().value() + " in " + zone.getId() +
                          ": Affected by maintenance event " + event.instanceEventId);
-                nodeRepository.retireAndDeprovision(zone.getId(), node.hostname().value());
+                nodeRepository.retire(zone.getId(), node.hostname().value(), true, true);
             }
         }
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index 56bf870c7fc..668ca0f4ee1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -10,7 +10,6 @@ import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
 
 import java.time.Duration;
-import java.time.temporal.ChronoUnit;
 import java.time.temporal.TemporalUnit;
 import java.util.Collections;
 import java.util.List;
@@ -71,7 +70,7 @@ public class ControllerMaintenance extends AbstractComponent {
         maintainers.add(new ArchiveAccessMaintainer(controller, metric, intervals.archiveAccessMaintainer));
         maintainers.add(new TenantRoleMaintainer(controller, intervals.tenantRoleMaintainer));
         maintainers.add(new ChangeRequestMaintainer(controller, intervals.changeRequestMaintainer));
-        maintainers.add(new VCMRMaintainer(controller, intervals.vcmrMaintainer));
+        maintainers.add(new VcmrMaintainer(controller, intervals.vcmrMaintainer));
         maintainers.add(new CloudTrialExpirer(controller, intervals.defaultInterval));
         maintainers.add(new RetriggerMaintainer(controller, intervals.retriggerMaintainer));
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java
index 10e6f9eb039..5622fcdac4e 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/HostInfoUpdater.java
@@ -7,7 +7,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository;
 import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService;
 import com.yahoo.vespa.hosted.controller.api.integration.entity.NodeEntity;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode;
 
 import java.time.Duration;
 import java.util.EnumSet;
@@ -48,13 +47,24 @@ public class HostInfoUpdater extends ControllerMaintainer {
                 for (var node : nodeRepository.list(zone, false)) {
                     if (!node.type().isHost()) continue;
                     NodeEntity nodeEntity = nodeEntities.get(registeredHostnameOf(node));
-                    if (!shouldUpdateSwitch(node, nodeEntity) && !shouldUpdateModel(node, nodeEntity)) continue;
+                    if (nodeEntity == null) continue;
 
-                    NodeRepositoryNode updatedNode = new NodeRepositoryNode();
-                    nodeEntity.switchHostname().ifPresent(updatedNode::setSwitchHostname);
-                    buildModelName(nodeEntity).ifPresent(updatedNode::setModelName);
-                    nodeRepository.patchNode(zone, node.hostname().value(), updatedNode);
-                    hostsUpdated++;
+                    boolean updatedHost = false;
+                    Optional<String> modelName = modelNameOf(nodeEntity);
+                    if (modelName.isPresent() && !modelName.equals(node.modelName())) {
+                        nodeRepository.updateModel(zone, node.hostname().value(), modelName.get());
+                        updatedHost = true;
+                    }
+
+                    Optional<String> switchHostname = nodeEntity.switchHostname();
+                    if (switchHostname.isPresent() && !switchHostname.equals(node.switchHostname())) {
+                        nodeRepository.updateSwitchHostname(zone, node.hostname().value(), switchHostname.get());
+                        updatedHost = true;
+                    }
+
+                    if (updatedHost) {
+                        hostsUpdated++;
+                    }
                 }
             }
         } finally {
@@ -65,9 +75,8 @@ public class HostInfoUpdater extends ControllerMaintainer {
         return 1.0;
     }
 
-    private static Optional<String> buildModelName(NodeEntity nodeEntity) {
-        if(nodeEntity.manufacturer().isEmpty() || nodeEntity.model().isEmpty())
-            return Optional.empty();
+    private static Optional<String> modelNameOf(NodeEntity nodeEntity) {
+        if (nodeEntity.manufacturer().isEmpty() || nodeEntity.model().isEmpty()) return Optional.empty();
         return Optional.of(nodeEntity.manufacturer().get() + " " + nodeEntity.model().get());
     }
 
@@ -80,17 +89,4 @@ public class HostInfoUpdater extends ControllerMaintainer {
         return matcher.replaceFirst("$1$2");
     }
 
-    private static boolean shouldUpdateSwitch(Node node, NodeEntity nodeEntity) {
-        if (nodeEntity == null) return false;
-        if (nodeEntity.switchHostname().isEmpty())  return false;
-        return !node.switchHostname().equals(nodeEntity.switchHostname());
-    }
-
-    private static boolean shouldUpdateModel(Node node, NodeEntity nodeEntity) {
-        if (nodeEntity == null) return false;
-        if (nodeEntity.model().isEmpty())  return false;
-        if (nodeEntity.manufacturer().isEmpty()) return false;
-        return !node.modelName().equals(buildModelName(nodeEntity));
-    }
-
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainer.java
index 19617a1f293..69c9bd83ba5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainer.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.maintenance;
 
-import com.fasterxml.jackson.databind.JsonNode;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.SystemName;
@@ -10,14 +9,12 @@ import com.yahoo.text.Text;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository;
-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.vcmr.ChangeRequest;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequest.Impact;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequestClient;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.HostAction;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.HostAction.State;
-import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VCMRReport;
+import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VcmrReport;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VespaChangeRequest;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VespaChangeRequest.Status;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
@@ -35,22 +32,25 @@ import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 /**
- * @author olaa
  *
- * Maintains status and execution of VCMRs
- * For now only retires all affected tenant hosts if zone capacity allows it
+ * Maintains status and execution of Vespa CMRs.
+ *
+ * Currently this retires all affected tenant hosts if zone capacity allows it.
+ *
+ * @author olaa
  */
-public class VCMRMaintainer extends ControllerMaintainer {
+public class VcmrMaintainer extends ControllerMaintainer {
+
+    private static final Logger LOG = Logger.getLogger(VcmrMaintainer.class.getName());
+    private static final Duration ALLOWED_RETIREMENT_TIME = Duration.ofHours(60);
+    private static final Duration ALLOWED_POSTPONEMENT_TIME = Duration.ofDays(7);
 
-    private final Logger logger = Logger.getLogger(VCMRMaintainer.class.getName());
-    private final Duration ALLOWED_RETIREMENT_TIME = Duration.ofHours(60);
-    private final Duration ALLOWED_POSTPONEMENT_TIME = Duration.ofDays(7);
     private final CuratorDb curator;
     private final NodeRepository nodeRepository;
     private final ChangeRequestClient changeRequestClient;
     private final SystemName system;
 
-    public VCMRMaintainer(Controller controller, Duration interval) {
+    public VcmrMaintainer(Controller controller, Duration interval) {
         super(controller, interval, null, SystemName.allOf(Predicate.not(SystemName::isPublic)));
         this.curator = controller.curator();
         this.nodeRepository = controller.serviceRegistry().configServer().nodeRepository();
@@ -145,7 +145,7 @@ public class VCMRMaintainer extends ControllerMaintainer {
                 .orElse(new HostAction(node.hostname().value(), State.NONE, Instant.now()));
 
         if (changeRequest.getChangeRequestSource().isClosed()) {
-            logger.fine(() -> changeRequest.getChangeRequestSource().getId() + " is closed, recycling " + node.hostname());
+            LOG.fine(() -> changeRequest.getChangeRequestSource().getId() + " is closed, recycling " + node.hostname());
             recycleNode(changeRequest.getZoneId(), node, hostAction);
             removeReport(changeRequest, node);
             return hostAction.withState(State.COMPLETE);
@@ -157,7 +157,7 @@ public class VCMRMaintainer extends ControllerMaintainer {
         addReport(changeRequest, node);
 
         if (isPostponed(changeRequest, hostAction)) {
-            logger.fine(() -> changeRequest.getChangeRequestSource().getId() + " is postponed, recycling " + node.hostname());
+            LOG.fine(() -> changeRequest.getChangeRequestSource().getId() + " is postponed, recycling " + node.hostname());
             recycleNode(changeRequest.getZoneId(), node, hostAction);
             return hostAction.withState(State.PENDING_RETIREMENT);
         }
@@ -168,12 +168,12 @@ public class VCMRMaintainer extends ControllerMaintainer {
 
         if (shouldRetire(changeRequest, hostAction)) {
             if (!node.wantToRetire()) {
-                logger.info(Text.format("Retiring %s due to %s", node.hostname().value(), changeRequest.getChangeRequestSource().getId()));
+                LOG.info(Text.format("Retiring %s due to %s", node.hostname().value(), changeRequest.getChangeRequestSource().getId()));
                 // TODO: Remove try/catch once retirement is stabilized
                 try {
                     setWantToRetire(changeRequest.getZoneId(), node, true);
                 } catch (Exception e) {
-                    logger.warning("Failed to retire host " + node.hostname() + ": " + Exceptions.toMessageString(e));
+                    LOG.warning("Failed to retire host " + node.hostname() + ": " + Exceptions.toMessageString(e));
                     // Check if retirement actually failed
                     if (!nodeRepository.getNode(changeRequest.getZoneId(), node.hostname().value()).wantToRetire()) {
                         return hostAction;
@@ -184,12 +184,12 @@ public class VCMRMaintainer extends ControllerMaintainer {
         }
 
         if (hasRetired(node, hostAction)) {
-            logger.fine(() -> node.hostname() + " has retired");
+            LOG.fine(() -> node.hostname() + " has retired");
             return hostAction.withState(State.RETIRED);
         }
 
         if (pendingRetirement(node, hostAction)) {
-            logger.fine(() -> node.hostname() + " is pending retirement");
+            LOG.fine(() -> node.hostname() + " is pending retirement");
             return hostAction.withState(State.PENDING_RETIREMENT);
         }
 
@@ -200,8 +200,8 @@ public class VCMRMaintainer extends ControllerMaintainer {
     private void recycleNode(ZoneId zoneId, Node node, HostAction hostAction) {
         if (hostAction.getState() == State.RETIRED &&
                 node.state() == Node.State.parked) {
-            logger.info("Setting " + node.hostname() + " to dirty");
-            nodeRepository.setState(zoneId, NodeState.dirty, node.hostname().value());
+            LOG.info("Setting " + node.hostname() + " to dirty");
+            nodeRepository.setState(zoneId, Node.State.dirty, node.hostname().value());
         }
         if (hostAction.getState() == State.RETIRING && node.wantToRetire()) {
             try {
@@ -275,9 +275,7 @@ public class VCMRMaintainer extends ControllerMaintainer {
     }
 
     private void setWantToRetire(ZoneId zoneId, Node node, boolean wantToRetire) {
-        var newNode = new NodeRepositoryNode();
-        newNode.setWantToRetire(wantToRetire);
-        nodeRepository.patchNode(zoneId, node.hostname().value(), newNode);
+        nodeRepository.retire(zoneId, node.hostname().value(), wantToRetire, false);
     }
 
     private void approveChangeRequest(VespaChangeRequest changeRequest) {
@@ -288,12 +286,12 @@ public class VCMRMaintainer extends ControllerMaintainer {
         if (changeRequest.getApproval() != ChangeRequest.Approval.REQUESTED)
             return;
 
-        logger.info("Approving " + changeRequest.getChangeRequestSource().getId());
+        LOG.info("Approving " + changeRequest.getChangeRequestSource().getId());
         changeRequestClient.approveChangeRequest(changeRequest);
     }
 
     private void removeReport(VespaChangeRequest changeRequest, Node node) {
-        var report = VCMRReport.fromReports(node.reports());
+        var report = VcmrReport.fromReports(node.reports());
 
         if (report.removeVcmr(changeRequest.getChangeRequestSource().getId())) {
             updateReport(changeRequest.getZoneId(), node, report);
@@ -301,7 +299,7 @@ public class VCMRMaintainer extends ControllerMaintainer {
     }
 
     private void addReport(VespaChangeRequest changeRequest, Node node) {
-        var report = VCMRReport.fromReports(node.reports());
+        var report = VcmrReport.fromReports(node.reports());
 
         var source = changeRequest.getChangeRequestSource();
         if (report.addVcmr(source.getId(), source.getPlannedStartTime(), source.getPlannedEndTime())) {
@@ -309,10 +307,9 @@ public class VCMRMaintainer extends ControllerMaintainer {
         }
     }
 
-    private void updateReport(ZoneId zoneId, Node node, VCMRReport report) {
-        logger.info(Text.format("Updating report for %s: %s", node.hostname(), report));
-        var newNode = new NodeRepositoryNode();
-        newNode.setReports(report.toNodeReports());
-        nodeRepository.patchNode(zoneId, node.hostname().value(), newNode);
+    private void updateReport(ZoneId zoneId, Node node, VcmrReport report) {
+        LOG.info(Text.format("Updating report for %s: %s", node.hostname(), report));
+        nodeRepository.updateReports(zoneId, node.hostname().value(), report.toNodeReports());
     }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 1a4fa3fcd79..2b884cc950b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -876,7 +876,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
             node.reservedTo().ifPresent(tenant -> nodeObject.setString("reservedTo", tenant.value()));
             nodeObject.setString("orchestration", valueOf(node.serviceState()));
             nodeObject.setString("version", node.currentVersion().toString());
-            nodeObject.setString("flavor", node.flavor());
+            node.flavor().ifPresent(flavor -> nodeObject.setString("flavor", flavor));
             toSlime(node.resources(), nodeObject);
             nodeObject.setString("clusterId", node.clusterId());
             nodeObject.setString("clusterType", valueOf(node.clusterType()));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java
index b16817c0f3d..8a9aa6b4b1b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java
@@ -17,17 +17,12 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepoStats;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.TargetVersions;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode;
-import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState;
 
 import java.net.URI;
 import java.time.Duration;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.UnaryOperator;
@@ -46,50 +41,56 @@ public class NodeRepositoryMock implements NodeRepository {
     private final Map<DeploymentId, Pair<Double, Double>> trafficFractions = new HashMap<>();
     private final Map<ZoneId, Map<TenantName, URI>> archiveUris = new HashMap<>();
 
-    private boolean allowPatching = false;
+    private boolean allowPatching = true;
     private boolean hasSpareCapacity = false;
 
     @Override
-    public void addNodes(ZoneId zone, Collection<NodeRepositoryNode> nodes) {
-        throw new UnsupportedOperationException();
+    public void addNodes(ZoneId zone, List<Node> nodes) {
+        Map<HostName, Node> existingNodes = nodeRepository.getOrDefault(zone, Map.of());
+        for (var node : nodes) {
+            if (existingNodes.containsKey(node.hostname())) {
+                throw new IllegalArgumentException("Node " + node.hostname() + " already added in zone " + zone);
+            }
+        }
+        putNodes(zone, nodes);
     }
 
     @Override
     public void deleteNode(ZoneId zone, String hostname) {
-        throw new UnsupportedOperationException();
+        require(zone, hostname);
+        nodeRepository.get(zone).remove(HostName.from(hostname));
     }
 
     @Override
-    public void setState(ZoneId zone, NodeState nodeState, String hostName) {
-        var existing = list(zone, List.of(HostName.from(hostName)));
-        if (existing.size() != 1) throw new IllegalArgumentException("Node " + hostName + " not found in " + zone);
-
-        var node = Node.builder(existing.get(0))
-                       .state(Node.State.valueOf(nodeState.name()))
-                       .build();
+    public void setState(ZoneId zone, Node.State state, String hostname) {
+        Node node = Node.builder(require(zone, hostname))
+                        .state(Node.State.valueOf(state.name()))
+                        .build();
         putNodes(zone, node);
     }
 
     @Override
     public Node getNode(ZoneId zone, String hostname) {
-        throw new UnsupportedOperationException();
+        return require(zone, hostname);
     }
 
     @Override
     public List<Node> list(ZoneId zone, boolean includeDeprovisioned) {
-        return List.copyOf(nodeRepository.getOrDefault(zone, Map.of()).values());
+        return nodeRepository.getOrDefault(zone, Map.of()).values().stream()
+                             .filter(node -> includeDeprovisioned || node.state() != Node.State.deprovisioned)
+                             .collect(Collectors.toList());
     }
 
     @Override
     public List<Node> list(ZoneId zone, ApplicationId application) {
-        return nodeRepository.getOrDefault(zone, Collections.emptyMap()).values().stream()
+        return nodeRepository.getOrDefault(zone, Map.of()).values().stream()
                              .filter(node -> node.owner().map(application::equals).orElse(false))
                              .collect(Collectors.toList());
     }
 
     @Override
     public List<Node> list(ZoneId zone, List<HostName> hostnames) {
-        return nodeRepository.getOrDefault(zone, Collections.emptyMap()).values().stream()
+        return nodeRepository.getOrDefault(zone, Map.of()).values().stream()
                              .filter(node -> hostnames.contains(node.hostname()))
                              .collect(Collectors.toList());
     }
@@ -179,42 +180,36 @@ public class NodeRepositoryMock implements NodeRepository {
     }
 
     @Override
-    public void retireAndDeprovision(ZoneId zoneId, String hostName) {
-        nodeRepository.get(zoneId).remove(HostName.from(hostName));
+    public void retire(ZoneId zone, String hostname, boolean wantToRetire, boolean wantToDeprovision) {
+        patchNodes(zone, hostname, (node) -> Node.builder(node).wantToRetire(wantToRetire).wantToDeprovision(wantToDeprovision).build());
     }
 
     @Override
-    public void patchNode(ZoneId zoneId, String hostName, NodeRepositoryNode node) {
-        if (!allowPatching) throw new UnsupportedOperationException();
-        List<Node> existing = list(zoneId, List.of(HostName.from(hostName)));
-        if (existing.size() != 1) throw new IllegalArgumentException("Node " + hostName + " not found in " + zoneId);
-
-        // Note: Only supports switchHostname, modelName and wantToRetire
-        Node.Builder newNode = Node.builder(existing.get(0));
-        if (node.getSwitchHostname() != null)
-            newNode.switchHostname(node.getSwitchHostname());
-        if (node.getModelName() != null)
-            newNode.modelName(node.getModelName());
-        if (node.getWantToRetire() != null)
-            newNode.wantToRetire(node.getWantToRetire());
-
-        Map<String, String> reports = new HashMap<>();
-        for (var kv : node.getReports().entrySet()) {
-            if (kv.getValue() == null) continue; // Null value clears a report
-            reports.put(kv.getKey(), kv.getValue().toString());
-        }
-        newNode.reports(reports);
+    public void updateReports(ZoneId zone, String hostname, Map<String, String> reports) {
+        Map<String, String> trimmedReports = reports.entrySet().stream()
+                                                    // Null value clears a report
+                                                    .filter(kv -> kv.getValue() != null)
+                                                    .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+        patchNodes(zone, hostname, (node) -> Node.builder(node).reports(trimmedReports).build());
+    }
+
+    @Override
+    public void updateModel(ZoneId zone, String hostname, String modelName) {
+        patchNodes(zone, hostname, (node) -> Node.builder(node).modelName(modelName).build());
+    }
 
-        putNodes(zoneId, newNode.build());
+    @Override
+    public void updateSwitchHostname(ZoneId zone, String hostname, String switchHostname) {
+        patchNodes(zone, hostname, (node) -> Node.builder(node).switchHostname(switchHostname).build());
     }
 
     @Override
-    public void reboot(ZoneId zoneId, String hostName) {
+    public void reboot(ZoneId zone, String hostname) {
         throw new UnsupportedOperationException();
     }
 
     @Override
-    public boolean isReplaceable(ZoneId zoneId, List<HostName> hostNames) {
+    public boolean isReplaceable(ZoneId zone, List<HostName> hostnames) {
         return hasSpareCapacity;
     }
 
@@ -250,14 +245,6 @@ public class NodeRepositoryMock implements NodeRepository {
         nodeRepository.clear();
     }
 
-    public Node require(HostName hostName) {
-        return nodeRepository.values().stream()
-                             .map(zoneNodes -> zoneNodes.get(hostName))
-                             .filter(Objects::nonNull)
-                             .findFirst()
-                             .orElseThrow(() -> new NoSuchElementException("No node with the hostname " + hostName + " is known."));
-    }
-
     /** Add a fixed set of nodes to given zone */
     public void addFixedNodes(ZoneId zone) {
         var nodeA = Node.builder()
@@ -300,8 +287,7 @@ public class NodeRepositoryMock implements NodeRepository {
     }
 
     public void doUpgrade(DeploymentId deployment, Optional<HostName> hostName, Version version) {
-        modifyNodes(deployment, hostName, node -> {
-            assert node.wantedVersion().equals(version);
+        patchNodes(deployment, hostName, node -> {
             return Node.builder(node)
                        .currentVersion(version)
                        .currentDockerImage(node.wantedDockerImage())
@@ -309,38 +295,20 @@ public class NodeRepositoryMock implements NodeRepository {
         });
     }
 
-    private void modifyNodes(DeploymentId deployment, Optional<HostName> hostname, UnaryOperator<Node> modification) {
-        List<Node> nodes = hostname.map(this::require)
-                                   .map(Collections::singletonList)
-                                   .orElse(list(deployment.zoneId(), deployment.applicationId()));
-        putNodes(deployment.zoneId(),
-                 nodes.stream().map(modification).collect(Collectors.toList()));
-    }
-
     public void requestRestart(DeploymentId deployment, Optional<HostName> hostname) {
-        modifyNodes(deployment, hostname, node -> Node.builder(node).wantedRestartGeneration(node.wantedRestartGeneration() + 1).build());
+        patchNodes(deployment, hostname, node -> Node.builder(node).wantedRestartGeneration(node.wantedRestartGeneration() + 1).build());
     }
 
     public void doRestart(DeploymentId deployment, Optional<HostName> hostname) {
-        modifyNodes(deployment, hostname, node -> Node.builder(node).restartGeneration(node.restartGeneration() + 1).build());
+        patchNodes(deployment, hostname, node -> Node.builder(node).restartGeneration(node.restartGeneration() + 1).build());
     }
 
     public void requestReboot(DeploymentId deployment, Optional<HostName> hostname) {
-        modifyNodes(deployment, hostname, node -> Node.builder(node).wantedRebootGeneration(node.wantedRebootGeneration() + 1).build());
+        patchNodes(deployment, hostname, node -> Node.builder(node).wantedRebootGeneration(node.wantedRebootGeneration() + 1).build());
     }
 
     public void doReboot(DeploymentId deployment, Optional<HostName> hostname) {
-        modifyNodes(deployment, hostname, node -> Node.builder(node).rebootGeneration(node.rebootGeneration() + 1).build());
-    }
-
-    public void addReport(ZoneId zoneId, HostName hostName, String reportId, String report) {
-        Node node = nodeRepository.getOrDefault(zoneId, Map.of()).get(hostName);
-        if (node == null) throw new IllegalArgumentException("No node named " + hostName + " in " + zoneId);
-
-        Map<String, String> reports = new HashMap<>(node.reports());
-        reports.put(reportId, report);
-        Node newNode = Node.builder(node).reports(reports).build();
-        putNodes(zoneId, newNode);
+        patchNodes(deployment, hostname, node -> Node.builder(node).rebootGeneration(node.rebootGeneration() + 1).build());
     }
 
     public NodeRepositoryMock allowPatching(boolean allowPatching) {
@@ -352,4 +320,33 @@ public class NodeRepositoryMock implements NodeRepository {
         this.hasSpareCapacity = hasSpareCapacity;
     }
 
+    private Node require(ZoneId zone, String hostname) {
+        return require(zone, HostName.from(hostname));
+    }
+
+    private Node require(ZoneId zone, HostName hostname) {
+        Node node = nodeRepository.getOrDefault(zone, Map.of()).get(hostname);
+        if (node == null) throw new IllegalArgumentException("Node not found in " + zone + ": " + hostname);
+        return node;
+    }
+
+    private void patchNodes(ZoneId zone, String hostname, UnaryOperator<Node> patcher) {
+        patchNodes(zone, Optional.of(HostName.from(hostname)), patcher);
+    }
+
+    private void patchNodes(DeploymentId deployment, Optional<HostName> hostname, UnaryOperator<Node> patcher) {
+        patchNodes(deployment.zoneId(), hostname, patcher);
+    }
+
+    private void patchNodes(ZoneId zone, Optional<HostName> hostname, UnaryOperator<Node> patcher) {
+        if (!allowPatching) throw new UnsupportedOperationException("Patching is disabled in this mock");
+        List<Node> nodes;
+        if (hostname.isPresent()) {
+            nodes = List.of(require(zone, hostname.get()));
+        } else {
+            nodes = list(zone, false);
+        }
+        putNodes(zone, nodes.stream().map(patcher).collect(Collectors.toList()));
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
index e838a693be1..bb10f29de72 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
@@ -42,15 +42,15 @@ public class CloudEventReporterTest {
     public void maintain() {
         setUpZones();
         CloudEventReporter cloudEventReporter = new CloudEventReporter(tester.controller(), Duration.ofMinutes(15));
-        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), getHostnames(unsupportedZone.getId()));
-        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), getHostnames(zone1.getId()));
-        assertEquals(Set.of("host4.com", "host5.com", "confighost.com"), getHostnames(zone2.getId()));
+        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), hostsNotDeprovisioning(unsupportedZone.getId()));
+        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), hostsNotDeprovisioning(zone1.getId()));
+        assertEquals(Set.of("host4.com", "host5.com", "confighost.com"), hostsNotDeprovisioning(zone2.getId()));
 
         mockEvents();
         cloudEventReporter.maintain();
-        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), getHostnames(unsupportedZone.getId()));
-        assertEquals(Set.of("host3.com"), getHostnames(zone1.getId()));
-        assertEquals(Set.of("host4.com"), getHostnames(zone2.getId()));
+        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), hostsNotDeprovisioning(unsupportedZone.getId()));
+        assertEquals(Set.of("host3.com"), hostsNotDeprovisioning(zone1.getId()));
+        assertEquals(Set.of("host4.com"), hostsNotDeprovisioning(zone2.getId()));
     }
 
     private void mockEvents() {
@@ -127,11 +127,12 @@ public class CloudEventReporterTest {
                    .build();
     }
 
-    private Set<String> getHostnames(ZoneId zoneId) {
+    private Set<String> hostsNotDeprovisioning(ZoneId zoneId) {
         return tester.configServer().nodeRepository().list(zoneId, false)
-                .stream()
-                .map(node -> node.hostname().value())
-                .collect(Collectors.toSet());
+                     .stream()
+                     .filter(node -> !node.wantToDeprovision())
+                     .map(node -> node.hostname().value())
+                     .collect(Collectors.toSet());
     }
 
     private ZoneApiMock createZone(String zoneId, String cloudNativeRegionName, String cloud) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainerTest.java
index f957b14ef95..1d66434ea42 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainerTest.java
@@ -9,7 +9,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequest;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.ChangeRequestSource;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.HostAction;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.HostAction.State;
-import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VCMRReport;
+import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VcmrReport;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VespaChangeRequest;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VespaChangeRequest.Status;
 import com.yahoo.vespa.hosted.controller.integration.NodeRepositoryMock;
@@ -30,10 +30,10 @@ import static org.junit.Assert.assertTrue;
 /**
  * @author olaa
  */
-public class VCMRMaintainerTest {
+public class VcmrMaintainerTest {
 
     private ControllerTester tester;
-    private VCMRMaintainer maintainer;
+    private VcmrMaintainer maintainer;
     private NodeRepositoryMock nodeRepo;
     private final ZoneId zoneId = ZoneId.from("prod.us-east-3");
     private final HostName host1 = HostName.from("host1");
@@ -43,13 +43,13 @@ public class VCMRMaintainerTest {
     @Before
     public void setup() {
         tester = new ControllerTester();
-        maintainer = new VCMRMaintainer(tester.controller(), Duration.ofMinutes(1));
+        maintainer = new VcmrMaintainer(tester.controller(), Duration.ofMinutes(1));
         nodeRepo = tester.serviceRegistry().configServer().nodeRepository().allowPatching(true);
     }
 
     @Test
     public void recycle_hosts_after_completion() {
-        var vcmrReport = new VCMRReport();
+        var vcmrReport = new VcmrReport();
         vcmrReport.addVcmr("id123", ZonedDateTime.now(), ZonedDateTime.now());
         var parkedNode = createNode(host1, NodeType.host, Node.State.parked, true);
         var failedNode = createNode(host2, NodeType.host, Node.State.failed, false);
@@ -169,7 +169,7 @@ public class VCMRMaintainerTest {
         assertEquals(1, approvedChangeRequests.size());
 
         activeNode = nodeRepo.list(zoneId, List.of(host2)).get(0);
-        var report = VCMRReport.fromReports(activeNode.reports());
+        var report = VcmrReport.fromReports(activeNode.reports());
         var reportAdded = report.getVcmrs().stream()
                         .filter(vcmr -> vcmr.getId().equals(changeRequestId))
                         .count() == 1;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
index 668baa50cc1..3a7e6e63574 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
@@ -100,7 +100,7 @@
       "name": "Upgrader"
     },
     {
-      "name": "VCMRMaintainer"
+      "name": "VcmrMaintainer"
     },
     {
       "name": "VersionStatusUpdater"