From 2ca0e3be242332ee66277001da35c278f2bf3a4f Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 9 Mar 2020 12:39:58 +0100 Subject: Simplify, no functional changes --- .../com/yahoo/vespa/hosted/provision/Node.java | 6 +- .../vespa/hosted/provision/NodeRepository.java | 129 ++++++++++----------- .../vespa/hosted/provision/NodeRepositoryTest.java | 1 + 3 files changed, 68 insertions(+), 68 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index f881f888752..bbff263cb7e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -390,10 +390,10 @@ public final class Node { @Override public String toString() { - return state + " node " + + return state + + ( parentHostname.isPresent() ? " child node " : " host " ) + hostname + - (allocation.map(allocation1 -> " " + allocation1).orElse("")) + - (parentHostname.map(parent -> " [on: " + parent + "]").orElse("")); + ( allocation.isPresent() ? " " + allocation : ""); } public enum State { 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 7d925b2a4aa..630bc0153b8 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 @@ -5,6 +5,7 @@ import com.google.inject.Inject; import com.yahoo.collections.ListMap; import com.yahoo.component.AbstractComponent; import com.yahoo.component.Version; +import com.yahoo.vespa.hosted.provision.Node.State; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Flavor; @@ -82,7 +83,7 @@ import java.util.stream.Stream; // 1) (new) - > provisioned -> (dirty ->) ready -> reserved -> active -> inactive -> dirty -> ready // 2) inactive -> reserved | parked // 3) reserved -> dirty -// 3) * -> failed | parked -> dirty | active | (removed) +// 3) * -> failed | parked -> dirty | active | deprovisioned // Nodes have an application assigned when in states reserved, active and inactive. // Nodes might have an application assigned in dirty. public class NodeRepository extends AbstractComponent { @@ -125,7 +126,7 @@ public class NodeRepository extends AbstractComponent { this.jobControl = new JobControl(db); // read and write all nodes to make sure they are stored in the latest version of the serialized format - for (Node.State state : Node.State.values()) + for (State state : State.values()) db.writeTo(state, db.getNodes(state), Agent.system, Optional.empty()); } @@ -162,7 +163,7 @@ public class NodeRepository extends AbstractComponent { * @param inState the states the node may be in. If no states are given, it will be returned from any state * @return the node, or empty if it was not found in any of the given states */ - public Optional getNode(String hostname, Node.State ... inState) { + public Optional getNode(String hostname, State ... inState) { return db.getNode(hostname, inState); } @@ -172,7 +173,7 @@ public class NodeRepository extends AbstractComponent { * @param inState the states to return nodes from. If no states are given, all nodes of the given type are returned * @return the node, or empty if it was not found in any of the given states */ - public List getNodes(Node.State ... inState) { + public List getNodes(State ... inState) { return new ArrayList<>(db.getNodes(inState)); } /** @@ -182,7 +183,7 @@ public class NodeRepository extends AbstractComponent { * @param inState the states to return nodes from. If no states are given, all nodes of the given type are returned * @return the node, or empty if it was not found in any of the given states */ - public List getNodes(NodeType type, Node.State ... inState) { + public List getNodes(NodeType type, State ... inState) { return db.getNodes(inState).stream().filter(node -> node.type().equals(type)).collect(Collectors.toList()); } @@ -210,9 +211,9 @@ public class NodeRepository extends AbstractComponent { return LoadBalancerList.copyOf(db.readLoadBalancers(predicate).values()); } - public List getNodes(ApplicationId id, Node.State ... inState) { return db.getNodes(id, inState); } - public List getInactive() { return db.getNodes(Node.State.inactive); } - public List getFailed() { return db.getNodes(Node.State.failed); } + public List getNodes(ApplicationId id, State ... inState) { return db.getNodes(id, inState); } + public List getInactive() { return db.getNodes(State.inactive); } + public List getFailed() { return db.getNodes(State.failed); } /** * Returns the ACL for the node (trusted nodes, networks and ports) @@ -253,7 +254,7 @@ public class NodeRepository extends AbstractComponent { node.allocation().ifPresent(allocation -> trustedNodes.addAll(candidates.parentsOf(candidates.owner(allocation.owner()).asList()).asList())); - if (node.state() == Node.State.ready) { + if (node.state() == State.ready) { // Tenant nodes in state ready, trust: // - All tenant nodes in zone. When a ready node is allocated to a an application there's a brief // window where current ACLs have not yet been applied on the node. To avoid service disruption @@ -348,7 +349,7 @@ public class NodeRepository extends AbstractComponent { existing.get() + ", " + existing.get().history() + "). Node to be added: " + node + ", " + node.history()); } - return db.addNodesInState(nodes.asList(), Node.State.reserved); + return db.addNodesInState(nodes.asList(), State.reserved); } /** Adds a list of (newly created) nodes to the node repository as provisioned nodes */ @@ -367,7 +368,7 @@ public class NodeRepository extends AbstractComponent { if (node.equals(other)) throw new IllegalArgumentException(message); } } - return db.addNodesInState(IP.Config.verify(nodes, list(lock)), Node.State.provisioned); + return db.addNodesInState(IP.Config.verify(nodes, list(lock)), State.provisioned); } } @@ -376,13 +377,13 @@ public class NodeRepository extends AbstractComponent { try (Mutex lock = lockUnallocated()) { List nodesWithResetFields = nodes.stream() .map(node -> { - if (node.state() != Node.State.provisioned && node.state() != Node.State.dirty) + if (node.state() != State.provisioned && node.state() != State.dirty) throw new IllegalArgumentException("Can not set " + node + " ready. It is not provisioned or dirty."); return node.with(node.status().withWantToRetire(false).withWantToDeprovision(false)); }) .collect(Collectors.toList()); - return db.writeTo(Node.State.ready, nodesWithResetFields, agent, Optional.of(reason)); + return db.writeTo(State.ready, nodesWithResetFields, agent, Optional.of(reason)); } } @@ -390,18 +391,18 @@ public class NodeRepository extends AbstractComponent { Node nodeToReady = getNode(hostname).orElseThrow(() -> new NoSuchNodeException("Could not move " + hostname + " to ready: Node not found")); - if (nodeToReady.state() == Node.State.ready) return nodeToReady; + if (nodeToReady.state() == State.ready) return nodeToReady; return setReady(Collections.singletonList(nodeToReady), agent, reason).get(0); } /** Reserve nodes. This method does not lock the node repository */ public List reserve(List nodes) { - return db.writeTo(Node.State.reserved, nodes, Agent.application, Optional.empty()); + return db.writeTo(State.reserved, nodes, Agent.application, Optional.empty()); } /** Activate nodes. This method does not lock the node repository */ public List activate(List nodes, NestedTransaction transaction) { - return db.writeTo(Node.State.active, nodes, Agent.application, Optional.empty(), transaction); + return db.writeTo(State.active, nodes, Agent.application, Optional.empty(), transaction); } /** @@ -421,7 +422,7 @@ public class NodeRepository extends AbstractComponent { public void deactivate(ApplicationId application, NestedTransaction transaction) { try (Mutex lock = lock(application)) { - deactivate(db.getNodes(application, Node.State.reserved, Node.State.active), transaction); + deactivate(db.getNodes(application, State.reserved, State.active), transaction); } } @@ -431,7 +432,7 @@ public class NodeRepository extends AbstractComponent { * This method does not lock */ public List deactivate(List nodes, NestedTransaction transaction) { - return db.writeTo(Node.State.inactive, nodes, Agent.application, Optional.empty(), transaction); + return db.writeTo(State.inactive, nodes, Agent.application, Optional.empty(), transaction); } /** Move nodes to the dirty state */ @@ -446,7 +447,7 @@ public class NodeRepository extends AbstractComponent { * @throws IllegalArgumentException if the node has hardware failure */ public Node setDirty(Node node, Agent agent, String reason) { - return db.writeTo(Node.State.dirty, node, agent, Optional.of(reason)); + return db.writeTo(State.dirty, node, agent, Optional.of(reason)); } public List dirtyRecursively(String hostname, Agent agent, String reason) { @@ -457,13 +458,13 @@ public class NodeRepository extends AbstractComponent { (nodeToDirty.type().isDockerHost() ? Stream.concat(list().childrenOf(hostname).asList().stream(), Stream.of(nodeToDirty)) : Stream.of(nodeToDirty)) - .filter(node -> node.state() != Node.State.dirty) + .filter(node -> node.state() != State.dirty) .collect(Collectors.toList()); List hostnamesNotAllowedToDirty = nodesToDirty.stream() - .filter(node -> node.state() != Node.State.provisioned) - .filter(node -> node.state() != Node.State.failed) - .filter(node -> node.state() != Node.State.parked) + .filter(node -> node.state() != State.provisioned) + .filter(node -> node.state() != State.failed) + .filter(node -> node.state() != State.parked) .map(Node::hostname) .collect(Collectors.toList()); if (!hostnamesNotAllowedToDirty.isEmpty()) { @@ -483,7 +484,7 @@ public class NodeRepository extends AbstractComponent { * @throws NoSuchNodeException if the node is not found */ public Node fail(String hostname, Agent agent, String reason) { - return move(hostname, true, Node.State.failed, agent, Optional.of(reason)); + return move(hostname, true, State.failed, agent, Optional.of(reason)); } /** @@ -492,7 +493,7 @@ public class NodeRepository extends AbstractComponent { * @return List of all the failed nodes in their new state */ public List failRecursively(String hostname, Agent agent, String reason) { - return moveRecursively(hostname, Node.State.failed, agent, Optional.of(reason)); + return moveRecursively(hostname, State.failed, agent, Optional.of(reason)); } /** @@ -502,7 +503,7 @@ public class NodeRepository extends AbstractComponent { * @throws NoSuchNodeException if the node is not found */ public Node park(String hostname, boolean keepAllocation, Agent agent, String reason) { - return move(hostname, keepAllocation, Node.State.parked, agent, Optional.of(reason)); + return move(hostname, keepAllocation, State.parked, agent, Optional.of(reason)); } /** @@ -511,7 +512,7 @@ public class NodeRepository extends AbstractComponent { * @return List of all the parked nodes in their new state */ public List parkRecursively(String hostname, Agent agent, String reason) { - return moveRecursively(hostname, Node.State.parked, agent, Optional.of(reason)); + return moveRecursively(hostname, State.parked, agent, Optional.of(reason)); } /** @@ -521,10 +522,10 @@ public class NodeRepository extends AbstractComponent { * @throws NoSuchNodeException if the node is not found */ public Node reactivate(String hostname, Agent agent, String reason) { - return move(hostname, true, Node.State.active, agent, Optional.of(reason)); + return move(hostname, true, State.active, agent, Optional.of(reason)); } - private List moveRecursively(String hostname, Node.State toState, Agent agent, Optional reason) { + private List moveRecursively(String hostname, State toState, Agent agent, Optional reason) { List moved = list().childrenOf(hostname).asList().stream() .map(child -> move(child, toState, agent, reason)) .collect(Collectors.toList()); @@ -533,7 +534,7 @@ public class NodeRepository extends AbstractComponent { return moved; } - private Node move(String hostname, boolean keepAllocation, Node.State toState, Agent agent, Optional reason) { + private Node move(String hostname, boolean keepAllocation, State toState, Agent agent, Optional reason) { Node node = getNode(hostname).orElseThrow(() -> new NoSuchNodeException("Could not move " + hostname + " to " + toState + ": Node not found")); @@ -544,13 +545,13 @@ public class NodeRepository extends AbstractComponent { return move(node, toState, agent, reason); } - private Node move(Node node, Node.State toState, Agent agent, Optional reason) { + private Node move(Node node, State toState, Agent agent, Optional reason) { if (toState == Node.State.active && ! node.allocation().isPresent()) throw new IllegalArgumentException("Could not set " + node.hostname() + " active. It has no allocation."); try (Mutex lock = lock(node)) { - if (toState == Node.State.active) { - for (Node currentActive : getNodes(node.allocation().get().owner(), Node.State.active)) { + if (toState == State.active) { + for (Node currentActive : getNodes(node.allocation().get().owner(), State.active)) { if (node.allocation().get().membership().cluster().equals(currentActive.allocation().get().membership().cluster()) && node.allocation().get().membership().index() == currentActive.allocation().get().membership().index()) throw new IllegalArgumentException("Could not move " + node + " to active:" + @@ -568,19 +569,20 @@ public class NodeRepository extends AbstractComponent { public Node markNodeAvailableForNewAllocation(String hostname, Agent agent, String reason) { Node node = getNode(hostname).orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'")); if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER && node.type() == NodeType.tenant) { - 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 (node.state() != State.dirty) { + throw new IllegalArgumentException("Cannot make " + hostname + " available for new allocation, " + + "must be in state dirty, but was in " + node.state()); } return removeRecursively(node, true).get(0); } - if (node.state() == Node.State.ready) return node; + if (node.state() == State.ready) return node; Node parentHost = node.parentHostname().flatMap(this::getNode).orElse(node); List failureReasons = NodeFailer.reasonsToFailParentHost(parentHost); if (!failureReasons.isEmpty()) { - throw new IllegalArgumentException("Node " + hostname + " cannot be readied because it has hard failures: " + failureReasons); + throw new IllegalArgumentException("Node " + hostname + " cannot be readied because it has hard failures: " + + failureReasons); } return setReady(Collections.singletonList(node), agent, reason).get(0); @@ -616,51 +618,48 @@ public class NodeRepository extends AbstractComponent { } /** - * Returns whether given node can be removed. Removal is allowed if: + * Throws if the given node cannot be removed. Removal is allowed if: * Tenant node: node is unallocated * 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|dirty|ready */ - private boolean canRemove(Node node, boolean deletingAsChild) { - if (node.type() == NodeType.tenant && node.allocation().isPresent()) { - throw new IllegalArgumentException("Node is currently allocated and cannot be removed: " + - node.allocation().get()); - } - if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER && !deletingAsChild) { - if (node.state() != Node.State.ready) { - throw new IllegalArgumentException( - String.format("Docker container %s can only be removed when in ready state", node.hostname())); - } - - } else if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) { - Set legalStates = EnumSet.of(Node.State.provisioned, Node.State.failed, Node.State.parked, - Node.State.dirty, Node.State.ready); - - if (! legalStates.contains(node.state())) { - throw new IllegalArgumentException(String.format("Child node %s can only be removed from following states: %s", - node.hostname(), legalStates.stream().map(Node.State::name).collect(Collectors.joining(", ")))); - } - } else { - Set legalStates = EnumSet.of(Node.State.provisioned, Node.State.failed, Node.State.parked); + private boolean canRemove(Node node, boolean removingAsChild) { + if (node.type() == NodeType.tenant && node.allocation().isPresent()) + illegal("Node is currently allocated and cannot be removed: " + node.allocation().get()); - if (! legalStates.contains(node.state())) { - throw new IllegalArgumentException(String.format("Node %s can only be removed from following states: %s", - node.hostname(), legalStates.stream().map(Node.State::name).collect(Collectors.joining(", ")))); - } + if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER && !removingAsChild) { + if (node.state() != State.ready) + illegal(node + " can not be removed as it is not in the state [ready]"); + } + else if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER) { // removing a child node + Set legalStates = EnumSet.of(State.provisioned, State.failed, State.parked, State.dirty, State.ready); + if ( ! legalStates.contains(node.state())) + illegal(node + " can not be removed as it is not in the states " + legalStates); + } + else { // a host + Set legalStates = EnumSet.of(State.provisioned, State.failed, State.parked); + if (! legalStates.contains(node.state())) + illegal(node + " can not be removed as it is not in the states " + legalStates); } return true; } + private void illegal(String message) { + throw new IllegalArgumentException(message); + } + /** * Increases the restart generation of the active nodes matching the filter. * * @return the nodes in their new state. */ public List restart(NodeFilter filter) { - return performOn(StateFilter.from(Node.State.active, filter), (node, lock) -> write(node.withRestart(node.allocation().get().restartGeneration().withIncreasedWanted()), lock)); + return performOn(StateFilter.from(State.active, filter), + (node, lock) -> write(node.withRestart(node.allocation().get().restartGeneration().withIncreasedWanted()), + lock)); } /** 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 c9671aeafbe..1316c838414 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 @@ -9,6 +9,7 @@ import org.junit.Test; import java.time.Instant; import java.util.Arrays; +import java.util.EnumSet; import java.util.HashSet; import java.util.Set; import java.util.function.Predicate; -- cgit v1.2.3 From 5d86852c1f81f6e71aeaf3d6ccd3665e94b2e918 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 9 Mar 2020 13:43:26 +0100 Subject: Simplify, no functional changes --- .../com/yahoo/vespa/hosted/provision/Node.java | 2 +- .../vespa/hosted/provision/NodeRepository.java | 57 ++++++++-------------- .../hosted/provision/restapi/v2/RestApiTest.java | 8 +-- 3 files changed, 26 insertions(+), 41 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index bbff263cb7e..f088d56fc89 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -393,7 +393,7 @@ public final class Node { return state + ( parentHostname.isPresent() ? " child node " : " host " ) + hostname + - ( allocation.isPresent() ? " " + allocation : ""); + ( allocation.isPresent() ? " " + allocation.get() : ""); } public enum State { 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 630bc0153b8..f97ac5dd3ba 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 @@ -290,9 +290,7 @@ public class NodeRepository extends AbstractComponent { break; default: - throw new IllegalArgumentException( - String.format("Don't know how to create ACL for node [hostname=%s type=%s]", - node.hostname(), node.type())); + illegal("Don't know how to create ACL for " + node + " of type " + node.type()); } return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts); @@ -324,9 +322,8 @@ public class NodeRepository extends AbstractComponent { /** Creates a new node object, without adding it to the node repo. If no IP address is given, it will be resolved */ public Node createNode(String openStackId, String hostname, IP.Config ipConfig, Optional parentHostname, Flavor flavor, Optional reservedTo, NodeType type) { - if (ipConfig.primary().isEmpty()) { // TODO: Remove this. Only test code hits this path + if (ipConfig.primary().isEmpty()) // TODO: Remove this. Only test code hits this path ipConfig = ipConfig.with(nameResolver.getAllByNameOrThrow(hostname)); - } return Node.create(openStackId, ipConfig, hostname, parentHostname, Optional.empty(), flavor, reservedTo, type); } @@ -337,17 +334,15 @@ public class NodeRepository extends AbstractComponent { /** Adds a list of newly created docker container nodes to the node repository as reserved nodes */ public List addDockerNodes(LockedNodeList nodes) { for (Node node : nodes) { - if (!node.flavor().getType().equals(Flavor.Type.DOCKER_CONTAINER)) { - throw new IllegalArgumentException("Cannot add " + node.hostname() + ": This is not a docker node"); - } - if (!node.allocation().isPresent()) { - throw new IllegalArgumentException("Cannot add " + node.hostname() + ": Docker containers needs to be allocated"); - } + if ( ! node.flavor().getType().equals(Flavor.Type.DOCKER_CONTAINER)) + illegal("Cannot add " + node + ": This is not a docker node"); + if ( ! node.allocation().isPresent()) + illegal("Cannot add " + node + ": Docker containers needs to be allocated"); Optional existing = getNode(node.hostname()); if (existing.isPresent()) - throw new IllegalArgumentException("Cannot add " + node.hostname() + ": A node with this name already exists (" + - existing.get() + ", " + existing.get().history() + "). Node to be added: " + - node + ", " + node.history()); + illegal("Cannot add " + node + ": A node with this name already exists (" + + existing.get() + ", " + existing.get().history() + "). Node to be added: " + + node + ", " + node.history()); } return db.addNodesInState(nodes.asList(), State.reserved); } @@ -378,7 +373,7 @@ public class NodeRepository extends AbstractComponent { List nodesWithResetFields = nodes.stream() .map(node -> { if (node.state() != State.provisioned && node.state() != State.dirty) - throw new IllegalArgumentException("Can not set " + node + " ready. It is not provisioned or dirty."); + illegal("Can not set " + node + " ready. It is not provisioned or dirty."); return node.with(node.status().withWantToRetire(false).withWantToDeprovision(false)); }) .collect(Collectors.toList()); @@ -467,14 +462,11 @@ public class NodeRepository extends AbstractComponent { .filter(node -> node.state() != State.parked) .map(Node::hostname) .collect(Collectors.toList()); - if (!hostnamesNotAllowedToDirty.isEmpty()) { - throw new IllegalArgumentException("Could not deallocate " + hostname + ": " + - String.join(", ", hostnamesNotAllowedToDirty) + " must be in either provisioned, failed or parked state"); - } + if ( ! hostnamesNotAllowedToDirty.isEmpty()) + illegal("Could not deallocate " + nodeToDirty + ": " + + hostnamesNotAllowedToDirty + " are not in states [provisioned, failed, parked]"); - return nodesToDirty.stream() - .map(node -> setDirty(node, agent, reason)) - .collect(Collectors.toList()); + return nodesToDirty.stream().map(node -> setDirty(node, agent, reason)).collect(Collectors.toList()); } /** @@ -547,15 +539,14 @@ public class NodeRepository extends AbstractComponent { private Node move(Node node, State toState, Agent agent, Optional reason) { if (toState == Node.State.active && ! node.allocation().isPresent()) - throw new IllegalArgumentException("Could not set " + node.hostname() + " active. It has no allocation."); + illegal("Could not set " + node + " active. It has no allocation."); try (Mutex lock = lock(node)) { if (toState == State.active) { for (Node currentActive : getNodes(node.allocation().get().owner(), State.active)) { if (node.allocation().get().membership().cluster().equals(currentActive.allocation().get().membership().cluster()) && node.allocation().get().membership().index() == currentActive.allocation().get().membership().index()) - throw new IllegalArgumentException("Could not move " + node + " to active:" + - "It has the same cluster and index as an existing node"); + illegal("Could not set " + node + " active: Same cluster and index as " + currentActive); } } return db.writeTo(toState, node, agent, reason); @@ -569,10 +560,8 @@ public class NodeRepository extends AbstractComponent { public Node markNodeAvailableForNewAllocation(String hostname, Agent agent, String reason) { Node node = getNode(hostname).orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'")); if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER && node.type() == NodeType.tenant) { - if (node.state() != State.dirty) { - throw new IllegalArgumentException("Cannot make " + hostname + " available for new allocation, " + - "must be in state dirty, but was in " + node.state()); - } + if (node.state() != State.dirty) + illegal("Cannot make " + node + " available for new allocation as it is not in state [dirty]"); return removeRecursively(node, true).get(0); } @@ -580,10 +569,8 @@ public class NodeRepository extends AbstractComponent { Node parentHost = node.parentHostname().flatMap(this::getNode).orElse(node); List failureReasons = NodeFailer.reasonsToFailParentHost(parentHost); - if (!failureReasons.isEmpty()) { - throw new IllegalArgumentException("Node " + hostname + " cannot be readied because it has hard failures: " + - failureReasons); - } + if ( ! failureReasons.isEmpty()) + illegal(node + " cannot be readied because it has hard failures: " + failureReasons); return setReady(Collections.singletonList(node), agent, reason).get(0); } @@ -612,8 +599,6 @@ public class NodeRepository extends AbstractComponent { db.removeNodes(removed); return removed; - } catch (RuntimeException e) { - throw new IllegalArgumentException("Failed to delete " + node.hostname(), e); } } @@ -627,7 +612,7 @@ public class NodeRepository extends AbstractComponent { */ private boolean canRemove(Node node, boolean removingAsChild) { if (node.type() == NodeType.tenant && node.allocation().isPresent()) - illegal("Node is currently allocated and cannot be removed: " + node.allocation().get()); + illegal(node + " is currently allocated and cannot be removed"); if (node.flavor().getType() == Flavor.Type.DOCKER_CONTAINER && !removingAsChild) { if (node.state() != State.ready) 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 c26614c630c..74d75d50fc5 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 @@ -363,7 +363,7 @@ public class RestApiTest { "{\"message\":\"Updated host12.yahoo.com\"}"); assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host12.yahoo.com", new byte[0], Request.Method.PUT), 400, - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Node host12.yahoo.com cannot be readied because it has " + + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"provisioned host host12.yahoo.com cannot be readied because it has " + "hard failures: [diskSpace reported 1970-01-01T00:00:00.002Z: " + msg + "]\"}"); } @@ -428,7 +428,7 @@ public class RestApiTest { assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host1.yahoo.com", new byte[0], Request.Method.PUT), 400, - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot make host1.yahoo.com available for new allocation, must be in state dirty, but was in failed\"}"); + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot make failed host host1.yahoo.com allocated to tenant1.application1.instance1 as 'container/id1/0/0' available for new allocation as it is not in state [dirty]\"}"); // (... while dirty then ready works (the ready move will be initiated by node maintenance)) assertResponse(new Request("http://localhost:8080/nodes/v2/state/dirty/host1.yahoo.com", @@ -444,7 +444,7 @@ public class RestApiTest { "{\"message\":\"Moved host2.yahoo.com to parked\"}"); assertResponse(new Request("http://localhost:8080/nodes/v2/state/ready/host2.yahoo.com", new byte[0], Request.Method.PUT), - 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot make host2.yahoo.com available for new allocation, must be in state dirty, but was in parked\"}"); + 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot make parked host host2.yahoo.com allocated to tenant2.application2.instance2 as 'content/id2/0/0' available for new allocation as it is not in state [dirty]\"}"); // (... while dirty then ready works (the ready move will be initiated by node maintenance)) assertResponse(new Request("http://localhost:8080/nodes/v2/state/dirty/host2.yahoo.com", new byte[0], Request.Method.PUT), @@ -461,7 +461,7 @@ public class RestApiTest { // Attempt to DELETE allocated node assertResponse(new Request("http://localhost:8080/nodes/v2/node/host4.yahoo.com", new byte[0], Request.Method.DELETE), - 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Failed to delete host4.yahoo.com: Node is currently allocated and cannot be removed: allocated to tenant3.application3.instance3 as 'content/id3/0/0'\"}"); + 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"active child node host4.yahoo.com allocated to tenant3.application3.instance3 as 'content/id3/0/0' is currently allocated and cannot be removed\"}"); // PUT current restart generation with string instead of long assertResponse(new Request("http://localhost:8080/nodes/v2/node/host4.yahoo.com", -- cgit v1.2.3 From aa1758edef6a1bfaa06dc7c88b888dc9ea0061b9 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 9 Mar 2020 14:48:40 +0100 Subject: Add deprovisioned state --- .../java/com/yahoo/config/provision/NodeType.java | 4 +- .../controller/application/SystemApplication.java | 2 +- .../com/yahoo/vespa/hosted/provision/Node.java | 7 +- .../vespa/hosted/provision/NodeRepository.java | 53 ++--- .../provision/maintenance/FailedExpirer.java | 4 +- .../hosted/provision/maintenance/NodeFailer.java | 8 +- .../provision/maintenance/OsUpgradeActivator.java | 2 +- .../provision/maintenance/RetiredExpirer.java | 2 +- .../yahoo/vespa/hosted/provision/node/History.java | 21 +- .../com/yahoo/vespa/hosted/provision/node/IP.java | 2 +- .../vespa/hosted/provision/os/OsVersions.java | 2 +- .../persistence/CuratorDatabaseClient.java | 3 +- .../provision/persistence/NodeSerializer.java | 2 + .../provision/provisioning/DockerImages.java | 4 +- .../hosted/provision/restapi/v2/NodePatcher.java | 2 +- .../provision/restapi/v2/NodeSerializer.java | 2 + .../hosted/provision/restapi/v2/NodesResponse.java | 2 +- .../vespa/hosted/provision/NodeRepositoryTest.java | 8 +- .../provision/maintenance/CapacityCheckerTest.java | 243 ++++++++++++--------- .../maintenance/CapacityCheckerTester.java | 10 +- .../DynamicProvisioningMaintainerTest.java | 11 +- .../provision/maintenance/MetricsReporterTest.java | 7 +- .../restapi/v2/responses/states-recursive.json | 5 + .../provision/restapi/v2/responses/states.json | 3 + 24 files changed, 233 insertions(+), 176 deletions(-) (limited to 'node-repository') diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java index 58ae8b95e97..1d4265172a3 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java @@ -45,7 +45,7 @@ public enum NodeType { this.description = description; } - public boolean isDockerHost() { + public boolean isHost() { return !childNodeTypes.isEmpty(); } @@ -66,7 +66,7 @@ public enum NodeType { * @throws IllegalStateException if this type is not a host */ public List childNodeTypes() { - if (! isDockerHost()) + if (! isHost()) throw new IllegalStateException(this + " has no children"); return childNodeTypes; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java index 66015c76b06..a0831d3d7ff 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java @@ -65,7 +65,7 @@ public enum SystemApplication { /** Returns whether this should receive OS upgrades */ public boolean shouldUpgradeOs() { - return nodeType.isDockerHost(); + return nodeType.isHost(); } /** All known system applications */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index f088d56fc89..d42efd7ba29 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -398,7 +398,7 @@ public final class Node { public enum State { - /** This node has been requested (from OpenStack) but is not yet ready for use */ + /** This host has been requested (from OpenStack) but is not yet ready for use */ provisioned, /** This node is free and ready for use */ @@ -424,7 +424,10 @@ public final class Node { * This state follows the same rules as failed except that it will never be automatically moved out of * this state. */ - parked; + parked, + + /** This host has previously been in use but is now removed. */ + deprovisioned; /** Returns whether this is a state where the node is assigned to an application */ public boolean isAllocated() { 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 f97ac5dd3ba..42e67092bfd 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 @@ -450,7 +450,7 @@ public class NodeRepository extends AbstractComponent { new IllegalArgumentException("Could not deallocate " + hostname + ": Node not found")); List nodesToDirty = - (nodeToDirty.type().isDockerHost() ? + (nodeToDirty.type().isHost() ? Stream.concat(list().childrenOf(hostname).asList().stream(), Stream.of(nodeToDirty)) : Stream.of(nodeToDirty)) .filter(node -> node.state() != State.dirty) @@ -587,30 +587,35 @@ public class NodeRepository extends AbstractComponent { public List removeRecursively(Node node, boolean force) { try (Mutex lock = lockUnallocated()) { - List removed = new ArrayList<>(); - - if (node.type().isDockerHost()) { - list().childrenOf(node).asList().stream() - .filter(child -> force || canRemove(child, true)) - .forEach(removed::add); - } - - if (force || canRemove(node, false)) removed.add(node); - db.removeNodes(removed); - - return removed; + requireRemovable(node, false, force); + + if (node.type().isHost()) { + List children = list().childrenOf(node).asList(); + children.forEach(child -> requireRemovable(child, true, force)); + db.removeNodes(children); + move(node, State.deprovisioned, Agent.system, Optional.empty()); + List removed = new ArrayList<>(children); + removed.add(node); + return removed; + } + else { + db.removeNodes(List.of(node)); + return List.of(node); + } } } /** * Throws if the given node cannot be removed. Removal is allowed if: - * Tenant node: node is unallocated - * 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|dirty|ready + * - Tenant node: node is unallocated + * - 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|dirty|ready */ - private boolean canRemove(Node node, boolean removingAsChild) { + private void requireRemovable(Node node, boolean removingAsChild, boolean force) { + if (force) return; + if (node.type() == NodeType.tenant && node.allocation().isPresent()) illegal(node + " is currently allocated and cannot be removed"); @@ -628,12 +633,6 @@ public class NodeRepository extends AbstractComponent { if (! legalStates.contains(node.state())) illegal(node + " can not be removed as it is not in the states " + legalStates); } - - return true; - } - - private void illegal(String message) { - throw new IllegalArgumentException(message); } /** @@ -742,4 +741,8 @@ public class NodeRepository extends AbstractComponent { return node.allocation().isPresent() ? lock(node.allocation().get().owner()) : lockUnallocated(); } + private void illegal(String message) { + throw new IllegalArgumentException(message); + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index 264df716fa8..cc7abfe933b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -100,8 +100,8 @@ public class FailedExpirer extends Maintainer { List nodesToRecycle = new ArrayList<>(); for (Node candidate : nodes) { if (NodeFailer.hasHardwareIssue(candidate, nodeRepository)) { - List unparkedChildren = !candidate.type().isDockerHost() ? Collections.emptyList() : - nodeRepository.list().childrenOf(candidate).asList().stream() + List unparkedChildren = !candidate.type().isHost() ? Collections.emptyList() : + nodeRepository.list().childrenOf(candidate).asList().stream() .filter(node -> node.state() != Node.State.parked) .map(Node::hostname) .collect(Collectors.toList()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java index b0b8f7d8d2c..ce4b161d8b3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java @@ -102,7 +102,7 @@ public class NodeFailer extends Maintainer { for (Map.Entry entry : getReadyNodesByFailureReason().entrySet()) { Node node = entry.getKey(); if (throttle(node)) { - if (node.type().isDockerHost()) throttledHostFailures++; + if (node.type().isHost()) throttledHostFailures++; else throttledNodeFailures++; continue; } @@ -121,7 +121,7 @@ public class NodeFailer extends Maintainer { continue; } if (throttle(node)) { - if (node.type().isDockerHost()) throttledHostFailures++; + if (node.type().isHost()) throttledHostFailures++; else throttledNodeFailures++; continue; } @@ -207,7 +207,7 @@ public class NodeFailer extends Maintainer { nodesByFailureReason.put(node, "Node has been down longer than " + downTimeLimit); } else if (hostSuspended(node, activeNodes)) { Node hostNode = node.parentHostname().flatMap(parent -> nodeRepository().getNode(parent)).orElse(node); - if (hostNode.type().isDockerHost()) { + if (hostNode.type().isHost()) { List failureReports = reasonsToFailParentHost(hostNode); if (failureReports.size() > 0) { if (hostNode.equals(node)) { @@ -237,7 +237,7 @@ public class NodeFailer extends Maintainer { } private boolean expectConfigRequests(Node node) { - return !node.type().isDockerHost(); + return !node.type().isHost(); } private boolean hasNodeRequestedConfigAfter(Node node, Instant instant) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java index c22bda8a5cf..a5d3a807e1f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java @@ -24,7 +24,7 @@ public class OsUpgradeActivator extends Maintainer { @Override protected void maintain() { for (var nodeType : NodeType.values()) { - if (!nodeType.isDockerHost()) continue; + if (!nodeType.isHost()) continue; var active = canUpgradeOsOf(nodeType); nodeRepository().osVersions().setActive(nodeType, active); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java index 1d31917b3e1..dfdf4b0fa1f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java @@ -81,7 +81,7 @@ public class RetiredExpirer extends Maintainer { * - Orchestrator allows it */ private boolean canRemove(Node node) { - if (node.type().isDockerHost()) { + if (node.type().isHost()) { if (nodeRepository() .list().childrenOf(node).asList().stream() .allMatch(child -> child.state() == Node.State.parked || diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java index ef6f531cc89..959071c83c4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/History.java @@ -80,15 +80,16 @@ public class History { // If the event is a re-reservation, allow the new one to override the older one. if (from == to && from != Node.State.reserved) return this; switch (to) { - case provisioned: return this.with(new Event(Event.Type.provisioned, agent, at)); - case ready: return this.withoutApplicationEvents().with(new Event(Event.Type.readied, agent, at)); - case active: return this.with(new Event(Event.Type.activated, agent, at)); - case inactive: return this.with(new Event(Event.Type.deactivated, agent, at)); - case reserved: return this.with(new Event(Event.Type.reserved, agent, at)); - case failed: return this.with(new Event(Event.Type.failed, agent, at)); - case dirty: return this.with(new Event(Event.Type.deallocated, agent, at)); - case parked: return this.with(new Event(Event.Type.parked, agent, at)); - default: return this; + case provisioned: return this.with(new Event(Event.Type.provisioned, agent, at)); + case deprovisioned: return this.with(new Event(Event.Type.deprovisioned, agent, at)); + case ready: return this.withoutApplicationEvents().with(new Event(Event.Type.readied, agent, at)); + case active: return this.with(new Event(Event.Type.activated, agent, at)); + case inactive: return this.with(new Event(Event.Type.deactivated, agent, at)); + case reserved: return this.with(new Event(Event.Type.reserved, agent, at)); + case failed: return this.with(new Event(Event.Type.failed, agent, at)); + case dirty: return this.with(new Event(Event.Type.deallocated, agent, at)); + case parked: return this.with(new Event(Event.Type.parked, agent, at)); + default: return this; } } @@ -128,7 +129,7 @@ public class History { public enum Type { // State move events - provisioned(false), readied, reserved, activated, deactivated, deallocated, parked, + provisioned(false), deprovisioned(false), readied, reserved, activated, deactivated, deallocated, parked, // The node was scheduled for retirement wantToRetire, // The active node was retired diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java index aa824e7a930..3a6d684ddd0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java @@ -133,7 +133,7 @@ public class IP { var addresses = new HashSet<>(node.ipConfig().primary()); var otherAddresses = new HashSet<>(other.ipConfig().primary()); - if (node.type().isDockerHost()) { // Addresses of a host can never overlap with any other nodes + if (node.type().isHost()) { // Addresses of a host can never overlap with any other nodes addresses.addAll(node.ipConfig().pool().asSet()); otherAddresses.addAll(other.ipConfig().pool().asSet()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java index e10ff3d24cd..dc46eaec744 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java @@ -141,7 +141,7 @@ public class OsVersions { } private static void require(NodeType nodeType) { - if (!nodeType.isDockerHost()) { + if (!nodeType.isHost()) { throw new IllegalArgumentException("Node type '" + nodeType + "' does not support OS upgrades"); } } 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 f211ea9eac5..9555266966c 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 @@ -245,7 +245,7 @@ public class CuratorDatabaseClient { /** Returns whether to reboot node as part of transition to given state. This is done to get rid of any lingering * unwanted state (e.g. processes) on non-host nodes. */ private boolean rebootOnTransitionTo(Node.State state, Node node) { - if (node.type().isDockerHost()) return false; // Reboot of host nodes is handled by NodeRebooter + if (node.type().isHost()) return false; // Reboot of host nodes is handled by NodeRebooter if (zone.environment().isTest()) return false; // We want to reuse nodes quickly in test environments return node.state() != Node.State.dirty && state == Node.State.dirty; @@ -333,6 +333,7 @@ public class CuratorDatabaseClient { case provisioned: return "provisioned"; case ready: return "ready"; case reserved: return "reserved"; + case deprovisioned: return "deprovisioned"; default: throw new RuntimeException("Node state " + state + " does not map to a directory name"); } } 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 9614c1aa5c7..c08ede7f63c 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 @@ -358,6 +358,7 @@ public class NodeSerializer { private History.Event.Type eventTypeFromString(String eventTypeString) { switch (eventTypeString) { case "provisioned" : return History.Event.Type.provisioned; + case "deprovisioned" : return History.Event.Type.deprovisioned; case "readied" : return History.Event.Type.readied; case "reserved" : return History.Event.Type.reserved; case "activated" : return History.Event.Type.activated; @@ -379,6 +380,7 @@ public class NodeSerializer { private String toString(History.Event.Type nodeEventType) { switch (nodeEventType) { case provisioned : return "provisioned"; + case deprovisioned : return "deprovisioned"; case readied : return "readied"; case reserved : return "reserved"; case activated : return "activated"; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java index 4416106f23e..a3beeb51dbb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java @@ -62,7 +62,7 @@ public class DockerImages { /** Returns the image to use for given node and zone */ public DockerImage dockerImageFor(Node node) { - if (node.type().isDockerHost()) { + if (node.type().isHost()) { // Docker hosts do not run in containers, and thus has no image. Return the image of the child node type // instead as this allows the host to pre-download the (likely) image its node will run. // @@ -92,7 +92,7 @@ public class DockerImages { /** Set the docker image for nodes of given type */ public void setDockerImage(NodeType nodeType, Optional dockerImage) { - if (nodeType.isDockerHost()) { + if (nodeType.isHost()) { throw new IllegalArgumentException("Setting docker image for " + nodeType + " nodes is unsupported"); } try (Lock lock = db.lockDockerImages()) { 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 a060f9c8b55..3d081997dc2 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 @@ -96,7 +96,7 @@ public class NodePatcher { private List applyFieldRecursive(String name, Inspector value) { switch (name) { case WANT_TO_RETIRE: - List childNodes = node.type().isDockerHost() ? nodes.get().childrenOf(node).asList() : List.of(); + List childNodes = node.type().isHost() ? nodes.get().childrenOf(node).asList() : List.of(); return childNodes.stream() .map(child -> applyField(child, name, value)) .collect(Collectors.toList()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java index 5ca2667ad5d..492994c2f0e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodeSerializer.java @@ -22,6 +22,7 @@ public class NodeSerializer { case "provisioned": return Node.State.provisioned; case "ready": return Node.State.ready; case "reserved": return Node.State.reserved; + case "deprovisioned": return Node.State.deprovisioned; default: throw new IllegalArgumentException("Unknown node state '" + state + "'"); } } @@ -36,6 +37,7 @@ public class NodeSerializer { case provisioned: return "provisioned"; case ready: return "ready"; case reserved: return "reserved"; + case deprovisioned: return "deprovisioned"; default: throw new IllegalArgumentException("Unknown node state '" + state + "'"); } } 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 7f283452538..3a9025e9b3e 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 { node.status().osVersion().current().ifPresent(version -> object.setString("currentOsVersion", version.toFullString())); node.status().osVersion().wanted().ifPresent(version -> object.setString("wantedOsVersion", version.toFullString())); node.status().firmwareVerifiedAt().ifPresent(instant -> object.setLong("currentFirmwareCheck", instant.toEpochMilli())); - if (node.type().isDockerHost()) + if (node.type().isHost()) nodeRepository.firmwareChecks().requiredAfter().ifPresent(after -> object.setLong("wantedFirmwareCheck", after.toEpochMilli())); node.status().vespaVersion().ifPresent(version -> object.setString("vespaVersion", version.toFullString())); currentDockerImage(node).ifPresent(dockerImage -> object.setString("currentDockerImage", dockerImage.asString())); 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 1316c838414..c111c48e07c 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 @@ -131,16 +131,16 @@ public class NodeRepositoryTest { // 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()); + assertEquals(5, tester.nodeRepository().getNodes().size()); + assertEquals(Node.State.deprovisioned, tester.nodeRepository().getNode("host2").get().state()); // Now node10 is in provisioned, set node11 to failed and node12 to ready, and it should be OK to delete host1 tester.nodeRepository().fail("node11", Agent.system, getClass().getSimpleName()); tester.nodeRepository().setReady("node12", Agent.system, getClass().getSimpleName()); tester.nodeRepository().removeRecursively("node12"); // Remove one of the children first instead - assertEquals(3, tester.nodeRepository().getNodes().size()); - + assertEquals(4, tester.nodeRepository().getNodes().size()); tester.nodeRepository().removeRecursively("host1"); - assertEquals(0, tester.nodeRepository().getNodes().size()); + assertEquals(Node.State.deprovisioned, tester.nodeRepository().getNode("host1").get().state()); } @Test diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java index 9d79af804ce..5813585554d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; -import org.junit.Before; import org.junit.Test; import java.io.IOException; @@ -20,19 +19,13 @@ import static org.junit.Assert.fail; * @author mgimle */ public class CapacityCheckerTest { - private CapacityCheckerTester tester; - - @Before - public void setup() { - tester = new CapacityCheckerTester(); - } @Test public void testWithRealData() throws IOException { + CapacityCheckerTester tester = new CapacityCheckerTester(); String path = "./src/test/resources/zookeeper_dump.json"; - tester.cleanRepository(); - tester.restoreNodeRepositoryFromJsonFile(Paths.get(path)); + tester.populateNodeRepositoryFromJsonFile(Paths.get(path)); var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); assertTrue(tester.nodeRepository.getNodes(NodeType.host).containsAll(failurePath.get().hostsCausingFailure)); @@ -40,6 +33,7 @@ public class CapacityCheckerTest { @Test public void testOvercommittedHosts() { + CapacityCheckerTester tester = new CapacityCheckerTester(); tester.createNodes(7, 4, 10, new NodeResources(-1, 10, 100, 1), 10, 0, new NodeResources(1, 10, 100, 1), 10); @@ -49,37 +43,50 @@ public class CapacityCheckerTest { @Test public void testEdgeCaseFailurePaths() { - tester.createNodes(1, 1, - 0, new NodeResources(1, 10, 100, 1), 10, - 0, new NodeResources(1, 10, 100, 1), 10); - var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertFalse("Computing worst case host loss with no hosts should return an empty optional.", failurePath.isPresent()); + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(1, 1, + 0, new NodeResources(1, 10, 100, 1), 10, + 0, new NodeResources(1, 10, 100, 1), 10); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertFalse("Computing worst case host loss with no hosts should return an empty optional.", failurePath.isPresent()); + } // Odd edge case that should never be able to occur in prod - tester.createNodes(1, 10, - 10, new NodeResources(10, 1000, 10000, 1), 100, - 1, new NodeResources(10, 1000, 10000, 1), 100); - failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - assertTrue("Computing worst case host loss if all hosts have to be removed should result in an non-empty failureReason with empty nodes.", - failurePath.get().failureReason.tenant.isEmpty() && failurePath.get().failureReason.host.isEmpty()); - assertEquals(tester.nodeRepository.getNodes(NodeType.host).size(), failurePath.get().hostsCausingFailure.size()); - - tester.createNodes(3, 30, - 10, new NodeResources(0, 0, 10000, 1), 1000, - 0, new NodeResources(0, 0, 0, 0), 0); - failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.allocationFailures; - assertEquals("When there are multiple lacking resources, all failures are multipleReasonFailures", - failureReasons.size(), failureReasons.multipleReasonFailures().size()); - assertEquals(0, failureReasons.singularReasonFailures().size()); - } else fail(); + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(1, 10, + 10, new NodeResources(10, 1000, 10000, 1), 100, + 1, new NodeResources(10, 1000, 10000, 1), 100); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + assertTrue("Computing worst case host loss if all hosts have to be removed should result in an non-empty failureReason with empty nodes.", + failurePath.get().failureReason.tenant.isEmpty() && failurePath.get().failureReason.host.isEmpty()); + assertEquals(tester.nodeRepository.getNodes(NodeType.host).size(), failurePath.get().hostsCausingFailure.size()); + } + + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(3, 30, + 10, new NodeResources(0, 0, 10000, 1), 1000, + 0, new NodeResources(0, 0, 0, 0), 0); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + if (failurePath.get().failureReason.tenant.isPresent()) { + var failureReasons = failurePath.get().failureReason.allocationFailures; + assertEquals("When there are multiple lacking resources, all failures are multipleReasonFailures", + failureReasons.size(), failureReasons.multipleReasonFailures().size()); + assertEquals(0, failureReasons.singularReasonFailures().size()); + } + else { + fail(); + } + } } @Test public void testIpFailurePaths() { + CapacityCheckerTester tester = new CapacityCheckerTester(); tester.createNodes(1, 10, 10, new NodeResources(10, 1000, 10000, 1), 1, 10, new NodeResources(10, 1000, 10000, 1), 1); @@ -95,79 +102,111 @@ public class CapacityCheckerTest { @Test public void testNodeResourceFailurePaths() { - tester.createNodes(1, 10, - 10, new NodeResources(1, 100, 1000, 1), 100, - 10, new NodeResources(0, 100, 1000, 1), 100); - var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.allocationFailures; - assertEquals("All failures should be due to hosts lacking cpu cores.", - failureReasons.singularReasonFailures().insufficientVcpu(), failureReasons.size()); - } else fail(); - - tester.createNodes(1, 10, - 10, new NodeResources(10, 1, 1000, 1), 100, - 10, new NodeResources(10, 0, 1000, 1), 100); - failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.allocationFailures; - assertEquals("All failures should be due to hosts lacking memory.", - failureReasons.singularReasonFailures().insufficientMemoryGb(), failureReasons.size()); - } else fail(); - - tester.createNodes(1, 10, - 10, new NodeResources(10, 100, 10, 1), 100, - 10, new NodeResources(10, 100, 0, 1), 100); - failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.allocationFailures; - assertEquals("All failures should be due to hosts lacking disk space.", - failureReasons.singularReasonFailures().insufficientDiskGb(), failureReasons.size()); - } else fail(); - - int emptyHostsWithSlowDisk = 10; - tester.createNodes(1, 10, List.of(new NodeResources(1, 10, 100, 1)), - 10, new NodeResources(0, 0, 0, 0), 100, - 10, new NodeResources(10, 1000, 10000, 1, NodeResources.DiskSpeed.slow), 100); - failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.allocationFailures; - assertEquals("All empty hosts should be invalid due to having incompatible disk speed.", - failureReasons.singularReasonFailures().incompatibleDiskSpeed(), emptyHostsWithSlowDisk); - } else fail(); - + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(1, 10, + 10, new NodeResources(1, 100, 1000, 1), 100, + 10, new NodeResources(0, 100, 1000, 1), 100); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + if (failurePath.get().failureReason.tenant.isPresent()) { + var failureReasons = failurePath.get().failureReason.allocationFailures; + assertEquals("All failures should be due to hosts lacking cpu cores.", + failureReasons.singularReasonFailures().insufficientVcpu(), failureReasons.size()); + } else { + fail(); + } + } + + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(1, 10, + 10, new NodeResources(10, 1, 1000, 1), 100, + 10, new NodeResources(10, 0, 1000, 1), 100); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + if (failurePath.get().failureReason.tenant.isPresent()) { + var failureReasons = failurePath.get().failureReason.allocationFailures; + assertEquals("All failures should be due to hosts lacking memory.", + failureReasons.singularReasonFailures().insufficientMemoryGb(), failureReasons.size()); + } + else { + fail(); + } + } + + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(1, 10, + 10, new NodeResources(10, 100, 10, 1), 100, + 10, new NodeResources(10, 100, 0, 1), 100); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + if (failurePath.get().failureReason.tenant.isPresent()) { + var failureReasons = failurePath.get().failureReason.allocationFailures; + assertEquals("All failures should be due to hosts lacking disk space.", + failureReasons.singularReasonFailures().insufficientDiskGb(), failureReasons.size()); + } else { + fail(); + } + } + + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + int emptyHostsWithSlowDisk = 10; + tester.createNodes(1, 10, List.of(new NodeResources(1, 10, 100, 1)), + 10, new NodeResources(0, 0, 0, 0), 100, + 10, new NodeResources(10, 1000, 10000, 1, NodeResources.DiskSpeed.slow), 100); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + if (failurePath.get().failureReason.tenant.isPresent()) { + var failureReasons = failurePath.get().failureReason.allocationFailures; + assertEquals("All empty hosts should be invalid due to having incompatible disk speed.", + failureReasons.singularReasonFailures().incompatibleDiskSpeed(), emptyHostsWithSlowDisk); + } else { + fail(); + } + } } - @Test public void testParentHostPolicyIntegrityFailurePaths() { - tester.createNodes(1, 1, - 10, new NodeResources(1, 100, 1000, 1), 100, - 10, new NodeResources(10, 1000, 10000, 1), 100); - var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.allocationFailures; - assertEquals("With only one type of tenant, all failures should be due to violation of the parent host policy.", - failureReasons.singularReasonFailures().violatesParentHostPolicy(), failureReasons.size()); - } else fail(); - - tester.createNodes(1, 2, - 10, new NodeResources(10, 100, 1000, 1), 1, - 0, new NodeResources(0, 0, 0, 0), 0); - failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); - assertTrue(failurePath.isPresent()); - if (failurePath.get().failureReason.tenant.isPresent()) { - var failureReasons = failurePath.get().failureReason.allocationFailures; - assertNotEquals("Fewer distinct children than hosts should result in some parent host policy violations.", - failureReasons.size(), failureReasons.singularReasonFailures().violatesParentHostPolicy()); - assertNotEquals(0, failureReasons.singularReasonFailures().violatesParentHostPolicy()); - } else fail(); + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(1, 1, + 10, new NodeResources(1, 100, 1000, 1), 100, + 10, new NodeResources(10, 1000, 10000, 1), 100); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + if (failurePath.get().failureReason.tenant.isPresent()) { + var failureReasons = failurePath.get().failureReason.allocationFailures; + assertEquals("With only one type of tenant, all failures should be due to violation of the parent host policy.", + failureReasons.singularReasonFailures().violatesParentHostPolicy(), failureReasons.size()); + } + else { + fail(); + } + } + + { + CapacityCheckerTester tester = new CapacityCheckerTester(); + tester.createNodes(1, 2, + 10, new NodeResources(10, 100, 1000, 1), 1, + 0, new NodeResources(0, 0, 0, 0), 0); + var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); + assertTrue(failurePath.isPresent()); + if (failurePath.get().failureReason.tenant.isPresent()) { + var failureReasons = failurePath.get().failureReason.allocationFailures; + assertNotEquals("Fewer distinct children than hosts should result in some parent host policy violations.", + failureReasons.size(), failureReasons.singularReasonFailures().violatesParentHostPolicy()); + assertNotEquals(0, failureReasons.singularReasonFailures().violatesParentHostPolicy()); + } + else { + fail(); + } + } } + } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java index 96236b5fb84..48439907ce4 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java @@ -175,7 +175,6 @@ public class CapacityCheckerTester { void createNodes(int childrenPerHost, int numDistinctChildren, List childResources, int numHosts, NodeResources hostExcessCapacity, int hostExcessIps, int numEmptyHosts, NodeResources emptyHostExcessCapacity, int emptyHostExcessIps) { - cleanRepository(); List possibleChildren = createDistinctChildren(numDistinctChildren, childResources); List nodes = new ArrayList<>(); @@ -275,7 +274,7 @@ public class CapacityCheckerTester { } } - public void restoreNodeRepositoryFromJsonFile(Path path) throws IOException { + public void populateNodeRepositoryFromJsonFile(Path path) throws IOException { byte[] jsonData = Files.readAllBytes(path); ObjectMapper om = new ObjectMapper(); @@ -293,11 +292,4 @@ public class CapacityCheckerTester { updateCapacityChecker(); } - void cleanRepository() { - nodeRepository.getNodes(NodeType.host).forEach(n -> nodeRepository.removeRecursively(n, true)); - nodeRepository.getNodes().forEach(n -> nodeRepository.removeRecursively(n, true)); - if (nodeRepository.getNodes().size() != 0) { - throw new IllegalStateException("Cleaning repository didn't remove all nodes! [" + nodeRepository.getNodes().size() + "]"); - } - } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index f2b6581db34..4aa73e0bd31 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -121,8 +121,8 @@ public class DynamicProvisioningMaintainerTest { verify(hostProvisioner).deprovision(argThatLambda(node -> node.hostname().equals("host2"))); verify(hostProvisioner).deprovision(argThatLambda(node -> node.hostname().equals("host3"))); verifyNoMoreInteractions(hostProvisioner); - assertFalse(tester.nodeRepository.getNode("host2").isPresent()); - assertFalse(tester.nodeRepository.getNode("host3").isPresent()); + assertEquals(Node.State.deprovisioned, tester.nodeRepository.getNode("host2").get().state()); + assertEquals(Node.State.deprovisioned, tester.nodeRepository.getNode("host3").get().state()); } @Test @@ -137,11 +137,14 @@ public class DynamicProvisioningMaintainerTest { @Test public void provision_deficit_and_deprovision_excess() { - flagSource.withListFlag(Flags.PREPROVISION_CAPACITY.id(), List.of(new PreprovisionCapacity(2, 4, 8, 1), new PreprovisionCapacity(2, 3, 2, 2)), PreprovisionCapacity.class); + flagSource.withListFlag(Flags.PREPROVISION_CAPACITY.id(), + List.of(new PreprovisionCapacity(2, 4, 8, 1), + new PreprovisionCapacity(2, 3, 2, 2)), + PreprovisionCapacity.class); addNodes(); maintainer.convergeToCapacity(tester.nodeRepository.list()); - assertFalse(tester.nodeRepository.getNode("host2").isPresent()); + assertEquals(Node.State.deprovisioned, tester.nodeRepository.getNode("host2").get().state()); assertTrue(tester.nodeRepository.getNode("host3").isPresent()); verify(hostProvisioner).deprovision(argThatLambda(node -> node.hostname().equals("host2"))); verify(hostProvisioner, times(2)).provisionHosts(argThatLambda(list -> list.size() == 1), eq(new NodeResources(2, 3, 2, 1)), any()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index ddca903a5c2..59879576117 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -38,6 +38,7 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -54,6 +55,7 @@ import static org.mockito.Mockito.when; * @author smorgrav */ public class MetricsReporterTest { + private final ServiceMonitor serviceMonitor = mock(ServiceMonitor.class); private final ApplicationInstanceReference reference = mock(ApplicationInstanceReference.class); @@ -94,6 +96,7 @@ public class MetricsReporterTest { expectedMetrics.put("hostedVespa.inactiveHosts", 0); expectedMetrics.put("hostedVespa.dirtyHosts", 0); expectedMetrics.put("hostedVespa.failedHosts", 0); + expectedMetrics.put("hostedVespa.deprovisionedHosts", 0); expectedMetrics.put("hostedVespa.pendingRedeployments", 42); expectedMetrics.put("hostedVespa.docker.totalCapacityDisk", 0.0); expectedMetrics.put("hostedVespa.docker.totalCapacityMem", 0.0); @@ -219,8 +222,8 @@ public class MetricsReporterTest { public static class TestMetric implements Metric { - public Map values = new HashMap<>(); - public Map> context = new HashMap<>(); + public Map values = new LinkedHashMap<>(); + public Map> context = new LinkedHashMap<>(); @Override public void set(String key, Number val, Context ctx) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states-recursive.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states-recursive.json index c13dca5439a..f8343756559 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states-recursive.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states-recursive.json @@ -58,6 +58,11 @@ "url": "http://localhost:8080/nodes/v2/state/parked", "nodes": [ ] + }, + "deprovisioned": { + "url": "http://localhost:8080/nodes/v2/state/deprovisioned", + "nodes": [ + ] } } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states.json index 4b2de7532dd..69579148df3 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/states.json @@ -23,6 +23,9 @@ }, "parked": { "url": "http://localhost:8080/nodes/v2/state/parked" + }, + "deprovisioned": { + "url": "http://localhost:8080/nodes/v2/state/deprovisioned" } } } \ No newline at end of file -- cgit v1.2.3 From 4753b5cadf9bcf74ad9cef6de2273f495d47c47f Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 9 Mar 2020 16:06:56 +0100 Subject: Recycle deprovisioned nodes on provision --- .../vespa/hosted/provision/NodeRepository.java | 39 +++++++++++++++------- .../maintenance/DynamicProvisioningMaintainer.java | 2 +- .../yahoo/vespa/hosted/provision/node/Agent.java | 7 ++-- .../provision/persistence/NodeSerializer.java | 2 ++ .../provision/provisioning/GroupPreparer.java | 3 +- .../provision/restapi/v2/NodesApiHandler.java | 2 +- .../provision/testutils/MockNodeRepository.java | 4 +-- .../vespa/hosted/provision/NodeRepositoryTest.java | 22 ++++++++++++ .../hosted/provision/NodeRepositoryTester.java | 4 +-- .../maintenance/CapacityCheckerTester.java | 5 +-- .../provision/maintenance/FailedExpirerTest.java | 10 +++--- .../provision/maintenance/MaintenanceTester.java | 4 +-- .../provision/maintenance/MetricsReporterTest.java | 6 ++-- .../provision/maintenance/NodeFailTester.java | 4 +-- .../OperatorChangeApplicationMaintainerTest.java | 6 ++-- .../PeriodicApplicationMaintainerTest.java | 4 +-- .../maintenance/ReservationExpirerTest.java | 2 +- .../provision/maintenance/RetiredExpirerTest.java | 4 +-- .../provisioning/DynamicDockerAllocationTest.java | 3 +- .../provision/provisioning/ProvisioningTester.java | 6 ++-- .../hosted/provision/restapi/v2/RestApiTest.java | 4 +-- 21 files changed, 93 insertions(+), 50 deletions(-) (limited to 'node-repository') 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 42e67092bfd..9f2bf81b475 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 @@ -80,7 +80,7 @@ import java.util.stream.Stream; * @author bratseth */ // Node state transitions: -// 1) (new) - > provisioned -> (dirty ->) ready -> reserved -> active -> inactive -> dirty -> ready +// 1) (new) | deprovisioned - > provisioned -> (dirty ->) ready -> reserved -> active -> inactive -> dirty -> ready // 2) inactive -> reserved | parked // 3) reserved -> dirty // 3) * -> failed | parked -> dirty | active | deprovisioned @@ -347,23 +347,38 @@ public class NodeRepository extends AbstractComponent { return db.addNodesInState(nodes.asList(), State.reserved); } - /** Adds a list of (newly created) nodes to the node repository as provisioned nodes */ - public List addNodes(List nodes) { + /** + * Adds a list of (newly created) nodes to the node repository as provisioned nodes. + * If any of the nodes already exists in the deprovisioned state, they will be moved back to provisioned instead + * and the returned list will contain the existing (moved) node. + */ + public List addNodes(List nodes, Agent agent) { try (Mutex lock = lockUnallocated()) { + List nodesToAdd = new ArrayList<>(); + List nodesToMove = new ArrayList<>(); for (int i = 0; i < nodes.size(); i++) { var node = nodes.get(i); - var message = "Cannot add " + node.hostname() + ": A node with this name already exists"; - - // Check for existing node - if (getNode(node.hostname()).isPresent()) throw new IllegalArgumentException(message); - // Check for duplicates in given list + // Check for duplicates for (int j = 0; j < i; j++) { - var other = nodes.get(j); - if (node.equals(other)) throw new IllegalArgumentException(message); + if (node.equals(nodes.get(j))) + illegal("Cannot add nodes: " + node + " is duplicated in the argument list"); + } + + Optional existing = getNode(node.hostname()); + if (existing.isPresent()) { + if (existing.get().state() != State.deprovisioned) + illegal("Cannot add " + node + ": A node with this name already exists"); + nodesToMove.add(existing.get()); + } + else { + nodesToAdd.add(node); } } - return db.addNodesInState(IP.Config.verify(nodes, list(lock)), State.provisioned); + List resultingNodes = new ArrayList<>(); + resultingNodes.addAll(db.addNodesInState(IP.Config.verify(nodesToAdd, list(lock)), State.provisioned)); + nodesToMove.forEach(node -> resultingNodes.add(move(node, State.provisioned, agent, Optional.empty()))); + return resultingNodes; } } @@ -578,7 +593,7 @@ public class NodeRepository extends AbstractComponent { /** * 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 + * @return a List of all the nodes that have been removed or (for hosts) deprovisioned */ public List removeRecursively(String hostname) { Node node = getNode(hostname).orElseThrow(() -> new NotFoundException("No node with hostname '" + hostname + "'")); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java index 94c90b58548..e0c0e288406 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java @@ -127,7 +127,7 @@ public class DynamicProvisioningMaintainer extends Maintainer { nodeRepository().database().getProvisionIndexes(1), resources, preprovisionAppId).stream() .map(ProvisionedHost::generateHost) .collect(Collectors.toList()); - nodeRepository().addNodes(hosts); + nodeRepository().addNodes(hosts, Agent.DynamicProvisioningMaintainer); } catch (OutOfCapacityException | IllegalArgumentException | IllegalStateException e) { log.log(Level.WARNING, "Failed to pre-provision " + resources + ":" + e.getMessage()); } catch (RuntimeException e) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java index 7522e411e42..18a8fe7be6a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java @@ -2,13 +2,13 @@ package com.yahoo.vespa.hosted.provision.node; /** - * The enum of kinds of actions making changes to the system. + * The enum of agents making changes to the system. * * @author bratseth */ public enum Agent { operator, // A hosted Vespa operator. Some logic recognizes these events. - application, // An application package change depoyment + application, // An application package change deployment system, // An unspecified system agent // Specific system agents: NodeFailer, @@ -17,5 +17,6 @@ public enum Agent { FailedExpirer, InactiveExpirer, ProvisionedExpirer, - ReservationExpirer + ReservationExpirer, + DynamicProvisioningMaintainer } 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 c08ede7f63c..5546f88ff47 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 @@ -411,6 +411,7 @@ public class NodeSerializer { case "InactiveExpirer" : return Agent.InactiveExpirer; case "ProvisionedExpirer" : return Agent.ProvisionedExpirer; case "ReservationExpirer" : return Agent.ReservationExpirer; + case "DynamicProvisioningMaintainer" : return Agent.DynamicProvisioningMaintainer; } throw new IllegalArgumentException("Unknown node event agent '" + eventAgentField.asString() + "'"); } @@ -426,6 +427,7 @@ public class NodeSerializer { case InactiveExpirer : return "InactiveExpirer"; case ProvisionedExpirer : return "ProvisionedExpirer"; case ReservationExpirer : return "ReservationExpirer"; + case DynamicProvisioningMaintainer : return "DynamicProvisioningMaintainer"; } throw new IllegalArgumentException("Serialized form of '" + agent + "' not defined"); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index af6fa8edf64..e90f177d99d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.flags.custom.PreprovisionCapacity; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.node.Agent; import java.util.List; import java.util.Optional; @@ -98,7 +99,7 @@ public class GroupPreparer { List hosts = provisionedHosts.stream() .map(ProvisionedHost::generateHost) .collect(Collectors.toList()); - nodeRepository.addNodes(hosts); + nodeRepository.addNodes(hosts, Agent.application); // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit List nodes = provisionedHosts.stream() 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 e75f2bbe5b8..afea92f3c60 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 @@ -202,7 +202,7 @@ public class NodesApiHandler extends LoggingRequestHandler { public int addNodes(InputStream jsonStream) { List nodes = createNodesFromSlime(toSlime(jsonStream).get()); - return nodeRepository.addNodes(nodes).size(); + return nodeRepository.addNodes(nodes, Agent.operator).size(); } private Slime toSlime(InputStream jsonStream) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java index 43d4c731759..3a6d5aa6cbe 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java @@ -128,7 +128,7 @@ public class MockNodeRepository extends NodeRepository { flavors.getFlavorOrThrow("default"), Optional.empty(), NodeType.config)); // Ready all nodes, except 7 and 55 - nodes = addNodes(nodes); + nodes = addNodes(nodes, Agent.system); nodes.remove(node7); nodes.remove(node55); nodes = setDirty(nodes, Agent.system, getClass().getSimpleName()); @@ -170,7 +170,7 @@ public class MockNodeRepository extends NodeRepository { new Flavor(new NodeResources(10, 48, 500, 1, fast, local)), Optional.empty(), NodeType.tenant)); largeNodes.add(createNode("node14", "host14.yahoo.com", ipConfig(14), Optional.empty(), new Flavor(new NodeResources(10, 48, 500, 1, fast, local)), Optional.empty(), NodeType.tenant)); - addNodes(largeNodes); + addNodes(largeNodes, Agent.system); setReady(largeNodes, Agent.system, getClass().getSimpleName()); ApplicationId app4 = ApplicationId.from(TenantName.from("tenant4"), ApplicationName.from("application4"), InstanceName.from("instance4")); ClusterSpec cluster4 = ClusterSpec.request(ClusterSpec.Type.container, 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 c111c48e07c..8830204547e 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 @@ -2,11 +2,14 @@ package com.yahoo.vespa.hosted.provision; import com.yahoo.config.provision.NodeType; +import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.provision.node.Agent; +import com.yahoo.vespa.hosted.provision.node.History; import com.yahoo.vespa.hosted.provision.node.Report; import com.yahoo.vespa.hosted.provision.node.Reports; import org.junit.Test; +import java.time.Duration; import java.time.Instant; import java.util.Arrays; import java.util.EnumSet; @@ -19,6 +22,7 @@ import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** @@ -143,6 +147,24 @@ public class NodeRepositoryTest { assertEquals(Node.State.deprovisioned, tester.nodeRepository().getNode("host1").get().state()); } + @Test + public void deprovisioned_hosts_are_resurrected_on_add() { + NodeRepositoryTester tester = new NodeRepositoryTester(); + Instant testStart = tester.nodeRepository().clock().instant(); + + ((ManualClock)tester.nodeRepository().clock()).advance(Duration.ofSeconds(1)); + tester.addNode("id1", "host1", "default", NodeType.host); + tester.addNode("id2", "host2", "default", NodeType.host); + assertFalse(tester.nodeRepository().getNode("host1").get().history().hasEventAfter(History.Event.Type.deprovisioned, testStart)); + + tester.nodeRepository().removeRecursively("host1"); + assertEquals(Node.State.deprovisioned, tester.nodeRepository().getNode("host1").get().state()); + assertTrue(tester.nodeRepository().getNode("host1").get().history().hasEventAfter(History.Event.Type.deprovisioned, testStart)); + + Node existing = tester.addNode("id1", "host1", "default", NodeType.host); + assertTrue(existing.history().hasEventAfter(History.Event.Type.deprovisioned, testStart)); + } + @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/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java index 95555185292..57c45636df9 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 @@ -54,13 +54,13 @@ public class NodeRepositoryTester { public Node addNode(String id, String hostname, String flavor, NodeType type) { Node node = nodeRepository.createNode(id, hostname, Optional.empty(), nodeFlavors.getFlavorOrThrow(flavor), type); - return nodeRepository.addNodes(Collections.singletonList(node)).get(0); + return nodeRepository.addNodes(Collections.singletonList(node), Agent.system).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); + return nodeRepository.addNodes(Collections.singletonList(node), Agent.system).get(0); } /** diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java index 48439907ce4..5e8d22275af 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java @@ -25,6 +25,7 @@ import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.flags.InMemoryFlagSource; 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.IP; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; @@ -181,7 +182,7 @@ public class CapacityCheckerTester { nodes.addAll(createHostsWithChildren(childrenPerHost, possibleChildren, numHosts, hostExcessCapacity, hostExcessIps)); nodes.addAll(createEmptyHosts(numHosts, numEmptyHosts, emptyHostExcessCapacity, emptyHostExcessIps)); - nodeRepository.addNodes(nodes); + nodeRepository.addNodes(nodes, Agent.system); updateCapacityChecker(); } @@ -288,7 +289,7 @@ public class CapacityCheckerTester { nodes.add(createNodeFromModel(nmod)); } - nodeRepository.addNodes(nodes); + nodeRepository.addNodes(nodes, Agent.system); updateCapacityChecker(); } 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 d0179cdda00..ec604ab1387 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 @@ -275,10 +275,12 @@ public class FailedExpirerTest { } public FailureScenario withNode(NodeType type, NodeResources flavor, String hostname, String parentHostname) { - nodeRepository.addNodes(List.of( - nodeRepository.createNode(UUID.randomUUID().toString(), hostname, - Optional.ofNullable(parentHostname), new Flavor(flavor), type) - )); + nodeRepository.addNodes(List.of(nodeRepository.createNode(UUID.randomUUID().toString(), + hostname, + Optional.ofNullable(parentHostname), + new Flavor(flavor), + type)), + Agent.system); return this; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java index 246f2509397..9f77e1b107f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java @@ -48,7 +48,7 @@ public class MaintenanceTester { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("node" + i, "host" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.tenant)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodes = simulateInitialReboot(nodes); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); @@ -58,7 +58,7 @@ public class MaintenanceTester { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("hostNode" + i, "realHost" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodes = simulateInitialReboot(nodes); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index 59879576117..dc7ae74ec6e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -83,9 +83,9 @@ public class MetricsReporterTest { DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true, new InMemoryFlagSource()); Node node = nodeRepository.createNode("openStackId", "hostname", Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.tenant); - nodeRepository.addNodes(List.of(node)); + nodeRepository.addNodes(List.of(node), Agent.system); Node hostNode = nodeRepository.createNode("openStackId2", "parent", Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.proxy); - nodeRepository.addNodes(List.of(hostNode)); + nodeRepository.addNodes(List.of(hostNode), Agent.system); Map expectedMetrics = new HashMap<>(); expectedMetrics.put("hostedVespa.provisionedHosts", 1); @@ -150,7 +150,7 @@ public class MetricsReporterTest { Node dockerHost = Node.create("openStackId1", new IP.Config(Set.of("::1"), ipAddressPool), "dockerHost", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), Optional.empty(), NodeType.host); - nodeRepository.addNodes(List.of(dockerHost)); + nodeRepository.addNodes(List.of(dockerHost), Agent.system); nodeRepository.dirtyRecursively("dockerHost", Agent.system, getClass().getSimpleName()); nodeRepository.setReady("dockerHost", Agent.system, getClass().getSimpleName()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java index e0e0a320763..35a93ebe8d7 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java @@ -242,7 +242,7 @@ public class NodeFailTester { for (int i = startIndex; i < startIndex + count; i++) nodes.add(nodeRepository.createNode("node" + i, "host" + i, parentHostname, flavor, nodeType)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); return nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } @@ -251,7 +251,7 @@ public class NodeFailTester { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("parent" + i, "parent" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); return nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java index e2745760637..d833eab9eb0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java @@ -102,7 +102,7 @@ public class OperatorChangeApplicationMaintainerTest { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("node" + i, "host" + i, Optional.empty(), flavor, NodeType.tenant)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } @@ -111,7 +111,7 @@ public class OperatorChangeApplicationMaintainerTest { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("hostNode" + i, "realHost" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } @@ -120,7 +120,7 @@ public class OperatorChangeApplicationMaintainerTest { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("proxyNode" + i, "proxyHost" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.proxy)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java index 5c87684da76..71a2b6b7e4c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java @@ -222,7 +222,7 @@ public class PeriodicApplicationMaintainerTest { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("node" + i, "host" + i, Optional.empty(), flavor, NodeType.tenant)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } @@ -231,7 +231,7 @@ public class PeriodicApplicationMaintainerTest { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("hostNode" + i, "realHost" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java index 83e41211da9..64be8b45f67 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java @@ -54,7 +54,7 @@ public class ReservationExpirerTest { nodes.add(nodeRepository.createNode(UUID.randomUUID().toString(), UUID.randomUUID().toString(), Optional.empty(), new Flavor(new NodeResources(2, 8, 50, 1)), NodeType.tenant)); nodes.add(nodeRepository.createNode(UUID.randomUUID().toString(), UUID.randomUUID().toString(), Optional.empty(), new Flavor(new NodeResources(2, 8, 50, 1)), NodeType.tenant)); nodes.add(nodeRepository.createNode(UUID.randomUUID().toString(), UUID.randomUUID().toString(), Optional.empty(), flavors.getFlavorOrThrow("default"), NodeType.host)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); // Reserve 2 nodes diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java index c82cbafaaa1..c6b74d5eae2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java @@ -216,7 +216,7 @@ public class RetiredExpirerTest { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("node" + i, "node" + i, Optional.empty(), flavor, NodeType.tenant)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } @@ -225,7 +225,7 @@ public class RetiredExpirerTest { List nodes = new ArrayList<>(count); for (int i = 0; i < count; i++) nodes.add(nodeRepository.createNode("parent" + i, "parent" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.host)); - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java index 7eb379aa404..25f61b1088f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java @@ -393,7 +393,6 @@ public class DynamicDockerAllocationTest { NodeResources.DiskSpeed.slow, hosts.get(0).flavor().get().resources().diskSpeed()); } - @SuppressWarnings("deprecation") @Test public void testSwitchingFromLegacyFlavorSyntaxToResourcesDoesNotCauseReallocation() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).flavorsConfig(flavorsConfig()).build(); @@ -430,7 +429,7 @@ public class DynamicDockerAllocationTest { clusterSpec.with(Optional.of(ClusterSpec.Group.from(0))), index); // Need to add group here so that group is serialized in node allocation Node node1aAllocation = node1a.allocate(id, clusterMembership1, node1a.flavor().resources(), Instant.now()); - tester.nodeRepository().addNodes(Collections.singletonList(node1aAllocation)); + tester.nodeRepository().addNodes(Collections.singletonList(node1aAllocation), Agent.system); NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(tester.getCurator())); tester.nodeRepository().activate(Collections.singletonList(node1aAllocation), transaction); transaction.commit(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 03d0298900a..9c2903c7aef 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -305,7 +305,7 @@ public class ProvisioningTester { reservedTo, type)); } - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); return nodes; } @@ -328,7 +328,7 @@ public class ProvisioningTester { nodes.add(node); } - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); @@ -400,7 +400,7 @@ public class ProvisioningTester { nodes.add(nodeRepository.createNode("openstack-id", hostname, parentHostId, new Flavor(flavor), NodeType.tenant)); } - nodes = nodeRepository.addNodes(nodes); + nodes = nodeRepository.addNodes(nodes, Agent.system); nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName()); nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName()); return nodes; 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 74d75d50fc5..fe7c09863e2 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 @@ -124,7 +124,7 @@ public class RestApiTest { assertResponse(new Request("http://localhost:8080/nodes/v2/node", ("[" + asNodeJson("host8.yahoo.com", "default", "127.0.254.8") + "]").getBytes(StandardCharsets.UTF_8), Request.Method.POST), 400, - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot add host8.yahoo.com: A node with this name already exists\"}"); + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot add provisioned host host8.yahoo.com: A node with this name already exists\"}"); // DELETE a provisioned node assertResponse(new Request("http://localhost:8080/nodes/v2/node/host9.yahoo.com", @@ -483,7 +483,7 @@ public class RestApiTest { ("[" + asNodeJson("host8.yahoo.com", "default", "127.0.254.1", "::254:1") + "," + asNodeJson("host8.yahoo.com", "large-variant", "127.0.253.1", "::253:1") + "]").getBytes(StandardCharsets.UTF_8), Request.Method.POST), 400, - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot add host8.yahoo.com: A node with this name already exists\"}"); + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot add nodes: provisioned host host8.yahoo.com is duplicated in the argument list\"}"); // Attempt to PATCH field not relevant for child node assertResponse(new Request("http://localhost:8080/nodes/v2/node/test-node-pool-102-2", -- cgit v1.2.3 From fcedbb36c7caea42e9ab9f6816fdcd28d71aeae9 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 9 Mar 2020 21:26:02 +0100 Subject: Remove hosts on aws --- .../main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'node-repository') 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 9f2bf81b475..5174d1c16d9 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 @@ -608,8 +608,11 @@ public class NodeRepository extends AbstractComponent { List children = list().childrenOf(node).asList(); children.forEach(child -> requireRemovable(child, true, force)); db.removeNodes(children); - move(node, State.deprovisioned, Agent.system, Optional.empty()); List removed = new ArrayList<>(children); + if (zone.cloud().value().equals("aws")) + db.removeNodes(List.of(node)); + else + move(node, State.deprovisioned, Agent.system, Optional.empty()); removed.add(node); return removed; } -- cgit v1.2.3 From 2e8dfe4a5c8bc571262ae4b92feb31cebddc7c5b Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 9 Mar 2020 22:17:00 +0100 Subject: Don't change method name --- .../src/main/java/com/yahoo/config/provision/NodeType.java | 4 ++-- .../vespa/hosted/controller/application/SystemApplication.java | 2 +- .../java/com/yahoo/vespa/hosted/provision/NodeRepository.java | 4 ++-- .../yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java | 2 +- .../com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java | 8 ++++---- .../vespa/hosted/provision/maintenance/OsUpgradeActivator.java | 2 +- .../yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java | 2 +- .../src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java | 2 +- .../main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java | 2 +- .../vespa/hosted/provision/persistence/CuratorDatabaseClient.java | 2 +- .../yahoo/vespa/hosted/provision/provisioning/DockerImages.java | 4 ++-- .../com/yahoo/vespa/hosted/provision/restapi/v2/NodePatcher.java | 2 +- .../yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java | 2 +- 13 files changed, 19 insertions(+), 19 deletions(-) (limited to 'node-repository') diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java index 1d4265172a3..58ae8b95e97 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeType.java @@ -45,7 +45,7 @@ public enum NodeType { this.description = description; } - public boolean isHost() { + public boolean isDockerHost() { return !childNodeTypes.isEmpty(); } @@ -66,7 +66,7 @@ public enum NodeType { * @throws IllegalStateException if this type is not a host */ public List childNodeTypes() { - if (! isHost()) + if (! isDockerHost()) throw new IllegalStateException(this + " has no children"); return childNodeTypes; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java index a0831d3d7ff..66015c76b06 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java @@ -65,7 +65,7 @@ public enum SystemApplication { /** Returns whether this should receive OS upgrades */ public boolean shouldUpgradeOs() { - return nodeType.isHost(); + return nodeType.isDockerHost(); } /** All known system applications */ 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 5174d1c16d9..5a797038f94 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 @@ -465,7 +465,7 @@ public class NodeRepository extends AbstractComponent { new IllegalArgumentException("Could not deallocate " + hostname + ": Node not found")); List nodesToDirty = - (nodeToDirty.type().isHost() ? + (nodeToDirty.type().isDockerHost() ? Stream.concat(list().childrenOf(hostname).asList().stream(), Stream.of(nodeToDirty)) : Stream.of(nodeToDirty)) .filter(node -> node.state() != State.dirty) @@ -604,7 +604,7 @@ public class NodeRepository extends AbstractComponent { try (Mutex lock = lockUnallocated()) { requireRemovable(node, false, force); - if (node.type().isHost()) { + if (node.type().isDockerHost()) { List children = list().childrenOf(node).asList(); children.forEach(child -> requireRemovable(child, true, force)); db.removeNodes(children); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index cc7abfe933b..af37aa5becf 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -100,7 +100,7 @@ public class FailedExpirer extends Maintainer { List nodesToRecycle = new ArrayList<>(); for (Node candidate : nodes) { if (NodeFailer.hasHardwareIssue(candidate, nodeRepository)) { - List unparkedChildren = !candidate.type().isHost() ? Collections.emptyList() : + List unparkedChildren = !candidate.type().isDockerHost() ? Collections.emptyList() : nodeRepository.list().childrenOf(candidate).asList().stream() .filter(node -> node.state() != Node.State.parked) .map(Node::hostname) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java index ce4b161d8b3..b0b8f7d8d2c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java @@ -102,7 +102,7 @@ public class NodeFailer extends Maintainer { for (Map.Entry entry : getReadyNodesByFailureReason().entrySet()) { Node node = entry.getKey(); if (throttle(node)) { - if (node.type().isHost()) throttledHostFailures++; + if (node.type().isDockerHost()) throttledHostFailures++; else throttledNodeFailures++; continue; } @@ -121,7 +121,7 @@ public class NodeFailer extends Maintainer { continue; } if (throttle(node)) { - if (node.type().isHost()) throttledHostFailures++; + if (node.type().isDockerHost()) throttledHostFailures++; else throttledNodeFailures++; continue; } @@ -207,7 +207,7 @@ public class NodeFailer extends Maintainer { nodesByFailureReason.put(node, "Node has been down longer than " + downTimeLimit); } else if (hostSuspended(node, activeNodes)) { Node hostNode = node.parentHostname().flatMap(parent -> nodeRepository().getNode(parent)).orElse(node); - if (hostNode.type().isHost()) { + if (hostNode.type().isDockerHost()) { List failureReports = reasonsToFailParentHost(hostNode); if (failureReports.size() > 0) { if (hostNode.equals(node)) { @@ -237,7 +237,7 @@ public class NodeFailer extends Maintainer { } private boolean expectConfigRequests(Node node) { - return !node.type().isHost(); + return !node.type().isDockerHost(); } private boolean hasNodeRequestedConfigAfter(Node node, Instant instant) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java index a5d3a807e1f..c22bda8a5cf 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OsUpgradeActivator.java @@ -24,7 +24,7 @@ public class OsUpgradeActivator extends Maintainer { @Override protected void maintain() { for (var nodeType : NodeType.values()) { - if (!nodeType.isHost()) continue; + if (!nodeType.isDockerHost()) continue; var active = canUpgradeOsOf(nodeType); nodeRepository().osVersions().setActive(nodeType, active); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java index dfdf4b0fa1f..1d31917b3e1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java @@ -81,7 +81,7 @@ public class RetiredExpirer extends Maintainer { * - Orchestrator allows it */ private boolean canRemove(Node node) { - if (node.type().isHost()) { + if (node.type().isDockerHost()) { if (nodeRepository() .list().childrenOf(node).asList().stream() .allMatch(child -> child.state() == Node.State.parked || diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java index 3a6d684ddd0..aa824e7a930 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java @@ -133,7 +133,7 @@ public class IP { var addresses = new HashSet<>(node.ipConfig().primary()); var otherAddresses = new HashSet<>(other.ipConfig().primary()); - if (node.type().isHost()) { // Addresses of a host can never overlap with any other nodes + if (node.type().isDockerHost()) { // Addresses of a host can never overlap with any other nodes addresses.addAll(node.ipConfig().pool().asSet()); otherAddresses.addAll(other.ipConfig().pool().asSet()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java index dc46eaec744..e10ff3d24cd 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java @@ -141,7 +141,7 @@ public class OsVersions { } private static void require(NodeType nodeType) { - if (!nodeType.isHost()) { + if (!nodeType.isDockerHost()) { throw new IllegalArgumentException("Node type '" + nodeType + "' does not support OS upgrades"); } } 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 9555266966c..87fc2c6323a 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 @@ -245,7 +245,7 @@ public class CuratorDatabaseClient { /** Returns whether to reboot node as part of transition to given state. This is done to get rid of any lingering * unwanted state (e.g. processes) on non-host nodes. */ private boolean rebootOnTransitionTo(Node.State state, Node node) { - if (node.type().isHost()) return false; // Reboot of host nodes is handled by NodeRebooter + if (node.type().isDockerHost()) return false; // Reboot of host nodes is handled by NodeRebooter if (zone.environment().isTest()) return false; // We want to reuse nodes quickly in test environments return node.state() != Node.State.dirty && state == Node.State.dirty; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java index a3beeb51dbb..4416106f23e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerImages.java @@ -62,7 +62,7 @@ public class DockerImages { /** Returns the image to use for given node and zone */ public DockerImage dockerImageFor(Node node) { - if (node.type().isHost()) { + if (node.type().isDockerHost()) { // Docker hosts do not run in containers, and thus has no image. Return the image of the child node type // instead as this allows the host to pre-download the (likely) image its node will run. // @@ -92,7 +92,7 @@ public class DockerImages { /** Set the docker image for nodes of given type */ public void setDockerImage(NodeType nodeType, Optional dockerImage) { - if (nodeType.isHost()) { + if (nodeType.isDockerHost()) { throw new IllegalArgumentException("Setting docker image for " + nodeType + " nodes is unsupported"); } try (Lock lock = db.lockDockerImages()) { 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 3d081997dc2..a060f9c8b55 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 @@ -96,7 +96,7 @@ public class NodePatcher { private List applyFieldRecursive(String name, Inspector value) { switch (name) { case WANT_TO_RETIRE: - List childNodes = node.type().isHost() ? nodes.get().childrenOf(node).asList() : List.of(); + List childNodes = node.type().isDockerHost() ? nodes.get().childrenOf(node).asList() : List.of(); return childNodes.stream() .map(child -> applyField(child, name, value)) .collect(Collectors.toList()); 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 3a9025e9b3e..7f283452538 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 { node.status().osVersion().current().ifPresent(version -> object.setString("currentOsVersion", version.toFullString())); node.status().osVersion().wanted().ifPresent(version -> object.setString("wantedOsVersion", version.toFullString())); node.status().firmwareVerifiedAt().ifPresent(instant -> object.setLong("currentFirmwareCheck", instant.toEpochMilli())); - if (node.type().isHost()) + if (node.type().isDockerHost()) nodeRepository.firmwareChecks().requiredAfter().ifPresent(after -> object.setLong("wantedFirmwareCheck", after.toEpochMilli())); node.status().vespaVersion().ifPresent(version -> object.setString("vespaVersion", version.toFullString())); currentDockerImage(node).ifPresent(dockerImage -> object.setString("currentDockerImage", dockerImage.asString())); -- cgit v1.2.3