diff options
3 files changed, 20 insertions, 22 deletions
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java index f1f40a6726e..ffd5dffece9 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerResources.java @@ -120,4 +120,8 @@ public class ContainerResources { public String toString() { return toStringCpu() + ", " + toStringMemory(); } + + public ContainerResources withMemoryBytes(long memoryBytes) { + return new ContainerResources(cpus, cpuShares, memoryBytes); + } } 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 0e651383adc..407814f3b4d 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 @@ -213,7 +213,10 @@ public class NodeAgentImpl implements NodeAgent { shouldRestartServices(context, existingContainer.get()).ifPresent(restartReason -> { context.log(logger, "Will restart services: " + restartReason); - restartServices(context, existingContainer.get()); + orchestratorSuspendNode(context); + + dockerOperations.restartVespa(context); + context.log(logger, "Restarted services"); currentRestartGeneration = context.node().wantedRestartGeneration(); }); } @@ -223,7 +226,7 @@ public class NodeAgentImpl implements NodeAgent { private Optional<String> shouldRestartServices( NodeAgentContext context, Container existingContainer) { NodeSpec node = context.node(); - if (node.wantedRestartGeneration().isEmpty()) return Optional.empty(); + if (!existingContainer.state.isRunning() || node.state() != NodeState.active) return Optional.empty(); // Restart generation is only optional because it does not exist for unallocated nodes if (currentRestartGeneration.get() < node.wantedRestartGeneration().get()) { @@ -231,25 +234,9 @@ public class NodeAgentImpl implements NodeAgent { + currentRestartGeneration.get() + " -> " + node.wantedRestartGeneration().get()); } - // Restart services if wanted memory changes (searchnode and container needs to be restarted to pick up changes) - ContainerResources wantedContainerResources = getContainerResources(context); - if (!wantedContainerResources.equalsMemory(existingContainer.resources)) { - return Optional.of("Container should be running with different memory allocation, wanted: " + - wantedContainerResources.toStringMemory() + ", actual: " + existingContainer.resources.toStringMemory()); - } - return Optional.empty(); } - private void restartServices(NodeAgentContext context, Container existingContainer) { - if (existingContainer.state.isRunning() && context.node().state() == NodeState.active) { - context.log(logger, "Restarting services"); - // Since we are restarting the services we need to suspend the node. - orchestratorSuspendNode(context); - dockerOperations.restartVespa(context); - } - } - private void stopServicesIfNeeded(NodeAgentContext context) { if (hasStartedServices && context.node().owner().isEmpty()) stopServices(context); @@ -305,6 +292,12 @@ public class NodeAgentImpl implements NodeAgent { currentRebootGeneration, context.node().wantedRebootGeneration())); } + ContainerResources wantedContainerResources = getContainerResources(context); + if (!wantedContainerResources.equalsMemory(existingContainer.resources)) { + return Optional.of("Container should be running with different memory allocation, wanted: " + + wantedContainerResources.toStringMemory() + ", actual: " + existingContainer.resources.toStringMemory()); + } + if (containerState == STARTING) return Optional.of("Container failed to start"); return Optional.empty(); } @@ -343,7 +336,8 @@ public class NodeAgentImpl implements NodeAgent { orchestratorSuspendNode(context); - dockerOperations.updateContainer(context, wantedContainerResources); + // Only update CPU resources + dockerOperations.updateContainer(context, wantedContainerResources.withMemoryBytes(existingContainer.resources.memoryBytes())); } private ContainerResources getContainerResources(NodeAgentContext context) { 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 bf5e5a67a79..465a430b497 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 @@ -259,7 +259,7 @@ public class NodeAgentImplTest { } @Test - public void vespaIsRestartedIfMemoryChanged() { + public void containerIsRecreatedIfMemoryChanged() { NodeSpec.Builder specBuilder = nodeBuilder .state(NodeState.active) .wantedDockerImage(dockerImage).currentDockerImage(dockerImage) @@ -280,9 +280,9 @@ public class NodeAgentImplTest { InOrder inOrder = inOrder(orchestrator, dockerOperations); inOrder.verify(orchestrator).resume(any(String.class)); - inOrder.verify(dockerOperations, never()).removeContainer(any(), any()); + inOrder.verify(dockerOperations).removeContainer(eq(secondContext), any()); inOrder.verify(dockerOperations, never()).updateContainer(any(), any()); - inOrder.verify(dockerOperations).restartVespa(eq(secondContext)); + inOrder.verify(dockerOperations, never()).restartVespa(any()); nodeAgent.doConverge(secondContext); inOrder.verify(orchestrator).resume(any(String.class)); |