diff options
Diffstat (limited to 'node-admin')
3 files changed, 32 insertions, 82 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java index 9a94231437d..91ff159ac41 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java @@ -130,6 +130,6 @@ public class NodeAttributes { wantToDeprovision.map(depr -> "wantToDeprovision=" + depr)) .filter(Optional::isPresent) .map(Optional::get) - .collect(Collectors.joining(", ", "NodeAttributes{", "}")); + .collect(Collectors.joining(", ", "{", "}")); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index ef137c55ffb..d85344dffd2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -33,6 +33,7 @@ import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -236,26 +237,33 @@ public class NodeAgentImpl implements NodeAgent { } private void updateNodeRepoWithCurrentAttributes(final NodeSpec node) { - final NodeAttributes currentNodeAttributes = new NodeAttributes() - .withRestartGeneration(node.getCurrentRestartGeneration()) - .withRebootGeneration(node.getCurrentRebootGeneration()) - .withDockerImage(node.getCurrentDockerImage().orElse(new DockerImage(""))); - - final NodeAttributes wantedNodeAttributes = new NodeAttributes() - .withRestartGeneration(node.getWantedRestartGeneration()) - // update reboot gen with wanted gen if set, we ignore reboot for Docker nodes but - // want the two to be equal in node repo - .withRebootGeneration(node.getWantedRebootGeneration()) - .withDockerImage(node.getWantedDockerImage().filter(n -> containerState == UNKNOWN).orElse(new DockerImage(""))); - - publishStateToNodeRepoIfChanged(currentNodeAttributes, wantedNodeAttributes); + final NodeAttributes currentNodeAttributes = new NodeAttributes(); + final NodeAttributes newNodeAttributes = new NodeAttributes(); + + if (!Objects.equals(node.getCurrentRestartGeneration(), node.getWantedRestartGeneration())) { + currentNodeAttributes.withRestartGeneration(node.getCurrentRestartGeneration()); + newNodeAttributes.withRestartGeneration(node.getWantedRestartGeneration()); + } + + if (!Objects.equals(node.getCurrentRebootGeneration(), node.getWantedRebootGeneration())) { + currentNodeAttributes.withRebootGeneration(node.getCurrentRebootGeneration()); + newNodeAttributes.withRebootGeneration(node.getWantedRebootGeneration()); + } + + Optional<DockerImage> actualDockerImage = node.getWantedDockerImage().filter(n -> containerState == UNKNOWN); + if (!Objects.equals(node.getCurrentDockerImage(), actualDockerImage)) { + currentNodeAttributes.withDockerImage(node.getCurrentDockerImage().orElse(new DockerImage(""))); + newNodeAttributes.withDockerImage(actualDockerImage.orElse(new DockerImage(""))); + } + + publishStateToNodeRepoIfChanged(currentNodeAttributes, newNodeAttributes); } - private void publishStateToNodeRepoIfChanged(NodeAttributes currentAttributes, NodeAttributes wantedAttributes) { - if (!currentAttributes.equals(wantedAttributes)) { + private void publishStateToNodeRepoIfChanged(NodeAttributes currentAttributes, NodeAttributes newAttributes) { + if (!currentAttributes.equals(newAttributes)) { context.log(logger, "Publishing new set of attributes to node repo: %s -> %s", - currentAttributes, wantedAttributes); - nodeRepository.updateNodeAttributes(context.hostname().value(), wantedAttributes); + currentAttributes, newAttributes); + nodeRepository.updateNodeAttributes(context.hostname().value(), newAttributes); } } @@ -285,8 +293,8 @@ public class NodeAgentImpl implements NodeAgent { private Optional<String> shouldRestartServices(NodeSpec node) { if (!node.getWantedRestartGeneration().isPresent()) return Optional.empty(); - if (!node.getCurrentRestartGeneration().isPresent() || - node.getCurrentRestartGeneration().get() < node.getWantedRestartGeneration().get()) { + // Restart generation is only optional because it does not exist for unallocated nodes + if (node.getCurrentRestartGeneration().get() < node.getWantedRestartGeneration().get()) { return Optional.of("Restart requested - wanted restart generation has been bumped: " + node.getCurrentRestartGeneration().get() + " -> " + node.getWantedRestartGeneration().get()); } @@ -466,7 +474,6 @@ public class NodeAgentImpl implements NodeAgent { new IllegalStateException(String.format("Node '%s' missing from node repository", context.hostname()))); expectNodeNotInNodeRepo = false; - Optional<Container> container = getContainer(); if (!node.equals(lastNode)) { // Every time the node spec changes, we should clear the metrics for this container as the dimensions diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 4e90c8f5a13..648d1f96094 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -87,17 +87,12 @@ public class NodeAgentImplTest { @Test public void upToDateContainerIsUntouched() { - final long restartGeneration = 1; - final long rebootGeneration = 0; final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .currentDockerImage(dockerImage) .state(Node.State.active) .wantedVespaVersion(vespaVersion) .vespaVersion(vespaVersion) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) - .wantedRebootGeneration(rebootGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -120,17 +115,12 @@ public class NodeAgentImplTest { @Test public void verifyRemoveOldFilesIfDiskFull() { - final long restartGeneration = 1; - final long rebootGeneration = 0; final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .currentDockerImage(dockerImage) .state(Node.State.active) .wantedVespaVersion(vespaVersion) .vespaVersion(vespaVersion) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) - .wantedRebootGeneration(rebootGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -181,14 +171,12 @@ public class NodeAgentImplTest { @Test public void absentContainerCausesStart() { final Optional<Long> restartGeneration = Optional.of(1L); - final long rebootGeneration = 0; final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .state(Node.State.active) .wantedVespaVersion(vespaVersion) .wantedRestartGeneration(restartGeneration.get()) .currentRestartGeneration(restartGeneration.get()) - .wantedRebootGeneration(rebootGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(null, false); @@ -210,26 +198,19 @@ public class NodeAgentImplTest { inOrder.verify(aclMaintainer, times(1)).converge(); inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context)); inOrder.verify(nodeRepository).updateNodeAttributes( - hostName, new NodeAttributes() - .withRestartGeneration(restartGeneration) - .withRebootGeneration(rebootGeneration) - .withDockerImage(dockerImage)); + hostName, new NodeAttributes().withDockerImage(dockerImage)); inOrder.verify(orchestrator).resume(hostName); } @Test public void containerIsNotStoppedIfNewImageMustBePulled() { final DockerImage newDockerImage = new DockerImage("new-image"); - final long wantedRestartGeneration = 2; - final long currentRestartGeneration = 1; final NodeSpec node = nodeBuilder .wantedDockerImage(newDockerImage) .currentDockerImage(dockerImage) .state(Node.State.active) .wantedVespaVersion(vespaVersion) .vespaVersion(vespaVersion) - .wantedRestartGeneration(wantedRestartGeneration) - .currentRestartGeneration(currentRestartGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -250,16 +231,12 @@ public class NodeAgentImplTest { @Test public void containerIsRestartedIfFlavorChanged() { - final long wantedRestartGeneration = 1; - final long currentRestartGeneration = 1; NodeSpec.Builder specBuilder = nodeBuilder .wantedDockerImage(dockerImage) .currentDockerImage(dockerImage) .state(Node.State.active) .wantedVespaVersion(vespaVersion) - .vespaVersion(vespaVersion) - .wantedRestartGeneration(wantedRestartGeneration) - .currentRestartGeneration(currentRestartGeneration); + .vespaVersion(vespaVersion); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); NodeSpec firstSpec = specBuilder.build(); @@ -316,15 +293,12 @@ public class NodeAgentImplTest { @Test public void failedNodeRunningContainerShouldStillBeRunning() { - final long restartGeneration = 1; final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .currentDockerImage(dockerImage) .state(Node.State.failed) .wantedVespaVersion(vespaVersion) .vespaVersion(vespaVersion) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -340,13 +314,8 @@ public class NodeAgentImplTest { @Test public void readyNodeLeadsToNoAction() { - final long restartGeneration = 1; - final long rebootGeneration = 0; final NodeSpec node = nodeBuilder .state(Node.State.ready) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) - .wantedRebootGeneration(rebootGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(null,false); @@ -368,18 +337,12 @@ public class NodeAgentImplTest { @Test public void inactiveNodeRunningContainerShouldStillBeRunning() { - final long restartGeneration = 1; - final long rebootGeneration = 0; - final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .currentDockerImage(dockerImage) .state(Node.State.inactive) .wantedVespaVersion(vespaVersion) .vespaVersion(vespaVersion) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) - .wantedRebootGeneration(rebootGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -397,16 +360,10 @@ public class NodeAgentImplTest { @Test public void reservedNodeDoesNotUpdateNodeRepoWithVersion() { - final long restartGeneration = 1; - final long rebootGeneration = 0; - final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .state(Node.State.reserved) .wantedVespaVersion(vespaVersion) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) - .wantedRebootGeneration(rebootGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(null, false); @@ -450,10 +407,7 @@ public class NodeAgentImplTest { verify(orchestrator, never()).suspend(any(String.class)); // current Docker image and vespa version should be cleared verify(nodeRepository, times(1)).updateNodeAttributes( - any(String.class), eq(new NodeAttributes() - .withRestartGeneration(wantedRestartGeneration) - .withRebootGeneration(0L) - .withDockerImage(new DockerImage("")))); + eq(hostName), eq(new NodeAttributes().withDockerImage(new DockerImage("")))); } @Test @@ -503,14 +457,11 @@ public class NodeAgentImplTest { @Test public void resumeProgramRunsUntilSuccess() { - final long restartGeneration = 1; final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .currentDockerImage(dockerImage) .state(Node.State.active) .vespaVersion(vespaVersion) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) .build(); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -569,15 +520,10 @@ public class NodeAgentImplTest { @Test public void start_container_subtask_failure_leads_to_container_restart() { - final long restartGeneration = 1; - final long rebootGeneration = 0; final NodeSpec node = nodeBuilder .wantedDockerImage(dockerImage) .state(Node.State.active) .wantedVespaVersion(vespaVersion) - .wantedRestartGeneration(restartGeneration) - .currentRestartGeneration(restartGeneration) - .wantedRebootGeneration(rebootGeneration) .build(); NodeAgentImpl nodeAgent = spy(makeNodeAgent(null, false)); @@ -694,7 +640,6 @@ public class NodeAgentImplTest { @Test public void testRunningConfigServer() { - final long rebootGeneration = 0; final NodeSpec node = nodeBuilder .nodeType(NodeType.config) .wantedDockerImage(dockerImage) @@ -720,9 +665,7 @@ public class NodeAgentImplTest { inOrder.verify(aclMaintainer, times(1)).converge(); inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context)); inOrder.verify(nodeRepository).updateNodeAttributes( - hostName, new NodeAttributes() - .withRebootGeneration(rebootGeneration) - .withDockerImage(dockerImage)); + hostName, new NodeAttributes().withDockerImage(dockerImage)); inOrder.verify(orchestrator).resume(hostName); } |