Restart vespa if wanted memory changes

Restart instead of removing and starting Docker container

2 files changed, 18 insertions, 19 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 ac1946c1fd8..480105f9076 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
@@ -210,7 +210,7 @@ public class NodeAgentImpl implements NodeAgent {
                 return Optional.empty();
             }
 
-            shouldRestartServices(context.node()).ifPresent(restartReason -> {
+            shouldRestartServices(context, existingContainer.get()).ifPresent(restartReason -> {
                 context.log(logger, "Will restart services: " + restartReason);
                 restartServices(context, existingContainer.get());
                 currentRestartGeneration = context.node().wantedRestartGeneration();
@@ -220,14 +220,23 @@ public class NodeAgentImpl implements NodeAgent {
         return existingContainer;
     }
 
-    private Optional<String> shouldRestartServices(NodeSpec node) {
-        if (!node.wantedRestartGeneration().isPresent()) return Optional.empty();
+    private Optional<String> shouldRestartServices( NodeAgentContext context, Container existingContainer) {
+        NodeSpec node = context.node();
+        if (node.wantedRestartGeneration().isEmpty()) return Optional.empty();
 
         // Restart generation is only optional because it does not exist for unallocated nodes
         if (currentRestartGeneration.get() < node.wantedRestartGeneration().get()) {
             return Optional.of("Restart requested - wanted restart generation has been bumped: "
                     + 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();
     }
 
@@ -290,16 +299,6 @@ public class NodeAgentImpl implements NodeAgent {
                     currentRebootGeneration, context.node().wantedRebootGeneration()));
         }
 
-        // Even though memory can be easily changed with docker update, we need to restart the container
-        // for proton to pick up the change. If/when proton could detect available memory correctly (rather than reading
-        // VESPA_TOTAL_MEMORY_MB env. variable set in DockerOperation), it would be enough with a services restart
-        // TODO: Change to Vespa restart once all tenant applications are > 7.111
-        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();
     }
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 c0b032bc4d4..d3bf93da71b 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
@@ -268,13 +268,15 @@ public class NodeAgentImplTest {
     }
 
     @Test
-    public void containerIsRestartedIfMemoryChanged() {
+    public void vespaIsRestartedIfMemoryChanged() {
         NodeSpec.Builder specBuilder = nodeBuilder
                 .wantedDockerImage(dockerImage)
                 .currentDockerImage(dockerImage)
                 .state(NodeState.active)
                 .wantedVespaVersion(vespaVersion)
-                .currentVespaVersion(vespaVersion);
+                .currentVespaVersion(vespaVersion)
+                .wantedRestartGeneration(1)
+                .currentRestartGeneration(1);
 
         NodeAgentContext firstContext = createContext(specBuilder.build());
         NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true);
@@ -290,11 +292,9 @@ public class NodeAgentImplTest {
 
         InOrder inOrder = inOrder(orchestrator, dockerOperations);
         inOrder.verify(orchestrator).resume(any(String.class));
-        inOrder.verify(orchestrator).suspend(any(String.class));
-        inOrder.verify(dockerOperations).removeContainer(eq(secondContext), any());
-        inOrder.verify(dockerOperations, times(1)).createContainer(eq(secondContext), any(), any());
-        inOrder.verify(dockerOperations).startContainer(eq(secondContext));
+        inOrder.verify(dockerOperations, never()).removeContainer(any(), any());
         inOrder.verify(dockerOperations, never()).updateContainer(any(), any());
+        inOrder.verify(dockerOperations).restartVespa(eq(secondContext));
 
         nodeAgent.doConverge(secondContext);
         inOrder.verify(orchestrator).resume(any(String.class));