diff options
author | Martin Polden <mpolden@mpolden.no> | 2021-04-15 12:36:58 +0200 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2021-04-15 12:38:10 +0200 |
commit | 0ca7682b346e9e78cfe1e410fcc11ccb97d8b029 (patch) | |
tree | 24a6e43e6ae99fe75d8ebf833c8037f7347ceec3 | |
parent | 205027ee37315c838faa63bb303671f4fd0145b3 (diff) |
Remove unnecessary restore feature
5 files changed, 5 insertions, 75 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index bde0343174a..b468c4a31d7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -199,21 +199,6 @@ public class Nodes { return setReady(List.of(nodeToReady), agent, reason).get(0); } - /** Restore a node that has been rebuilt */ - public Node restore(String hostname, Agent agent, String reason) { - // A deprovisioned host has no children so this doesn't need to to be recursive - try (NodeMutex lock = lockAndGetRequired(hostname)) { - Node node = lock.node(); - if (node.state() != Node.State.deprovisioned) illegal("Can not move node " + hostname + " to " + - Node.State.provisioned + ". It is not in " + - Node.State.deprovisioned); - if (!node.status().wantToRebuild()) illegal("Can not move node " + hostname + " to " + - Node.State.provisioned + - ". Rebuild has not been requested"); - return db.writeTo(Node.State.provisioned, node, agent, Optional.of(reason)); - } - } - /** Reserve nodes. This method does <b>not</b> lock the node repository */ public List<Node> reserve(List<Node> nodes) { return db.writeTo(Node.State.reserved, nodes, Agent.application, Optional.empty()); @@ -511,9 +496,7 @@ public class Nodes { if (zone.getCloud().dynamicProvisioning()) db.removeNodes(List.of(node)); else { - if (!node.status().wantToRebuild()) { // Keep IP addresses if we're rebuilding - node = node.with(IP.Config.EMPTY); - } + node = node.with(IP.Config.EMPTY); move(node, Node.State.deprovisioned, Agent.system, Optional.empty()); } removed.add(node); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java index f5fb7948251..7deaf36ba17 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java @@ -159,10 +159,6 @@ public class NodesV2ApiHandler extends LoggingRequestHandler { List<Node> breakfixedNodes = nodeRepository.nodes().breakfixRecursively(path.get("hostname"), Agent.operator, "Breakfixed through the nodes/v2 API"); return new MessageResponse("Moved " + hostnamesAsString(breakfixedNodes) + " to " + Node.State.breakfixed); } - else if (path.matches("/nodes/v2/state/provisioned/{hostname}")) { - Node restoredNode = nodeRepository.nodes().restore(path.get("hostname"), Agent.operator, "Restored through the nodes/v2 API"); - return new MessageResponse("Moved " + hostnamesAsString(List.of(restoredNode)) + " to " + Node.State.provisioned); - } throw new NotFoundException("Cannot put to path '" + path + "'"); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java index f4cd5f5eeb5..567e71c7f9e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java @@ -18,7 +18,6 @@ import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -219,47 +218,6 @@ public class NodeRepositoryTest { } @Test - public void restore_rebuilt_host() { - NodeRepositoryTester tester = new NodeRepositoryTester(); - assertEquals(0, tester.nodeRepository().nodes().list().size()); - - String host1 = "host1"; - String host2 = "host2"; - tester.addHost("id1", host1, "default", NodeType.host); - tester.addHost("id2", host2, "default", NodeType.host); - assertEquals(2, tester.nodeRepository().nodes().list().size()); - - // One host is requested to rebuild, two hosts are parked - tester.nodeRepository().nodes().rebuild(host2, Agent.system, tester.clock().instant()); - tester.nodeRepository().nodes().park(host1, false, Agent.system, getClass().getSimpleName()); - tester.nodeRepository().nodes().park(host2, false, Agent.system, getClass().getSimpleName()); - IP.Config ipConfigOfHost2 = tester.nodeRepository().nodes().node(host2).get().ipConfig(); - - // Two hosts are removed - tester.nodeRepository().nodes().removeRecursively(host1); - tester.nodeRepository().nodes().removeRecursively(host2); - assertEquals(2, tester.nodeRepository().nodes().list(Node.State.deprovisioned).size()); - - // Host not rebuilding cannot be restored - try { - tester.nodeRepository().nodes().restore(host1, Agent.system, getClass().getSimpleName()); - fail("Expected exception"); - } catch (IllegalArgumentException ignored) {} - - // Other host is restored - Node node = tester.nodeRepository().nodes().restore(host2, Agent.system, getClass().getSimpleName()); - assertSame(Node.State.provisioned, node.state()); - assertEquals("IP addresses are preserved", ipConfigOfHost2, node.ipConfig()); - assertTrue(node.status().wantToRetire()); - assertTrue(node.status().wantToRebuild()); - - // Readying host clears rebuild flag - node = tester.nodeRepository().nodes().setReady(host2, Agent.system, getClass().getSimpleName()); - assertFalse(node.status().wantToRetire()); - assertFalse(node.status().wantToRebuild()); - } - - @Test public void dirty_host_only_if_we_can_dirty_children() { NodeRepositoryTester tester = new NodeRepositoryTester(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java index f3e5778cb60..923abc72977 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java @@ -389,7 +389,9 @@ public class OsVersionsTest { tester.nodeRepository().nodes().park(node.hostname(), false, Agent.system, getClass().getSimpleName()); tester.nodeRepository().nodes().removeRecursively(node.hostname()); - node = tester.nodeRepository().nodes().restore(node.hostname(), Agent.system, getClass().getSimpleName()); + Node newNode = Node.create(node.id(), node.ipConfig(), node.hostname(), node.flavor(), node.type()) + .build(); + node = tester.nodeRepository().nodes().addNodes(List.of(newNode), Agent.system).get(0); node = tester.nodeRepository().nodes().setReady(node.hostname(), Agent.system, getClass().getSimpleName()); tester.prepareAndActivateInfraApplication(infraApplication, nodeType); node = tester.nodeRepository().nodes().node(node.hostname()).get(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java index a259607ef58..f545f996fd2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java @@ -235,22 +235,13 @@ public class NodesV2ApiTest { assertFile(new Request("http://localhost:8080/nodes/v2/node/host4.yahoo.com"), "node4-after-changes.json"); - // move a host marked as wantToRebuild to deprovisioned + // park and remove host assertResponse(new Request("http://localhost:8080/nodes/v2/state/parked/dockerhost1.yahoo.com", new byte[0], Request.Method.PUT), "{\"message\":\"Moved dockerhost1.yahoo.com to parked\"}"); assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost1.yahoo.com", new byte[0], Request.Method.DELETE), "{\"message\":\"Removed dockerhost1.yahoo.com\"}"); - // ... and then restore it - assertResponse(new Request("http://localhost:8080/nodes/v2/state/provisioned/dockerhost1.yahoo.com", - new byte[0], Request.Method.PUT), - "{\"message\":\"Moved dockerhost1.yahoo.com to provisioned\"}"); - - // move a host to deprovisioned - assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost1.yahoo.com", - new byte[0], Request.Method.DELETE), - "{\"message\":\"Removed dockerhost1.yahoo.com\"}"); // ... and then forget it completely assertResponse(new Request("http://localhost:8080/nodes/v2/node/dockerhost1.yahoo.com", new byte[0], Request.Method.DELETE), |