Use docker update for changing container resource allocation

5 files changed, 38 insertions, 11 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
index af8dfb1fd27..27dc4e5237b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.docker;
 
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
+import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
 import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
@@ -20,6 +21,8 @@ public interface DockerOperations {
 
     void removeContainer(NodeAgentContext context, Container container);
 
+    void updateContainer(NodeAgentContext context, ContainerResources containerResources);
+
     Optional<Container> getContainer(NodeAgentContext context);
 
     boolean pullImageAsyncIfNeeded(DockerImage dockerImage);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index 5edbf98e4bb..bdda155b2f3 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -171,6 +171,11 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
+    public void updateContainer(NodeAgentContext context, ContainerResources containerResources) {
+        docker.updateContainer(context.containerName(), containerResources);
+    }
+
+    @Override
     public Optional<Container> getContainer(NodeAgentContext context) {
         return docker.getContainer(context.containerName());
     }
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 a58aaa24575..96c6fed63de 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
@@ -310,13 +310,6 @@ public class NodeAgentImpl implements NodeAgent {
             return Optional.of("Container no longer running");
         }
 
-        ContainerResources wantedContainerResources = ContainerResources.from(
-                0, node.getMinCpuCores(), node.getMinMainMemoryAvailableGb());
-        if (!wantedContainerResources.equals(existingContainer.resources)) {
-            return Optional.of("Container should be running with different resource allocation, wanted: " +
-                    wantedContainerResources + ", actual: " + existingContainer.resources);
-        }
-
         if (currentRebootGeneration < node.getWantedRebootGeneration()) {
             return Optional.of(String.format("Container reboot wanted. Current: %d, Wanted: %d",
                     currentRebootGeneration, node.getWantedRebootGeneration()));
@@ -356,6 +349,21 @@ public class NodeAgentImpl implements NodeAgent {
         return Optional.of(existingContainer);
     }
 
+    private void updateContainerIfNeeded(NodeAgentContext context, Container existingContainer) {
+        ContainerResources wantedContainerResources = ContainerResources.from(
+                0, context.node().getMinCpuCores(), context.node().getMinMainMemoryAvailableGb());
+
+        if (wantedContainerResources.equals(existingContainer.resources)) return;
+        context.log(logger, "Container should be running with different resource allocation, wanted: %s, current: %s",
+                wantedContainerResources, existingContainer.resources);
+
+        if (context.node().getState() == Node.State.active) {
+            orchestratorSuspendNode(context);
+        }
+
+        dockerOperations.updateContainer(context, wantedContainerResources);
+    }
+
 
     private void scheduleDownLoadIfNeeded(NodeSpec node) {
         if (node.getCurrentDockerImage().equals(node.getWantedDockerImage())) return;
@@ -438,6 +446,8 @@ public class NodeAgentImpl implements NodeAgent {
                     startContainer(context);
                     containerState = UNKNOWN;
                     aclMaintainer.ifPresent(AclMaintainer::converge);
+                } else {
+                    updateContainerIfNeeded(context, container.get());
                 }
 
                 startServicesIfNeeded(context);
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
index f980642a7b8..87b07ca4ed9 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
@@ -67,6 +67,15 @@ public class DockerMock implements Docker {
     }
 
     @Override
+    public void updateContainer(ContainerName containerName, ContainerResources containerResources) {
+        synchronized (monitor) {
+            Container container = containersByContainerName.get(containerName);
+            containersByContainerName.put(containerName,
+                    new Container(container.hostname, container.image, containerResources, container.name, container.state, container.pid));
+        }
+    }
+
+    @Override
     public Optional<Container> getContainer(ContainerName containerName) {
         synchronized (monitor) {
             return Optional.ofNullable(containersByContainerName.get(containerName));
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 7eee068e472..59a40fe3ccc 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
@@ -233,7 +233,7 @@ public class NodeAgentImplTest {
     }
 
     @Test
-    public void containerIsRestartedIfFlavorChanged() {
+    public void containerIsUpdatedIfFlavorChanged() {
         NodeSpec.Builder specBuilder = nodeBuilder
                 .wantedDockerImage(dockerImage)
                 .currentDockerImage(dockerImage)
@@ -257,9 +257,9 @@ public class NodeAgentImplTest {
         inOrder.verify(orchestrator).resume(any(String.class));
         inOrder.verify(orchestrator).resume(any(String.class));
         inOrder.verify(orchestrator).suspend(any(String.class));
-        inOrder.verify(dockerOperations).removeContainer(eq(thirdContext), any());
-        inOrder.verify(dockerOperations, times(1)).createContainer(eq(thirdContext), any());
-        inOrder.verify(dockerOperations).startContainer(eq(thirdContext));
+        inOrder.verify(dockerOperations).updateContainer(eq(thirdContext), any());
+        inOrder.verify(dockerOperations, never()).removeContainer(any(), any());
+        inOrder.verify(dockerOperations, never()).startContainer(any());
         inOrder.verify(orchestrator).resume(any(String.class));
     }