diff options
Diffstat (limited to 'node-repository/src/main/java/com')
3 files changed, 91 insertions, 41 deletions
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 68bdffd75f9..7f595b4a541 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 @@ -467,29 +467,88 @@ public class NodeRepository extends AbstractComponent { } } + /* + * This method is used to enable a smooth rollout of dynamic docker flavor allocations. Once we have switch + * everything this can be simplified to only deleting the node. + * + * Should only be called by node-admin for docker containers + */ + public List<Node> markNodeAvailableForNewAllocation(String hostname) { + Node node = getNode(hostname).orElseThrow(() -> new NotFoundException("No node with hostname \"" + hostname + '"')); + if (node.flavor().getType() != Flavor.Type.DOCKER_CONTAINER) { + throw new IllegalArgumentException( + "Cannot make " + hostname + " available for new allocation, must be a docker container node"); + } else if (node.state() != Node.State.dirty) { + throw new IllegalArgumentException( + "Cannot make " + hostname + " available for new allocation, must be in state dirty, but was in " + node.state()); + } + + if (dynamicAllocationEnabled()) { + return removeRecursively(node, true); + } else { + return setReady(Collections.singletonList(node)); + } + } + /** - * Removes a node. A node must be in a legal state before it can be removed. + * Removes all the nodes that are children of hostname before finally removing the hostname itself. + * + * @return List of all the nodes that have been removed */ - public void remove(String hostname) { - Node nodeToRemove = getNode(hostname).orElseThrow(() -> new NotFoundException("No node with hostname \"" + hostname + '"')); - List<Node.State> legalStates = dynamicAllocationEnabled() ? - Arrays.asList(Node.State.provisioned, Node.State.failed, Node.State.parked, Node.State.dirty) : - Arrays.asList(Node.State.provisioned, Node.State.failed, Node.State.parked); - - if (! legalStates.contains(nodeToRemove.state())) { - throw new IllegalArgumentException("Can only remove node from following states: " + - legalStates.stream().map(Node.State::name).collect(Collectors.joining(", "))); + public List<Node> removeRecursively(String hostname) { + Node node = getNode(hostname).orElseThrow(() -> new NotFoundException("No node with hostname \"" + hostname + '"')); + return removeRecursively(node, false); + } + + private List<Node> removeRecursively(Node node, boolean force) { + try (Mutex lock = lockUnallocated()) { + List<Node> removed = node.type() != NodeType.host ? + new ArrayList<>() : + getChildNodes(node.hostname()).stream() + .filter(child -> force || verifyRemovalIsAllowed(child, true)) + .collect(Collectors.toList()); + + if (force || verifyRemovalIsAllowed(node, false)) removed.add(node); + db.removeNodes(removed); + + return removed; + } catch (RuntimeException e) { + throw new IllegalArgumentException("Failed to delete " + node.hostname(), e); } + } - if (nodeToRemove.state().equals(Node.State.dirty)) { - if (!(nodeToRemove.flavor().getType().equals(Flavor.Type.DOCKER_CONTAINER))) { - throw new IllegalArgumentException("Only docker nodes can be deleted from state dirty"); + /** + * Allowed to a node delete if: + * Non-docker-container node: iff in state provisioned|failed|parked + * Docker-container-node: + * If only removing the container node: node in state ready + * If also removing the parent node: child is in state provisioned|failed|parked|ready + */ + private boolean verifyRemovalIsAllowed(Node nodeToRemove, boolean deletingAsChild) { + // TODO: Enable once controller no longer deletes child nodes manually + /*if (nodeToRemove.flavor().getType() == Flavor.Type.DOCKER_CONTAINER && !deletingAsChild) { + if (nodeToRemove.state() != Node.State.ready) { + throw new IllegalArgumentException( + String.format("Docker container node %s can only be removed when in state ready", nodeToRemove.hostname())); } - } - try (Mutex lock = lock(nodeToRemove)) { - db.removeNode(nodeToRemove.state(), hostname); + } else */ if (nodeToRemove.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) { + List<Node.State> legalStates = Arrays.asList(Node.State.provisioned, Node.State.failed, Node.State.parked, Node.State.ready); + + if (! legalStates.contains(nodeToRemove.state())) { + throw new IllegalArgumentException(String.format("Child node %s can only be removed from following states: %s", + nodeToRemove.hostname(), legalStates.stream().map(Node.State::name).collect(Collectors.joining(", ")))); + } + } else { + List<Node.State> legalStates = Arrays.asList(Node.State.provisioned, Node.State.failed, Node.State.parked); + + if (! legalStates.contains(nodeToRemove.state())) { + throw new IllegalArgumentException(String.format("Node %s can only be removed from following states: %s", + nodeToRemove.hostname(), legalStates.stream().map(Node.State::name).collect(Collectors.joining(", ")))); + } } + + return true; } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index 8f218d7e6fc..d38c9179986 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -106,18 +106,21 @@ public class CuratorDatabaseClient { } /** - * Removes a node. + * Removes multiple nodes in a single transaction. * - * @param state the current state of the node - * @param hostName the host name of the node to remove + * @param nodes list of the nodes to remove */ - public void removeNode(Node.State state, String hostName) { - Path path = toPath(state, hostName); + public void removeNodes(List<Node> nodes) { NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); - curatorTransaction.add(CuratorOperations.delete(path.getAbsolute())); + + for (Node node : nodes) { + Path path = toPath(node.state(), node.hostname()); + CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction); + curatorTransaction.add(CuratorOperations.delete(path.getAbsolute())); + } + transaction.commit(); - log.log(LogLevel.INFO, "Removed: " + state + " node " + hostName); + nodes.forEach(node -> log.log(LogLevel.INFO, "Removed node " + node.hostname() + " in state " + node.state())); } /** 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 4e7bb1f7d16..b16ce5f818e 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 @@ -125,22 +125,10 @@ public class NodesApiHandler extends LoggingRequestHandler { 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 (remember to allow deletion of dirty nodes then). - * - * Should only be called by node-admin for docker containers (the docker constraint is - * enforced in the remove method) - */ String hostname = lastElement(path); - if (nodeRepository.dynamicAllocationEnabled()) { - nodeRepository.remove(hostname); - return new MessageResponse("Removed " + hostname); - } else { - nodeRepository.setReady(hostname); - return new MessageResponse("Moved " + hostname + " to ready"); - } + List<Node> available = nodeRepository.markNodeAvailableForNewAllocation(hostname); + return new MessageResponse("Marked following nodes as available for new allocation: " + + available.stream().map(Node::hostname).collect(Collectors.joining(", "))); } throw new NotFoundException("Cannot put to path '" + path + "'"); @@ -180,8 +168,8 @@ public class NodesApiHandler extends LoggingRequestHandler { String path = request.getUri().getPath(); if (path.startsWith("/nodes/v2/node/")) { String hostname = lastElement(path); - nodeRepository.remove(hostname); - return new MessageResponse("Removed " + hostname); + List<Node> removedNodes = nodeRepository.removeRecursively(hostname); + return new MessageResponse("Removed " + removedNodes.stream().map(Node::hostname).collect(Collectors.joining(", "))); } else if (path.startsWith("/nodes/v2/maintenance/inactive/")) { return setActive(lastElement(path), true); |