From 767d21202ed98f1b0103dfc109554d6a0a1e1bff Mon Sep 17 00:00:00 2001 From: toby Date: Sun, 21 May 2017 22:24:52 +0200 Subject: Add rest call to set node in ready - featureflag to switch between old and new dynamic behavior. Allow docker nodes to be deleted from state dirty. --- .../hosted/node/admin/nodeagent/NodeAgentImpl.java | 2 +- .../node/admin/noderepository/NodeRepository.java | 2 +- .../admin/noderepository/NodeRepositoryImpl.java | 10 ++++++---- .../admin/integrationTests/MultiDockerTest.java | 2 +- .../node/admin/integrationTests/NodeRepoMock.java | 4 ++-- .../node/admin/nodeagent/NodeAgentImplTest.java | 2 +- .../noderepository/NodeRepositoryImplTest.java | 6 +++--- .../vespa/hosted/provision/NodeRepository.java | 15 ++++++++++++--- .../provision/restapi/v2/NodesApiHandler.java | 22 ++++++++++++++++++---- 9 files changed, 45 insertions(+), 20 deletions(-) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index d8ee3550272..535a4b6677d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -207,7 +207,7 @@ public class NodeAgentImpl implements NodeAgent { .withRebootGeneration(nodeSpec.wantedRebootGeneration.orElse(0L)) .withDockerImage(new DockerImage("")) .withVespaVersion("")); - nodeRepository.markAsReady(nodeSpec.hostname); + nodeRepository.markNodeAvailableForNewAllocation(nodeSpec.hostname); } private void updateNodeRepoWithCurrentAttributes(final ContainerNodeSpec nodeSpec) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepository.java index 8bd55878c64..b2173f31387 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepository.java @@ -23,5 +23,5 @@ public interface NodeRepository { void markAsDirty(String hostName); - void markAsReady(String hostName); + void markNodeAvailableForNewAllocation(String hostName); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java index b03c07717c4..9c1c648941d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java @@ -155,15 +155,17 @@ public class NodeRepositoryImpl implements NodeRepository { @Override public void markAsDirty(String hostName) { - markNodeToState(hostName, Node.State.dirty); + // This will never happen once the new allocation scheme is rolled out. + markNodeToState(hostName, Node.State.dirty.name()); } @Override - public void markAsReady(final String hostName) { - markNodeToState(hostName, Node.State.ready); + public void markNodeAvailableForNewAllocation(final String hostName) { + // TODO replace with call to delete node when everything has been migrated to dynamic docker allocation + markNodeToState(hostName, "availablefornewallocations"); } - private void markNodeToState(String hostName, Node.State state) { + private void markNodeToState(String hostName, String state) { NodeMessageResponse response = requestExecutor.put( "/nodes/v2/state/" + state + "/" + hostName, port, diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java index f497d7cac41..ba3fe71637d 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/MultiDockerTest.java @@ -56,7 +56,7 @@ public class MultiDockerTest { callOrderVerifier.assertInOrder( "updateNodeAttributes with HostName: host1.test.yahoo.com, NodeAttributes{restartGeneration=1, rebootGeneration=0, dockerImage=image1, vespaVersion=''}", "updateNodeAttributes with HostName: host2.test.yahoo.com, NodeAttributes{restartGeneration=1, rebootGeneration=0, dockerImage=image2, vespaVersion=''}", - "markAsReady with HostName: host2.test.yahoo.com", + "markNodeAvailableForNewAllocation with HostName: host2.test.yahoo.com", "updateNodeAttributes with HostName: host3.test.yahoo.com, NodeAttributes{restartGeneration=1, rebootGeneration=0, dockerImage=image1, vespaVersion=''}"); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java index 9848c80d78a..e0aa2281436 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/NodeRepoMock.java @@ -79,7 +79,7 @@ public class NodeRepoMock implements NodeRepository { } @Override - public void markAsReady(String hostName) { + public void markNodeAvailableForNewAllocation(String hostName) { Optional cns = getContainerNodeSpec(hostName); synchronized (monitor) { @@ -90,7 +90,7 @@ public class NodeRepoMock implements NodeRepository { .nodeFlavor("docker") .build()); } - callOrderVerifier.add("markAsReady with HostName: " + hostName); + callOrderVerifier.add("markNodeAvailableForNewAllocation with HostName: " + hostName); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 12479db8ba3..512b5ae2913 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -334,7 +334,7 @@ public class NodeAgentImplTest { inOrder.verify(storageMaintainer, times(1)).removeOldFilesFromNode(eq(containerName)); inOrder.verify(dockerOperations, times(1)).removeContainer(any()); inOrder.verify(storageMaintainer, times(1)).archiveNodeData(eq(containerName)); - inOrder.verify(nodeRepository, times(1)).markAsReady(eq(hostName)); + inOrder.verify(nodeRepository, times(1)).markNodeAvailableForNewAllocation(eq(hostName)); verify(dockerOperations, never()).startContainer(eq(containerName), any()); verify(orchestrator, never()).resume(any(String.class)); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java index 5c6717075dc..15dce02f336 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java @@ -149,17 +149,17 @@ public class NodeRepositoryImplTest { NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "dockerhost4"); waitForJdiscContainerToServe(); - nodeRepositoryApi.markAsReady("host55.yahoo.com"); + nodeRepositoryApi.markNodeAvailableForNewAllocation("host55.yahoo.com"); try { - nodeRepositoryApi.markAsReady("host1.yahoo.com"); + nodeRepositoryApi.markNodeAvailableForNewAllocation("host1.yahoo.com"); fail("Expected failure because host1 is not registered as provisioned, dirty, failed or parked"); } catch (RuntimeException ignored) { // expected } try { - nodeRepositoryApi.markAsReady("host101.yahoo.com"); + nodeRepositoryApi.markNodeAvailableForNewAllocation("host101.yahoo.com"); fail("Expected failure because host101 does not exist"); } catch (RuntimeException ignored) { // expected diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 0b4d290a340..ede9d818823 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -451,14 +451,23 @@ public class NodeRepository extends AbstractComponent { } /** - * Removes a node. A node must be in the provisioned, failed or parked state before it can be removed. + * Removes a node. A node must be in a legal state before it can be removed. * - * @return true if the node was removed, false if it was not found in one of these states + * @return true if the node was removed, false if it was not found in one of the legal states */ public boolean remove(String hostname) { - Optional nodeToRemove = getNode(hostname, Node.State.provisioned, Node.State.failed, Node.State.parked); + + Node.State[] legalStates = {Node.State.provisioned, Node.State.failed, Node.State.parked}; + Node.State[] legalDynamicStates = {Node.State.provisioned, Node.State.failed, Node.State.parked, Node.State.dirty}; + + Optional nodeToRemove = getNode(hostname, dynamicAllocationEnabled() ? legalDynamicStates : legalStates); if ( ! nodeToRemove.isPresent()) return false; + // Only docker nodes are allowed to be deleted in state dirty. + if ( nodeToRemove.get().state().equals(Node.State.dirty)) { + if (!(nodeToRemove.get().flavor().getType().equals(Flavor.Type.DOCKER_CONTAINER))) return false; + } + try (Mutex lock = lock(nodeToRemove.get())) { return db.removeNode(nodeToRemove.get().state(), hostname); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java index a9ef14adbd9..5194614c9b3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java @@ -73,10 +73,10 @@ public class NodesApiHandler extends LoggingRequestHandler { case PATCH: return handlePATCH(request); default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported"); } - } + } catch (NotFoundException | NoSuchNodeException e) { return ErrorResponse.notFoundError(Exceptions.toMessageString(e)); - } + } catch (IllegalArgumentException e) { return ErrorResponse.badRequest(Exceptions.toMessageString(e)); } @@ -124,6 +124,20 @@ public class NodesApiHandler extends LoggingRequestHandler { nodeRepository.reactivate(lastElement(path), Agent.operator); return new MessageResponse("Moved " + lastElement(path) + " to active"); } + else if (path.startsWith("/nodes/v2/state/availablefornewallocations/")) { + /** + * This is a temporary "state" or rest call that we use to enable a smooth rollout of + * dynamic docker flavor allocations. Once we have switch everything we remove this + * and change the code in the nodeadmin to delete directly. + */ + if (nodeRepository.dynamicAllocationEnabled()) { + nodeRepository.remove(lastElement(path)); + return new MessageResponse("Removed " + lastElement(path)); + } else { + nodeRepository.setReady(lastElement(path)); + return new MessageResponse("Moved " + lastElement(path) + " to ready"); + } + } throw new NotFoundException("Cannot put to path '" + path + "'"); } @@ -141,11 +155,11 @@ public class NodesApiHandler extends LoggingRequestHandler { if (path.equals("/nodes/v2/command/restart")) { int restartCount = nodeRepository.restart(toNodeFilter(request)).size(); return new MessageResponse("Scheduled restart of " + restartCount + " matching nodes"); - } + } else if (path.equals("/nodes/v2/command/reboot")) { int rebootCount = nodeRepository.reboot(toNodeFilter(request)).size(); return new MessageResponse("Scheduled reboot of " + rebootCount + " matching nodes"); - } + } else if (path.equals("/nodes/v2/node")) { int addedNodes = addNodes(request.getData()); return new MessageResponse("Added " + addedNodes + " nodes to the provisioned state"); -- cgit v1.2.3