diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2020-10-01 13:42:24 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-10-01 13:42:24 +0200 |
commit | 6ff60a74e1eab18e180c0127bfab73ecdf3fa22a (patch) | |
tree | 2ed597819b9b4c73a714e9d8893ac60d250643bd | |
parent | 430f72a630a997b6d13bb5870e843d72db6982d4 (diff) | |
parent | b5fb609bd6cfd732d09006d857e2157fa4ad447d (diff) |
Merge pull request #14655 from vespa-engine/mpolden/fix-agent
Store correct agent when adding nodes
11 files changed, 53 insertions, 37 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 77ae6ecf1a7..e8a4620b520 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 @@ -428,7 +428,7 @@ public class NodeRepository extends AbstractComponent { existing.get() + ", " + existing.get().history() + "). Node to be added: " + node + ", " + node.history()); } - return db.addNodesInState(nodes.asList(), State.reserved); + return db.addNodesInState(nodes.asList(), State.reserved, Agent.system); } /** @@ -463,8 +463,7 @@ public class NodeRepository extends AbstractComponent { nodesToAdd.add(node); } - List<Node> resultingNodes = new ArrayList<>(); - resultingNodes.addAll(db.addNodesInState(IP.Config.verify(nodesToAdd, list(lock)), State.provisioned)); + List<Node> resultingNodes = db.addNodesInState(IP.Config.verify(nodesToAdd, list(lock)), State.provisioned, agent); db.removeNodes(nodesToRemove); return resultingNodes; } 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 56f7c951025..160ab86591a 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 @@ -108,17 +108,15 @@ public class CuratorDatabaseClient { provisionIndexCounter.initialize(100); } - /** - * Adds a set of nodes. Rollbacks/fails transaction if any node is not in the expected state. - */ - public List<Node> addNodesInState(List<Node> nodes, Node.State expectedState) { + /** Adds a set of nodes. Rollbacks/fails transaction if any node is not in the expected state. */ + public List<Node> addNodesInState(List<Node> nodes, Node.State expectedState, Agent agent) { NestedTransaction transaction = new NestedTransaction(); CuratorTransaction curatorTransaction = db.newCuratorTransactionIn(transaction); for (Node node : nodes) { if (node.state() != expectedState) throw new IllegalArgumentException(node + " is not in the " + expectedState + " state"); - node = node.with(node.history().recordStateTransition(null, expectedState, Agent.system, clock.instant())); + node = node.with(node.history().recordStateTransition(null, expectedState, agent, clock.instant())); curatorTransaction.add(CuratorOperations.create(toPath(node).getAbsolute(), nodeSerializer.toJson(node))); } transaction.commit(); 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 ad3f620d9bd..cfc727f72c5 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 @@ -36,18 +36,23 @@ import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; +import static org.junit.Assert.assertTrue; + /** * @author mgimle */ public class CapacityCheckerTester { public static final Zone zone = new Zone(Environment.prod, RegionName.from("us-east")); + private static final ApplicationId tenantHostApp = ApplicationId.from("hosted-vespa", "tenant-host", "default"); // Components with state public final ManualClock clock = new ManualClock(); @@ -115,10 +120,10 @@ public class CapacityCheckerTester { return distinctChildren; } - List<Node> createHostsWithChildren(int childrenPerHost, List<NodeModel> distinctChildren, int amount, NodeResources excessCapacity, int excessIps) { - List<Node> hosts = new ArrayList<>(); + Map<ApplicationId, List<Node>> createHostsWithChildren(int childrenPerHost, List<NodeModel> distinctChildren, int hostCount, NodeResources excessCapacity, int excessIps) { + Map<ApplicationId, List<Node>> hosts = new HashMap<>(); int j = 0; - for (int i = 0; i < amount; i++) { + for (int i = 0; i < hostCount; i++) { String parentRoot = ".not.a.real.hostname.yahoo.com"; String parentName = "parent" + i; String hostname = parentName + parentRoot; @@ -135,7 +140,8 @@ public class CapacityCheckerTester { Node childNode = createNodeFromModel(childModel); childResources.add(childNode.resources()); - hosts.add(childNode); + hosts.computeIfAbsent(childNode.allocation().get().owner(), (ignored) -> new ArrayList<>()) + .add(childNode); } final int hostindex = i; @@ -147,7 +153,8 @@ public class CapacityCheckerTester { Node node = nodeRepository.createNode(hostname, hostname, new IP.Config(Set.of("::"), availableIps), Optional.empty(), new Flavor(nr), Optional.empty(), NodeType.host); - hosts.add(node); + hosts.computeIfAbsent(tenantHostApp, (ignored) -> new ArrayList<>()) + .add(node); } return hosts; } @@ -159,13 +166,13 @@ public class CapacityCheckerTester { String parentName = "parent" + i; String hostname = parentName + parentRoot; - final int hostid = i; - Set<String> availableIps = IntStream.range(0, ips) - .mapToObj(n -> String.format("%04X::%04X", hostid, n)) + final int hostId = i; + Set<String> availableIps = IntStream.range(2000, 2000 + ips) + .mapToObj(n -> String.format("%04X::%04X", hostId, n)) .collect(Collectors.toSet()); Node node = nodeRepository.createNode(hostname, hostname, - new IP.Config(Set.of("::"), availableIps), Optional.empty(), + new IP.Config(Set.of("::" + (1000 + hostId)), availableIps), Optional.empty(), new Flavor(capacity), Optional.empty(), NodeType.host); hosts.add(node); } @@ -187,12 +194,14 @@ public class CapacityCheckerTester { int numHosts, NodeResources hostExcessCapacity, int hostExcessIps, int numEmptyHosts, NodeResources emptyHostExcessCapacity, int emptyHostExcessIps) { List<NodeModel> possibleChildren = createDistinctChildren(numDistinctChildren, childResources); + Map<ApplicationId, List<Node>> hostsWithChildren = createHostsWithChildren(childrenPerHost, possibleChildren, numHosts, hostExcessCapacity, hostExcessIps); + nodeRepository.addNodes(hostsWithChildren.getOrDefault(tenantHostApp, List.of()), Agent.system); + hostsWithChildren.forEach((applicationId, nodes) -> { + if (applicationId.equals(tenantHostApp)) return; + nodeRepository.addNodes(nodes, Agent.system); + }); + nodeRepository.addNodes(createEmptyHosts(numHosts, numEmptyHosts, emptyHostExcessCapacity, emptyHostExcessIps), Agent.system); - List<Node> nodes = new ArrayList<>(); - nodes.addAll(createHostsWithChildren(childrenPerHost, possibleChildren, numHosts, hostExcessCapacity, hostExcessIps)); - nodes.addAll(createEmptyHosts(numHosts, numEmptyHosts, emptyHostExcessCapacity, emptyHostExcessIps)); - - nodeRepository.addNodes(nodes, Agent.system); updateCapacityChecker(); } @@ -292,16 +301,26 @@ public class CapacityCheckerTester { ObjectMapper om = new ObjectMapper(); NodeRepositoryModel repositoryModel = om.readValue(jsonData, NodeRepositoryModel.class); - List<NodeModel> nmods = repositoryModel.nodes; + List<NodeModel> nodeModels = repositoryModel.nodes; - List<Node> nodes = new ArrayList<>(); - for (var nmod : nmods) { - if (nmod.type != NodeType.host && nmod.type != NodeType.tenant) continue; - nodes.add(createNodeFromModel(nmod)); + List<Node> hosts = new ArrayList<>(); + Map<ApplicationId, List<Node>> nodes = new HashMap<>(); + for (var model : nodeModels) { + Node node = createNodeFromModel(model); + assertTrue("Node is allocated", node.allocation().isPresent()); + if (model.type == NodeType.host) { + hosts.add(node); + } else if (model.type == NodeType.tenant) { + nodes.computeIfAbsent(node.allocation().get().owner(), (k) -> new ArrayList<>()) + .add(node); + } } - nodeRepository.addNodes(nodes, Agent.system); + nodeRepository.addNodes(hosts, Agent.system); + nodes.forEach((application, applicationNodes) -> { + nodeRepository.addNodes(applicationNodes, Agent.system); + }); updateCapacityChecker(); } 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 7cc81f218ec..8b3b6aee5c1 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 @@ -249,7 +249,7 @@ public class DynamicProvisioningMaintainerTest { createNode("proxyhost1", Optional.empty(), NodeType.proxyhost, Node.State.provisioned, Optional.empty()), createNode("proxyhost2", Optional.empty(), NodeType.proxyhost, Node.State.active, Optional.of(proxyHostApp)), createNode("proxy2", Optional.of("proxyhost2"), NodeType.proxy, Node.State.active, Optional.of(proxyApp))) - .forEach(node -> nodeRepository.database().addNodesInState(List.of(node), node.state())); + .forEach(node -> nodeRepository.database().addNodesInState(List.of(node), node.state(), Agent.system)); return this; } @@ -259,7 +259,7 @@ public class DynamicProvisioningMaintainerTest { private Node addNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, ApplicationId application) { Node node = createNode(hostname, parentHostname, nodeType, state, Optional.ofNullable(application)); - return nodeRepository.database().addNodesInState(List.of(node), node.state()).get(0); + return nodeRepository.database().addNodesInState(List.of(node), node.state(), Agent.system).get(0); } private Node createNode(String hostname, Optional<String> parentHostname, NodeType nodeType, Node.State state, Optional<ApplicationId> application) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index 29a121b8a21..88f5d41a4f0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -273,7 +273,7 @@ public class LoadBalancerProvisionerTest { nodeType); nodes.add(node); } - nodes = tester.nodeRepository().database().addNodesInState(nodes, Node.State.reserved); + nodes = tester.nodeRepository().database().addNodesInState(nodes, Node.State.reserved, Agent.system); nodes = tester.nodeRepository().setDirty(nodes, Agent.system, getClass().getSimpleName()); tester.nodeRepository().setReady(nodes, Agent.system, getClass().getSimpleName()); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/controller1.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/controller1.json index 035e15ead49..413c1af0c71 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/controller1.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/controller1.json @@ -18,7 +18,7 @@ { "event": "provisioned", "at": 123, - "agent": "system" + "agent": "operator" } ], "ipAddresses": [ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json index 17f8210fa4d..baf0325b284 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node11.json @@ -18,7 +18,7 @@ { "event": "provisioned", "at": 123, - "agent": "system" + "agent": "operator" } ], "ipAddresses": [ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node8.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node8.json index aa2b2acdb9f..8951d1905ab 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node8.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node8.json @@ -18,7 +18,7 @@ { "event": "provisioned", "at": 123, - "agent": "system" + "agent": "operator" } ], "ipAddresses": [ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node9.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node9.json index a5672f25d57..dac9fd30267 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node9.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/node9.json @@ -18,7 +18,7 @@ { "event": "provisioned", "at": 123, - "agent": "system" + "agent": "operator" } ], "ipAddresses": [ diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/parent2.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/parent2.json index ecc497172c7..2e20d1a8f35 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/parent2.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/parent2.json @@ -19,7 +19,7 @@ { "event": "provisioned", "at": 123, - "agent": "system" + "agent": "operator" } ], "ipAddresses": [ diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index 37b1fa1c9fb..6cbfa274c56 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -313,7 +313,7 @@ public class Curator implements AutoCloseable { try { return framework().getChildren().forPath(path.getAbsolute()); } catch (KeeperException.NoNodeException e) { - return List.of(); + return List.of(); } catch (Exception e) { throw new RuntimeException("Could not get children of " + path.getAbsolute(), e); } |