diff options
author | Valerij Fredriksen <valerijf@oath.com> | 2018-02-27 11:11:23 +0100 |
---|---|---|
committer | Valerij Fredriksen <valerijf@oath.com> | 2018-02-27 11:11:23 +0100 |
commit | 445b3c40a423a68a042649364cfe75b8bb057d2c (patch) | |
tree | d949e208ace18e2cc51a5a3e7a41dcda48ce7ceb | |
parent | 2e70254963e067ee2eb67fc541ef2ff03f2a1fe6 (diff) |
Check if node-repo is needed against current NodeSpec
2 files changed, 17 insertions, 43 deletions
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 7b0ba1cae58..fcbe4e15213 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 @@ -105,8 +105,6 @@ public class NodeAgentImpl implements NodeAgent { private ContainerState containerState = UNKNOWN; - // The attributes of the last successful node repo attribute update for this node. Used to avoid redundant calls. - private NodeAttributes lastAttributesSet = null; private ContainerNodeSpec lastNodeSpec = null; private CpuUsageReporter lastCpuMetric = new CpuUsageReporter(); @@ -230,7 +228,13 @@ public class NodeAgentImpl implements NodeAgent { } private void updateNodeRepoWithCurrentAttributes(final ContainerNodeSpec nodeSpec) { - final NodeAttributes nodeAttributes = new NodeAttributes() + final NodeAttributes currentNodeAttributes = new NodeAttributes() + .withRestartGeneration(nodeSpec.currentRestartGeneration.orElse(null)) + .withRebootGeneration(nodeSpec.currentRebootGeneration.orElse(0L)) + .withDockerImage(nodeSpec.currentDockerImage.orElse(new DockerImage(""))) + .withVespaVersion(nodeSpec.vespaVersion.orElse("")); + + final NodeAttributes wantedNodeAttributes = new NodeAttributes() .withRestartGeneration(nodeSpec.wantedRestartGeneration.orElse(null)) // update reboot gen with wanted gen if set, we ignore reboot for Docker nodes but // want the two to be equal in node repo @@ -238,18 +242,16 @@ public class NodeAgentImpl implements NodeAgent { .withDockerImage(nodeSpec.wantedDockerImage.filter(node -> containerState != ABSENT).orElse(new DockerImage(""))) .withVespaVersion(nodeSpec.wantedVespaVersion.filter(node -> containerState != ABSENT).orElse("")); - publishStateToNodeRepoIfChanged(nodeAttributes); + publishStateToNodeRepoIfChanged(currentNodeAttributes, wantedNodeAttributes); } - private void publishStateToNodeRepoIfChanged(NodeAttributes currentAttributes) { - // TODO: We should only update if the new current values do not match the node repo's current values - if (!currentAttributes.equals(lastAttributesSet)) { + private void publishStateToNodeRepoIfChanged(NodeAttributes currentAttributes, NodeAttributes wantedAttributes) { + if (!currentAttributes.equals(wantedAttributes)) { logger.info("Publishing new set of attributes to node repo: " - + lastAttributesSet + " -> " + currentAttributes); + + currentAttributes + " -> " + wantedAttributes); addDebugMessage("Publishing new set of attributes to node repo: {" + - lastAttributesSet + "} -> {" + currentAttributes + "}"); - nodeRepository.updateNodeAttributes(hostname, currentAttributes); - lastAttributesSet = currentAttributes; + currentAttributes + "} -> {" + wantedAttributes + "}"); + nodeRepository.updateNodeAttributes(hostname, wantedAttributes); } } 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 693a919ff12..0b9564fad8c 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 @@ -124,14 +124,6 @@ public class NodeAgentImplTest { final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository); // TODO: Verify this isn't run unless 1st time inOrder.verify(dockerOperations, times(1)).resumeNode(eq(containerName)); - // TODO: This should not happen when nothing is changed. Now it happens 1st time through. - inOrder.verify(nodeRepository).updateNodeAttributes( - hostName, - new NodeAttributes() - .withRestartGeneration(restartGeneration) - .withRebootGeneration(rebootGeneration) - .withDockerImage(dockerImage) - .withVespaVersion(vespaVersion)); inOrder.verify(orchestrator).resume(hostName); } @@ -319,12 +311,7 @@ public class NodeAgentImplTest { verify(dockerOperations, never()).removeContainer(any(), any()); verify(orchestrator, never()).resume(any(String.class)); - verify(nodeRepository).updateNodeAttributes( - hostName, new NodeAttributes() - .withRestartGeneration(restartGeneration) - .withRebootGeneration(rebootGeneration) - .withDockerImage(dockerImage) - .withVespaVersion(vespaVersion)); + verify(nodeRepository, never()).updateNodeAttributes(eq(hostName), any()); } @Test @@ -352,12 +339,7 @@ public class NodeAgentImplTest { verify(dockerOperations, never()).createContainer(eq(containerName), eq(nodeSpec)); verify(dockerOperations, never()).startContainer(eq(containerName), eq(nodeSpec)); verify(orchestrator, never()).resume(any(String.class)); - verify(nodeRepository).updateNodeAttributes( - hostName, new NodeAttributes() - .withRestartGeneration(restartGeneration) - .withRebootGeneration(rebootGeneration) - .withDockerImage(new DockerImage("")) - .withVespaVersion("")); + verify(nodeRepository, never()).updateNodeAttributes(eq(hostName), any()); } @Test @@ -386,12 +368,7 @@ public class NodeAgentImplTest { inOrder.verify(dockerOperations, never()).removeContainer(any(), any()); verify(orchestrator, never()).resume(any(String.class)); - verify(nodeRepository).updateNodeAttributes( - hostName, new NodeAttributes() - .withRestartGeneration(restartGeneration) - .withRebootGeneration(rebootGeneration) - .withDockerImage(dockerImage) - .withVespaVersion(vespaVersion)); + verify(nodeRepository, never()).updateNodeAttributes(eq(hostName), any()); } @Test @@ -414,12 +391,7 @@ public class NodeAgentImplTest { nodeAgent.converge(); - verify(nodeRepository).updateNodeAttributes( - hostName, new NodeAttributes() - .withRestartGeneration(restartGeneration) - .withRebootGeneration(rebootGeneration) - .withDockerImage(new DockerImage("")) - .withVespaVersion("")); + verify(nodeRepository, never()).updateNodeAttributes(eq(hostName), any()); } private void nodeRunningContainerIsTakenDownAndCleanedAndRecycled(Node.State nodeState, Optional<Long> wantedRestartGeneration) { |