diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2021-09-13 12:51:51 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-09-13 12:51:51 +0200 |
commit | 37dcc0dab402b6a12f59f2944bd95aafa7c11cb3 (patch) | |
tree | 7d1212803b85d1f32d34383e0785a78bc0cc9d17 | |
parent | 49e6cbb29ec21de114006052f70fa28123118263 (diff) | |
parent | 9f9a3da8d13802afcf0eaecb633bfee0ab24ee21 (diff) |
Merge pull request #19087 from vespa-engine/freva/remove-node-agent
Ensure unique NodeAgent ID for reallocations
5 files changed, 112 insertions, 4 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Event.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Event.java new file mode 100644 index 00000000000..ca374533940 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Event.java @@ -0,0 +1,54 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; + +import java.time.Instant; +import java.util.Objects; + +/** + * @author freva + */ +public class Event { + private final String agent; + private final String type; + private final Instant at; + + public Event(String agent, String type, Instant at) { + this.agent = Objects.requireNonNull(agent); + this.type = Objects.requireNonNull(type); + this.at = Objects.requireNonNull(at); + } + + public String agent() { + return agent; + } + + public String type() { + return type; + } + + public Instant at() { + return at; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Event event1 = (Event) o; + return agent.equals(event1.agent) && type.equals(event1.type) && at.equals(event1.at); + } + + @Override + public int hashCode() { + return Objects.hash(agent, type, at); + } + + @Override + public String toString() { + return "Event{" + + "agent='" + agent + '\'' + + ", type='" + type + '\'' + + ", at=" + at + + '}'; + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index 30bc1ef5ea3..e85d51ef992 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.DiskSize; import java.net.URI; import java.time.Instant; import java.util.EnumSet; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -60,6 +61,7 @@ public class NodeSpec { private final Set<String> additionalIpAddresses; private final NodeReports reports; + private final List<Event> events; private final Optional<String> parentHostname; private final Optional<URI> archiveUri; @@ -93,6 +95,7 @@ public class NodeSpec { Set<String> ipAddresses, Set<String> additionalIpAddresses, NodeReports reports, + List<Event> events, Optional<String> parentHostname, Optional<URI> archiveUri, Optional<ApplicationId> exclusiveTo) { @@ -128,9 +131,10 @@ public class NodeSpec { this.currentFirmwareCheck = Objects.requireNonNull(currentFirmwareCheck); this.resources = Objects.requireNonNull(resources); this.realResources = Objects.requireNonNull(realResources); - this.ipAddresses = Objects.requireNonNull(ipAddresses); - this.additionalIpAddresses = Objects.requireNonNull(additionalIpAddresses); + this.ipAddresses = Set.copyOf(ipAddresses); + this.additionalIpAddresses = Set.copyOf(additionalIpAddresses); this.reports = Objects.requireNonNull(reports); + this.events = List.copyOf(events); this.parentHostname = Objects.requireNonNull(parentHostname); this.archiveUri = Objects.requireNonNull(archiveUri); this.exclusiveTo = Objects.requireNonNull(exclusiveTo); @@ -263,6 +267,10 @@ public class NodeSpec { public NodeReports reports() { return reports; } + public List<Event> events() { + return events; + } + public Optional<String> parentHostname() { return parentHostname; } @@ -308,6 +316,7 @@ public class NodeSpec { Objects.equals(ipAddresses, that.ipAddresses) && Objects.equals(additionalIpAddresses, that.additionalIpAddresses) && Objects.equals(reports, that.reports) && + Objects.equals(events, that.events) && Objects.equals(parentHostname, that.parentHostname) && Objects.equals(archiveUri, that.archiveUri) && Objects.equals(exclusiveTo, that.exclusiveTo); @@ -342,6 +351,7 @@ public class NodeSpec { ipAddresses, additionalIpAddresses, reports, + events, parentHostname, archiveUri, exclusiveTo); @@ -376,6 +386,7 @@ public class NodeSpec { + " ipAddresses=" + ipAddresses + " additionalIpAddresses=" + additionalIpAddresses + " reports=" + reports + + " events=" + events + " parentHostname=" + parentHostname + " archiveUri=" + archiveUri + " exclusiveTo=" + exclusiveTo @@ -409,6 +420,7 @@ public class NodeSpec { private Set<String> ipAddresses = Set.of(); private Set<String> additionalIpAddresses = Set.of(); private NodeReports reports = new NodeReports(); + private List<Event> events = List.of(); private Optional<String> parentHostname = Optional.empty(); private Optional<URI> archiveUri = Optional.empty(); private Optional<ApplicationId> exclusiveTo = Optional.empty(); @@ -428,6 +440,7 @@ public class NodeSpec { currentRebootGeneration(node.currentRebootGeneration); orchestratorStatus(node.orchestratorStatus); reports(new NodeReports(node.reports)); + events(node.events); node.wantedDockerImage.ifPresent(this::wantedDockerImage); node.currentDockerImage.ifPresent(this::currentDockerImage); node.wantedVespaVersion.ifPresent(this::wantedVespaVersion); @@ -600,6 +613,11 @@ public class NodeSpec { return this; } + public Builder events(List<Event> events) { + this.events = events; + return this; + } + public Builder parentHostname(String parentHostname) { this.parentHostname = Optional.of(parentHostname); return this; @@ -714,6 +732,10 @@ public class NodeSpec { return reports; } + public List<Event> events() { + return events; + } + public Optional<String> parentHostname() { return parentHostname; } @@ -730,7 +752,7 @@ public class NodeSpec { wantedRebootGeneration, currentRebootGeneration, wantedFirmwareCheck, currentFirmwareCheck, modelName, resources, realResources, ipAddresses, additionalIpAddresses, - reports, parentHostname, archiveUri, exclusiveTo); + reports, events, parentHostname, archiveUri, exclusiveTo); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index 50c39d5407c..0ea9be96fd7 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -178,6 +178,7 @@ public class RealNodeRepository implements NodeRepository { node.ipAddresses, node.additionalIpAddresses, reports, + node.history.stream().map(event -> new Event(event.agent, event.event, Instant.ofEpochMilli(event.at))).collect(Collectors.toUnmodifiableList()), Optional.ofNullable(node.parentHostname), Optional.ofNullable(node.archiveUri).map(URI::create), Optional.ofNullable(node.exclusiveTo).map(ApplicationId::fromSerializedForm)); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java index 86caab9bf51..1165dc9f558 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.JsonNode; +import java.util.List; import java.util.Map; import java.util.Set; @@ -82,6 +83,8 @@ public class NodeRepositoryNode { public String archiveUri; @JsonProperty("exclusiveTo") public String exclusiveTo; + @JsonProperty("history") + public List<Event> history; @JsonProperty("reports") public Map<String, JsonNode> reports = null; @@ -123,6 +126,7 @@ public class NodeRepositoryNode { ", archiveUri=" + archiveUri + ", reports=" + reports + ", exclusiveTo=" + exclusiveTo + + ", history=" + history + '}'; } @@ -198,4 +202,21 @@ public class NodeRepositoryNode { } } + public static class Event { + @JsonProperty + public String agent; + @JsonProperty + public String event; + @JsonProperty + public long at; + + @Override + public String toString() { + return "Event{" + + "agent=" + agent + + ", event=" + event + + ", at=" + at + + '}'; + } + } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index 8c8f3d88a71..36668158dd6 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -79,7 +79,7 @@ public class NodeAdminImpl implements NodeAdmin { @Override public void refreshContainersToRun(Set<NodeAgentContext> nodeAgentContexts) { Map<String, NodeAgentContext> nodeAgentContextsByHostname = nodeAgentContexts.stream() - .collect(Collectors.toMap(nac -> nac.hostname().value(), Function.identity())); + .collect(Collectors.toMap(NodeAdminImpl::nodeAgentId, Function.identity())); // Stop and remove NodeAgents that should no longer be running diff(nodeAgentWithSchedulerByHostname.keySet(), nodeAgentContextsByHostname.keySet()) @@ -222,4 +222,14 @@ public class NodeAdminImpl implements NodeAdmin { NodeAgent nodeAgent = nodeAgentFactory.create(contextManager, context); return new NodeAgentWithScheduler(nodeAgent, contextManager); } + + private static String nodeAgentId(NodeAgentContext nac) { + // NodeAgentImpl has some internal state that should not be reused when the same hostname is re-allocated + // to a different application/cluster, solve this by including reservation timestamp in the key. + return nac.hostname().value() + "-" + nac.node().events().stream() + .filter(event -> "reserved".equals(event.type())) + .findFirst() + .map(event -> Long.toString(event.at().toEpochMilli())) + .orElse(""); + } } |