diff options
author | valerijf <valerijf@oath.com> | 2017-08-29 14:39:22 +0200 |
---|---|---|
committer | valerijf <valerijf@oath.com> | 2017-08-29 14:39:22 +0200 |
commit | 8af3bc056d712a3a1bc6b99bef3ddf5be3b89e94 (patch) | |
tree | 9ab53d71cf41ddedca43e898f342730a02eb5786 /node-repository/src/test/java | |
parent | 5f098c9e716d478764284756ed76071192ff0787 (diff) |
Implement recursive delete in node-repo
Diffstat (limited to 'node-repository/src/test/java')
5 files changed, 56 insertions, 14 deletions
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 bcae9d293f6..f7168e132f2 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 @@ -28,18 +28,18 @@ public class NodeRepositoryTest { @Test public void nodeRepositoryTest() { NodeRepositoryTester tester = new NodeRepositoryTester(); - assertEquals(0, tester.getNodes(NodeType.tenant).size()); + assertEquals(0, tester.nodeRepository().getNodes().size()); tester.addNode("id1", "host1", "default", NodeType.tenant); tester.addNode("id2", "host2", "default", NodeType.tenant); tester.addNode("id3", "host3", "default", NodeType.tenant); - assertEquals(3, tester.getNodes(NodeType.tenant).size()); + assertEquals(3, tester.nodeRepository().getNodes().size()); tester.nodeRepository().park("host2", Agent.system, "Parking to unit test"); - tester.nodeRepository().remove("host2"); + tester.nodeRepository().removeRecursively("host2"); - assertEquals(2, tester.getNodes(NodeType.tenant).size()); + assertEquals(2, tester.nodeRepository().getNodes().size()); } @Test @@ -70,20 +70,57 @@ public class NodeRepositoryTest { } @Test - public void only_allow_to_delete_dirty_nodes_when_dynamic_allocation_feature_enabled() { + public void only_allow_docker_containers_remove_in_provisioned_or_ready() { NodeRepositoryTester tester = new NodeRepositoryTester(); - tester.addNode("id1", "host1", "default", NodeType.host); + tester.addNode("id1", "host1", "docker", NodeType.tenant); tester.addNode("id2", "host2", "docker", NodeType.tenant); + tester.nodeRepository().fail("host2", Agent.system, "Failed for testing"); + + tester.nodeRepository().removeRecursively("host1"); // host1 is in state provisioned + try { + tester.nodeRepository().removeRecursively("host2"); + fail("Should not be able to delete docker tenant node in state dirty"); + } catch (IllegalArgumentException ignored) { + // Expected + } + tester.nodeRepository().setDirty("host2"); + tester.nodeRepository().setReady("host2"); + tester.nodeRepository().removeRecursively("host2"); + } + + @Test + public void delete_host_only_after_all_the_children_have_been_deleted() { + NodeRepositoryTester tester = new NodeRepositoryTester(); + + tester.addNode("id1", "host1", "default", NodeType.host); + tester.addNode("id2", "host2", "default", NodeType.host); + tester.addNode("node10", "node10", "host1", "docker", NodeType.tenant); + tester.addNode("node11", "node11", "host1", "docker", NodeType.tenant); + tester.addNode("node12", "node12", "host1", "docker", NodeType.tenant); + tester.addNode("node20", "node20", "host2", "docker", NodeType.tenant); + assertEquals(6, tester.nodeRepository().getNodes().size()); + + tester.nodeRepository().setDirty("node11"); try { - tester.nodeRepository().remove("host2"); - fail("Should not be able to delete tenant node in state dirty"); + tester.nodeRepository().removeRecursively("host1"); + fail("Should not be able to delete host node, one of the children are in state dirty"); } catch (IllegalArgumentException ignored) { // Expected } + assertEquals(6, tester.nodeRepository().getNodes().size()); - tester.curator().set(Path.fromString("/provision/v1/dynamicDockerAllocation"), new byte[0]); - tester.nodeRepository().remove("host2"); + // Should be OK to delete host2 as both host2 and its only child, node20, are in state provisioned + tester.nodeRepository().removeRecursively("host2"); + assertEquals(4, tester.nodeRepository().getNodes().size()); + + // Now node10 and node12 are in provisioned, set node11 to ready, and it should be OK to delete host1 + tester.nodeRepository().setReady("node11"); + tester.nodeRepository().removeRecursively("node12"); // Remove one of the children first instead + assertEquals(3, tester.nodeRepository().getNodes().size()); + + tester.nodeRepository().removeRecursively("host1"); + assertEquals(0, tester.nodeRepository().getNodes().size()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java index 784fc1a274a..3d01bde4291 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java @@ -51,6 +51,12 @@ public class NodeRepositoryTester { return nodeRepository.addNodes(Collections.singletonList(node)).get(0); } + public Node addNode(String id, String hostname, String parentHostname, String flavor, NodeType type) { + Node node = nodeRepository.createNode(id, hostname, Optional.of(parentHostname), + nodeFlavors.getFlavorOrThrow(flavor), type); + return nodeRepository.addNodes(Collections.singletonList(node)).get(0); + } + private FlavorsConfig createConfig() { FlavorConfigBuilder b = new FlavorConfigBuilder(); b.addFlavor("default", 2., 4., 100, Flavor.Type.BARE_METAL).cost(3); 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 e304aac5463..8fd67f949d9 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 @@ -342,7 +342,7 @@ public class NodeFailerTest { assertEquals(15, tester.nodeRepository.getNodes(NodeType.proxy, Node.State.active).size()); // The first down host is removed, which causes the second one to be moved to failed - tester.nodeRepository.remove(failedHost1); + tester.nodeRepository.removeRecursively(failedHost1); tester.failer.run(); assertEquals( 2, tester.deployer.redeployments); assertEquals(14, tester.nodeRepository.getNodes(NodeType.proxy, Node.State.active).size()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java index cb46d0a4624..bba5aa2db8d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.NodeType; -import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepositoryTester; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.zookeeper.ZooKeeperServer; @@ -49,7 +48,7 @@ public class ZooKeeperAccessMaintainerTest { assertEquals(asSet("host1,host2,host3,host4,host5,server1,server2"), ZooKeeperServer.getAllowedClientHostnames()); tester.nodeRepository().park("host2", Agent.system, "Parking to unit test"); - tester.nodeRepository().remove("host2"); + tester.nodeRepository().removeRecursively("host2"); maintainer.maintain(); assertEquals(2, tester.getNodes(NodeType.tenant).size()); 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 565dfe0457e..40ae1e1c2c5 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 @@ -401,7 +401,7 @@ public class RestApiTest { // Attempt to DELETE a node which is not put in a deletable state first assertResponse(new Request("http://localhost:8080/nodes/v2/node/host2.yahoo.com", new byte[0], Request.Method.DELETE), - 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can only remove node from following states: provisioned, failed, parked, dirty\"}"); + 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Failed to delete host2.yahoo.com: host2.yahoo.com can only be removed from following states: provisioned, failed, parked\"}"); // PUT current restart generation with string instead of long assertResponse(new Request("http://localhost:8080/nodes/v2/node/host4.yahoo.com", |