Merge branch 'master' into freva/set-security-opt

# Conflicts:
#	node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java

5 files changed, 20 insertions, 34 deletions

diff --git a/node-admin/pom.xml b/node-admin/pom.xml
index a61a55fcfd9..1188cd9fb76 100644
--- a/node-admin/pom.xml
+++ b/node-admin/pom.xml
@@ -70,13 +70,11 @@
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpcore</artifactId>
-            <version>4.4.1</version>
             <scope>compile</scope>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpclient</artifactId>
-            <version>4.5</version>
             <scope>compile</scope>
         </dependency>
         <dependency>
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 cedddc9b0b6..5c0e8f81704 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
@@ -6,15 +6,14 @@ 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;
-import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData;
+import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 
-import java.util.List;
 import java.util.Optional;
 
 public interface DockerOperations {
 
-    void createContainer(NodeAgentContext context, ContainerData containerData);
+    void createContainer(NodeAgentContext context, ContainerData containerData, ContainerResources containerResources);
 
     void startContainer(NodeAgentContext context);
 
@@ -49,9 +48,4 @@ public interface DockerOperations {
     void stopServices(NodeAgentContext context);
 
     Optional<ContainerStats> getContainerStats(NodeAgentContext context);
-
-    /**
-     * Returns the list of containers managed by node-admin
-     */
-    List<Container> getAllManagedContainers();
 }
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 6ea7daa2001..fd7984ecd06 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
@@ -54,7 +54,7 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
-    public void createContainer(NodeAgentContext context, ContainerData containerData) {
+    public void createContainer(NodeAgentContext context, ContainerData containerData, ContainerResources containerResources) {
         context.log(logger, "Creating container");
 
         // IPv6 - Assume always valid
@@ -65,8 +65,7 @@ public class DockerOperationsImpl implements DockerOperations {
         Docker.CreateContainerCommand command = docker.createContainerCommand(
                 context.node().getWantedDockerImage().get(), context.containerName())
                 .withHostName(context.node().getHostname())
-                .withResources(ContainerResources.from(
-                        0, context.node().getMinCpuCores(), context.node().getMinMainMemoryAvailableGb()))
+                .withResources(containerResources)
                 .withManagedBy(MANAGER_NAME)
                 .withUlimit("nofile", 262_144, 262_144)
                 // The nproc aka RLIMIT_NPROC resource limit works as follows:
@@ -267,11 +266,6 @@ public class DockerOperationsImpl implements DockerOperations {
         return docker.getContainerStats(context.containerName());
     }
 
-    @Override
-    public List<Container> getAllManagedContainers() {
-        return docker.getAllContainersManagedBy(MANAGER_NAME);
-    }
-
     private static void addMounts(NodeAgentContext context, Docker.CreateContainerCommand command) {
         final Path varLibSia = Paths.get("/var/lib/sia");
 
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 781b036e484..41755681ba3 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
@@ -202,7 +202,7 @@ public class NodeAgentImpl implements NodeAgent {
 
     private void startContainer(NodeAgentContext context) {
         ContainerData containerData = createContainerData(context);
-        dockerOperations.createContainer(context, containerData);
+        dockerOperations.createContainer(context, containerData, getContainerResources(context.node()));
         dockerOperations.startContainer(context);
         lastCpuMetric = new CpuUsageReporter();
 
@@ -363,8 +363,8 @@ public class NodeAgentImpl implements NodeAgent {
     }
 
 
-    private void scheduleDownLoadIfNeeded(NodeSpec node) {
-        if (node.getCurrentDockerImage().equals(node.getWantedDockerImage())) return;
+    private void scheduleDownLoadIfNeeded(NodeSpec node, Optional<Container> container) {
+        if (node.getWantedDockerImage().equals(container.map(c -> c.image))) return;
 
         if (dockerOperations.pullImageAsyncIfNeeded(node.getWantedDockerImage().get())) {
             imageBeingDownloaded = node.getWantedDockerImage().get();
@@ -432,7 +432,7 @@ public class NodeAgentImpl implements NodeAgent {
                         .filter(diskUtil -> diskUtil >= 0.8)
                         .ifPresent(diskUtil -> storageMaintainer.removeOldFilesFromNode(context));
 
-                scheduleDownLoadIfNeeded(node);
+                scheduleDownLoadIfNeeded(node, container);
                 if (isDownloadingImage()) {
                     context.log(logger, "Waiting for image to download " + imageBeingDownloaded.asString());
                     return;
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 577f8e9573d..bb11961d346 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
@@ -168,7 +168,7 @@ public class NodeAgentImplTest {
 
         nodeAgent.stopForHostSuspension();
         nodeAgent.doConverge(context);
-        inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any());
+        inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any(), any());
         inOrder.verify(dockerOperations, times(1)).startContainer(eq(context));
         inOrder.verify(dockerOperations, times(0)).startServices(eq(context)); // done as part of startContainer
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context));
@@ -200,7 +200,7 @@ public class NodeAgentImplTest {
 
         final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository, aclMaintainer, healthChecker);
         inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
-        inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any());
+        inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any(), any());
         inOrder.verify(dockerOperations, times(1)).startContainer(eq(context));
         inOrder.verify(aclMaintainer, times(1)).converge(eq(context));
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context));
@@ -310,7 +310,7 @@ public class NodeAgentImplTest {
         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());
+        inOrder.verify(dockerOperations, times(1)).createContainer(eq(secondContext), any(), any());
         inOrder.verify(dockerOperations).startContainer(eq(secondContext));
         inOrder.verify(dockerOperations, never()).updateContainer(any(), any());
 
@@ -343,7 +343,7 @@ public class NodeAgentImplTest {
             fail("Expected to throw an exception");
         } catch (OrchestratorException ignored) { }
 
-        verify(dockerOperations, never()).createContainer(eq(context), any());
+        verify(dockerOperations, never()).createContainer(eq(context), any(), any());
         verify(dockerOperations, never()).startContainer(eq(context));
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository, never()).updateNodeAttributes(any(String.class), any(NodeAttributes.class));
@@ -382,7 +382,7 @@ public class NodeAgentImplTest {
         // First time we fail to resume because health verification fails
         verify(orchestrator, times(1)).suspend(eq(hostName));
         verify(dockerOperations, times(1)).removeContainer(eq(context), any());
-        verify(dockerOperations, times(1)).createContainer(eq(context), any());
+        verify(dockerOperations, times(1)).createContainer(eq(context), any(), any());
         verify(dockerOperations, times(1)).startContainer(eq(context));
         verify(orchestrator, never()).resume(eq(hostName));
         verify(nodeRepository, never()).updateNodeAttributes(any(), any());
@@ -391,7 +391,7 @@ public class NodeAgentImplTest {
 
         // Do not reboot the container again
         verify(dockerOperations, times(1)).removeContainer(eq(context), any());
-        verify(dockerOperations, times(1)).createContainer(eq(context), any());
+        verify(dockerOperations, times(1)).createContainer(eq(context), any(), any());
         verify(orchestrator, times(1)).resume(eq(hostName));
         verify(nodeRepository, times(1)).updateNodeAttributes(eq(hostName), eq(new NodeAttributes()
                 .withRebootGeneration(wantedRebootGeneration)));
@@ -437,7 +437,7 @@ public class NodeAgentImplTest {
         // Should only be called once, when we initialize
         verify(dockerOperations, times(1)).getContainer(eq(context));
         verify(dockerOperations, never()).removeContainer(eq(context), any());
-        verify(dockerOperations, never()).createContainer(eq(context), any());
+        verify(dockerOperations, never()).createContainer(eq(context), any(), any());
         verify(dockerOperations, never()).startContainer(eq(context));
         verify(orchestrator, never()).resume(any(String.class));
         verify(nodeRepository, never()).updateNodeAttributes(eq(hostName), any());
@@ -510,7 +510,7 @@ public class NodeAgentImplTest {
         inOrder.verify(storageMaintainer, times(1)).archiveNodeStorage(eq(context));
         inOrder.verify(nodeRepository, times(1)).setNodeState(eq(hostName), eq(NodeState.ready));
 
-        verify(dockerOperations, never()).createContainer(eq(context), any());
+        verify(dockerOperations, never()).createContainer(eq(context), any(), any());
         verify(dockerOperations, never()).startContainer(eq(context));
         verify(dockerOperations, never()).suspendNode(eq(context));
         verify(dockerOperations, times(1)).stopServices(eq(context));
@@ -564,7 +564,7 @@ public class NodeAgentImplTest {
         nodeAgent.doConverge(context);
 
         verify(dockerOperations, times(1)).removeContainer(eq(context), any());
-        verify(dockerOperations, times(1)).createContainer(eq(context), any());
+        verify(dockerOperations, times(1)).createContainer(eq(context), any(), any());
         verify(dockerOperations, times(1)).startContainer(eq(context));
     }
 
@@ -626,7 +626,7 @@ public class NodeAgentImplTest {
         } catch (DockerException ignored) { }
 
         verify(dockerOperations, never()).removeContainer(eq(context), any());
-        verify(dockerOperations, times(1)).createContainer(eq(context), any());
+        verify(dockerOperations, times(1)).createContainer(eq(context), any(), any());
         verify(dockerOperations, times(1)).startContainer(eq(context));
         verify(nodeAgent, never()).resumeNodeIfNeeded(any());
 
@@ -636,7 +636,7 @@ public class NodeAgentImplTest {
         nodeAgent.doConverge(context);
 
         verify(dockerOperations, times(1)).removeContainer(eq(context), any());
-        verify(dockerOperations, times(2)).createContainer(eq(context), any());
+        verify(dockerOperations, times(2)).createContainer(eq(context), any(), any());
         verify(dockerOperations, times(2)).startContainer(eq(context));
         verify(nodeAgent, times(1)).resumeNodeIfNeeded(any());
     }
@@ -746,7 +746,7 @@ public class NodeAgentImplTest {
 
         final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository, aclMaintainer);
         inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(dockerImage));
-        inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any());
+        inOrder.verify(dockerOperations, times(1)).createContainer(eq(context), any(), any());
         inOrder.verify(dockerOperations, times(1)).startContainer(eq(context));
         inOrder.verify(aclMaintainer, times(1)).converge(eq(context));
         inOrder.verify(dockerOperations, times(1)).resumeNode(eq(context));