Merge pull request #5890 from vespa-engine/freva/simplify-node-patcher

Simplify NodePatcher

2 files changed, 22 insertions, 25 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
index a5c6176ee53..fd9ee237046 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java
@@ -65,7 +65,7 @@ public class NodePatcher {
     public List<Node> apply() {
         inspector.traverse((String name, Inspector value) -> {
             try {
-                node = applyField(name, value);
+                node = applyField(node, name, value);
             } catch (IllegalArgumentException e) {
                 throw new IllegalArgumentException("Could not set field '" + name + "'", e);
             }
@@ -87,17 +87,10 @@ public class NodePatcher {
     private List<Node> applyFieldRecursive(List<Node> childNodes, String name, Inspector value) {
         switch (name) {
             case HARDWARE_FAILURE_DESCRIPTION:
-                return childNodes.stream()
-                        .map(child -> child.with(child.status().withHardwareFailureDescription(asOptionalString(value))))
-                        .collect(Collectors.toList());
             case WANT_TO_RETIRE:
-                return childNodes.stream()
-                        .map(child -> child.with(child.status().withWantToRetire(asBoolean(value))))
-                        .collect(Collectors.toList());
-
             case WANT_TO_DEPROVISION:
                 return childNodes.stream()
-                        .map(child -> child.with(child.status().withWantToDeprovision(asBoolean(value))))
+                        .map(child -> applyField(child, name, value))
                         .collect(Collectors.toList());
 
             default :
@@ -105,7 +98,7 @@ public class NodePatcher {
         }
     }
 
-    private Node applyField(String name, Inspector value) {
+    private Node applyField(Node node, String name, Inspector value) {
         switch (name) {
             case "currentRebootGeneration" :
                 return node.withCurrentRebootGeneration(asLong(value), nodeRepository.clock().instant());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index 24ed065d736..b481edf0ac3 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -438,19 +438,24 @@ public class RestApiTest {
 
     @Test
     public void test_hardware_patching_of_docker_host() throws Exception {
-        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), Optional.of(false));
-        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com"), Optional.of(false));
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), false);
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com"), false);
 
         assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com",
-                        Utf8.toBytes("{" +
-                                "\"hardwareFailureDescription\": \"memory_mcelog\"" +
-                                "}"
-                        ),
+                        Utf8.toBytes("{\"hardwareFailureDescription\": \"memory_mcelog\"}"),
+                        Request.Method.PATCH),
+                "{\"message\":\"Updated dockerhost2.yahoo.com\"}");
+
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), true);
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com"), true);
+
+        assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com",
+                        Utf8.toBytes("{\"hardwareFailureDescription\": \"null\"}"),
                         Request.Method.PATCH),
                 "{\"message\":\"Updated dockerhost2.yahoo.com\"}");
 
-        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), Optional.of(true));
-        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com"), Optional.of(true));
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/host5.yahoo.com"), false);
+        assertHardwareFailure(new Request("http://localhost:8080/nodes/v2/node/dockerhost2.yahoo.com"), false);
     }
 
     @Test
@@ -581,20 +586,19 @@ public class RestApiTest {
                 "],";
     }
 
-    private Optional<Boolean> getHardwareFailure(String json) {
+    private boolean getHardwareFailure(String json) {
         Slime slime = SlimeUtils.jsonToSlime(json.getBytes());
         Cursor hardwareFailure = slime.get().field("hardwareFailure");
-        if (!hardwareFailure.valid()) {
-            return Optional.empty();
-        }
+        if (!hardwareFailure.valid())
+            throw new IllegalStateException("hardwareFailure is invalid");
 
-        return Optional.of(hardwareFailure.asBool());
+        return hardwareFailure.asBool();
     }
 
-    private void assertHardwareFailure(Request request, Optional<Boolean> expectedHardwareFailure) throws CharacterCodingException {
+    private void assertHardwareFailure(Request request, boolean expectedHardwareFailure) throws CharacterCodingException {
         Response response = container.handleRequest(request);
         String json = response.getBodyAsString();
-        Optional<Boolean> actualHardwareFailure = getHardwareFailure(json);
+        boolean actualHardwareFailure = getHardwareFailure(json);
         assertEquals(expectedHardwareFailure, actualHardwareFailure);
         assertEquals(200, response.getStatus());
     }