Make hardwareFailureType a String

7 files changed, 26 insertions, 72 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
index 35e2a102737..49916dad4aa 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Status.java
@@ -18,26 +18,15 @@ public class Status {
     private final Generation reboot;
     private final Optional<Version> vespaVersion;
     private final int failCount;
-    private final Optional<HardwareFailureType> hardwareFailure;
+    private final Optional<String> hardwareFailure;
     private final boolean wantToRetire;
     private final boolean wantToDeprovision;
     private final Optional<String> hardwareDivergence;
-    
-    public enum HardwareFailureType {
-        
-        /** There are mce log error messages */
-        memory_mcelog,
-        /** There are smart log error messages */
-        disk_smart,
-        /** There are kernel log error messages */
-        disk_kernel
-        
-    }
 
     public Status(Generation generation,
                   Optional<Version> vespaVersion,
                   int failCount,
-                  Optional<HardwareFailureType> hardwareFailure,
+                  Optional<String> hardwareFailure,
                   boolean wantToRetire,
                   boolean wantToDeprovision,
                   Optional<String> hardwareDivergence) {
@@ -76,10 +65,10 @@ public class Status {
     /** Returns how many times this node has been moved to the failed state. */
     public int failCount() { return failCount; }
 
-    public Status withHardwareFailure(Optional<HardwareFailureType> hardwareFailure) { return new Status(reboot, vespaVersion, failCount, hardwareFailure, wantToRetire, wantToDeprovision, hardwareDivergence); }
+    public Status withHardwareFailure(Optional<String> hardwareFailure) { return new Status(reboot, vespaVersion, failCount, hardwareFailure, wantToRetire, wantToDeprovision, hardwareDivergence); }
 
     /** Returns the type of the last hardware failure detected on this node, or empty if none */
-    public Optional<HardwareFailureType> hardwareFailure() { return hardwareFailure; }
+    public Optional<String> hardwareFailure() { return hardwareFailure; }
 
     /** Returns a copy of this with the want to retire flag changed */
     public Status withWantToRetire(boolean wantToRetire) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
index 9f876cf5b14..77ffd7415b3 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
@@ -106,7 +106,7 @@ public class NodeSerializer {
         object.setLong(currentRebootGenerationKey, node.status().reboot().current());
         node.status().vespaVersion().ifPresent(version -> object.setString(vespaVersionKey, version.toString()));
         object.setLong(failCountKey, node.status().failCount());
-        node.status().hardwareFailure().ifPresent(failure -> object.setString(hardwareFailureKey, toString(failure)));
+        node.status().hardwareFailure().ifPresent(failure -> object.setString(hardwareFailureKey, failure));
         object.setBool(wantToRetireKey, node.status().wantToRetire());
         object.setBool(wantToDeprovisionKey, node.status().wantToDeprovision());
         node.allocation().ifPresent(allocation -> toSlime(allocation, object.setObject(instanceKey)));
@@ -168,7 +168,7 @@ public class NodeSerializer {
         return new Status(generationFromSlime(object, rebootGenerationKey, currentRebootGenerationKey),
                           versionFromSlime(object.field(vespaVersionKey)),
                           (int)object.field(failCountKey).asLong(),
-                          hardwareFailureFromSlime(object.field(hardwareFailureKey)),
+                          hardwareFailureFromSlime(object),
                           object.field(wantToRetireKey).asBool(),
                           wantToDeprovision,
                           hardwareDivergenceFromSlime(object));
@@ -245,9 +245,11 @@ public class NodeSerializer {
         return ipAddresses.build();
     }
 
-    private Optional<Status.HardwareFailureType> hardwareFailureFromSlime(Inspector object) {
-        if ( ! object.valid()) return Optional.empty();
-        return Optional.of(hardwareFailureFromString(object.asString()));
+    private Optional<String> hardwareFailureFromSlime(Inspector object) {
+        if (object.field(hardwareFailureKey).valid()) {
+            return Optional.of(object.field(hardwareFailureKey).asString());
+        }
+        return Optional.empty();
     }
 
     // Enum <-> string mappings
@@ -324,21 +326,4 @@ public class NodeSerializer {
         throw new IllegalArgumentException("Serialized form of '" + type + "' not defined");
     }
 
-    private Status.HardwareFailureType hardwareFailureFromString(String hardwareFailureString) {
-        switch (hardwareFailureString) {
-            case "memory_mcelog" : return Status.HardwareFailureType.memory_mcelog;
-            case "disk_smart" : return Status.HardwareFailureType.disk_smart;
-            case "disk_kernel" : return Status.HardwareFailureType.disk_kernel;
-            default : throw new IllegalArgumentException("Unknown hardware failure '" + hardwareFailureString + "'");
-        }
-    }
-    private String toString(Status.HardwareFailureType type) {
-        switch (type) {
-            case memory_mcelog: return "memory_mcelog";
-            case disk_smart: return "disk_smart";
-            case disk_kernel: return "disk_kernel";
-            default : throw new IllegalArgumentException("Serialized form of '" + type + " not defined");
-        }
-    }
-
 }
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 07e005a3601..fd0992d19c6 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
@@ -12,7 +12,6 @@ import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Allocation;
-import com.yahoo.vespa.hosted.provision.node.Status;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -80,9 +79,11 @@ public class NodePatcher {
         boolean modified = false;
 
         if (inspector.field(HARDWARE_FAILURE_TYPE).valid()) {
+            Optional<String> hardwareFailure = inspector.type().equals(Type.NIX) ?
+                    Optional.empty() : Optional.of(inspector.field(HARDWARE_FAILURE_TYPE).asString());
             modified = true;
             children = children.stream()
-                    .map(node -> node.with(node.status().withHardwareFailure(toHardwareFailureType(asString(inspector.field(HARDWARE_FAILURE_TYPE))))))
+                    .map(node -> node.with(node.status().withHardwareFailure(hardwareFailure)))
                     .collect(Collectors.toList());
         }
 
@@ -113,7 +114,9 @@ public class NodePatcher {
             case "flavor" :
                 return node.with(nodeFlavors.getFlavorOrThrow(asString(value)));
             case HARDWARE_FAILURE_TYPE:
-                return node.with(node.status().withHardwareFailure(toHardwareFailureType(asString(value))));
+                Optional<String> hardwareFailure = value.type().equals(Type.NIX) ?
+                        Optional.empty() : Optional.of(asString(value));
+                return node.with(node.status().withHardwareFailure(hardwareFailure));
             case "parentHostname" :
                 return node.withParentHostname(asString(value));
             case "ipAddresses" :
@@ -125,10 +128,9 @@ public class NodePatcher {
             case "wantToDeprovision" :
                 return node.with(node.status().withWantToDeprovision(asBoolean(value)));
             case "hardwareDivergence" :
-                if (value.type().equals(Type.NIX)) {
-                    return node.with(node.status().withHardwareDivergence(Optional.empty()));
-                }
-                return node.with(node.status().withHardwareDivergence(Optional.of(asString(value))));
+                Optional<String> hardwareDivergence = value.type().equals(Type.NIX) ?
+                        Optional.empty() : Optional.of(asString(value));
+                return node.with(node.status().withHardwareDivergence(hardwareDivergence));
             default :
                 throw new IllegalArgumentException("Could not apply field '" + name + "' on a node: No such modifiable field");
         }
@@ -175,14 +177,4 @@ public class NodePatcher {
         return field.asBool();
     }
 
-    private Optional<Status.HardwareFailureType> toHardwareFailureType(String failureType) {
-        switch (failureType) {
-            case "memory_mcelog" : return Optional.of(Status.HardwareFailureType.memory_mcelog);
-            case "disk_smart" : return Optional.of(Status.HardwareFailureType.disk_smart);
-            case "disk_kernel" : return Optional.of(Status.HardwareFailureType.disk_kernel);
-            case "unknown" : throw new IllegalArgumentException("An actual hardware failure type must be provided, not 'unknown'");
-            default : throw new IllegalArgumentException("Unknown hardware failure '" + failureType + "'");
-        }
-    }
-
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
index 699782e3a93..d5ab5610980 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
@@ -172,7 +172,7 @@ class NodesResponse extends HttpResponse {
                 });
         object.setLong("failCount", node.status().failCount());
         object.setBool("hardwareFailure", node.status().hardwareFailure().isPresent());
-        node.status().hardwareFailure().ifPresent(failure -> object.setString("hardwareFailureType", toString(failure)));
+        node.status().hardwareFailure().ifPresent(failure -> object.setString("hardwareFailureType", failure));
         object.setBool("wantToRetire", node.status().wantToRetire());
         object.setBool("wantToDeprovision", node.status().wantToDeprovision());
         toSlime(node.history(), object.setArray("history"));
@@ -231,13 +231,4 @@ class NodesResponse extends HttpResponse {
                 .orElseThrow(() -> new RuntimeException("Node state '" + stateString + "' is not known"));
     }
 
-    private String toString(Status.HardwareFailureType type) {
-        switch (type) {
-            case memory_mcelog: return "memory_mcelog";
-            case disk_smart: return "disk_smart";
-            case disk_kernel: return "disk_kernel";
-            default : throw new IllegalArgumentException("Serialized form of '" + type + " not defined");
-        }
-    }
-
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java
index b9b33efd92b..7ae06b08450 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java
@@ -24,7 +24,6 @@ import com.yahoo.vespa.curator.transaction.CuratorTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
-import com.yahoo.vespa.hosted.provision.node.Status;
 import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder;
 import com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner;
 import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester;
@@ -116,7 +115,7 @@ public class FailedExpirerTest {
 
         // Set node2 to have a detected hardware failure
         Node node2 = nodeRepository.getNode("node2").get();
-        node2 = node2.with(node2.status().withHardwareFailure(Optional.of(Status.HardwareFailureType.memory_mcelog)));
+        node2 = node2.with(node2.status().withHardwareFailure(Optional.of("memory_mcelog")));
         nodeRepository.write(node2);
 
         // Allocate the nodes
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java
index 59c0de7d785..4750da1d43e 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java
@@ -6,7 +6,6 @@ import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
-import com.yahoo.vespa.hosted.provision.node.Status;
 import com.yahoo.vespa.orchestrator.ApplicationIdNotFoundException;
 import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException;
 import org.junit.Test;
@@ -70,8 +69,8 @@ public class NodeFailerTest {
         // Failures are detected on two ready nodes, which are then failed
         Node readyFail1 = tester.nodeRepository.getNodes(NodeType.tenant, Node.State.ready).get(2);
         Node readyFail2 = tester.nodeRepository.getNodes(NodeType.tenant, Node.State.ready).get(3);
-        tester.nodeRepository.write(readyFail1.with(readyFail1.status().withHardwareFailure(Optional.of(Status.HardwareFailureType.memory_mcelog))));
-        tester.nodeRepository.write(readyFail2.with(readyFail2.status().withHardwareFailure(Optional.of(Status.HardwareFailureType.disk_smart))));
+        tester.nodeRepository.write(readyFail1.with(readyFail1.status().withHardwareFailure(Optional.of("memory_mcelog"))));
+        tester.nodeRepository.write(readyFail2.with(readyFail2.status().withHardwareFailure(Optional.of("disk_smart"))));
         assertEquals(4, tester.nodeRepository.getNodes(NodeType.tenant, Node.State.ready).size());
         tester.failer.run();
         assertEquals(2, tester.nodeRepository.getNodes(NodeType.tenant, Node.State.ready).size());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java
index 05ce848bb79..e28e257686f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/SerializationTest.java
@@ -18,7 +18,6 @@ import com.yahoo.vespa.hosted.provision.Node.State;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.Generation;
 import com.yahoo.vespa.hosted.provision.node.History;
-import com.yahoo.vespa.hosted.provision.node.Status;
 import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder;
 import org.junit.Test;
 
@@ -74,7 +73,7 @@ public class SerializationTest {
         node = node.with(FlavorConfigBuilder.createDummies("large").getFlavorOrThrow("large"));
         node = node.with(node.status().withVespaVersion(Version.fromString("1.2.3")));
         node = node.with(node.status().withIncreasedFailCount().withIncreasedFailCount());
-        node = node.with(node.status().withHardwareFailure(Optional.of(Status.HardwareFailureType.memory_mcelog)));
+        node = node.with(node.status().withHardwareFailure(Optional.of("memory_mcelog")));
         node = node.with(NodeType.tenant);
         Node copy = nodeSerializer.fromJson(Node.State.provisioned, nodeSerializer.toJson(node));
 
@@ -88,7 +87,7 @@ public class SerializationTest {
         assertEquals("large", copy.flavor().name());
         assertEquals("1.2.3", copy.status().vespaVersion().get().toString());
         assertEquals(2, copy.status().failCount());
-        assertEquals(Status.HardwareFailureType.memory_mcelog, copy.status().hardwareFailure().get());
+        assertEquals("memory_mcelog", copy.status().hardwareFailure().get());
         assertEquals(node.allocation().get().owner(), copy.allocation().get().owner());
         assertEquals(node.allocation().get().membership(), copy.allocation().get().membership());
         assertEquals(node.allocation().get().isRemovable(), copy.allocation().get().isRemovable());