diff options
10 files changed, 60 insertions, 55 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java index cfa0452ebf9..2aa1d12c491 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java @@ -20,8 +20,11 @@ import java.util.Optional; */ public interface ContainerEngine { - /** Create a new container */ - void createContainer(NodeAgentContext context, ContainerData containerData, ContainerResources containerResources); + /** + * Create a new container + * @return ContainerData that can be used to write files inside container + */ + ContainerData createContainer(NodeAgentContext context, ContainerResources containerResources); /** Start a created container */ void startContainer(NodeAgentContext context); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java index 8a66373c28b..9060261b806 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java @@ -41,8 +41,8 @@ public class ContainerOperations { this.containerStatsCollector = new ContainerStatsCollector(cgroup, fileSystem); } - public void createContainer(NodeAgentContext context, ContainerData containerData, ContainerResources containerResources) { - containerEngine.createContainer(context, containerData, containerResources); + public ContainerData createContainer(NodeAgentContext context, ContainerResources containerResources) { + return containerEngine.createContainer(context, containerResources); } public void startContainer(NodeAgentContext context) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java index 26c3d101c69..7e30ae8d649 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java @@ -23,8 +23,11 @@ public interface ContainerData { */ void addFile(ContainerPath path, String data, String permissions); - /** Add directory in container at path. */ - void addDirectory(ContainerPath path); + /** + * @param path Container path to create directory at + * @param permissions optional file permissions, see {@link UnixPath#setPermissions(String)} for format. + */ + void addDirectory(ContainerPath path, String... permissions); /** * Symlink to a file in container at path. 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 9da7f1dbdb6..61e777a9576 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 @@ -27,9 +27,7 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.identity.CredentialsMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.VespaServiceDumper; import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; -import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; -import java.nio.file.Path; import java.time.Clock; import java.time.Duration; import java.time.Instant; @@ -233,10 +231,10 @@ public class NodeAgentImpl implements NodeAgent { } private Container startContainer(NodeAgentContext context) { - ContainerData containerData = createContainerData(context); ContainerResources wantedResources = warmUpDuration(context).isNegative() ? getContainerResources(context) : getContainerResources(context).withUnlimitedCpus(); - containerOperations.createContainer(context, containerData, wantedResources); + ContainerData containerData = containerOperations.createContainer(context, wantedResources); + writeContainerData(context, containerData); containerOperations.startContainer(context); currentRebootGeneration = context.node().wantedRebootGeneration(); @@ -598,29 +596,7 @@ public class NodeAgentImpl implements NodeAgent { } } - protected ContainerData createContainerData(NodeAgentContext context) { - return new ContainerData() { - @Override - public void addFile(ContainerPath path, String data) { - throw new UnsupportedOperationException("addFile not implemented"); - } - - @Override - public void addFile(ContainerPath path, String data, String permissions) { - throw new UnsupportedOperationException("addFile not implemented"); - } - - @Override - public void addDirectory(ContainerPath path) { - throw new UnsupportedOperationException("addDirectory not implemented"); - } - - @Override - public void createSymlink(ContainerPath symlink, Path target) { - throw new UnsupportedOperationException("createSymlink not implemented"); - } - }; - } + protected void writeContainerData(NodeAgentContext context, ContainerData containerData) { } protected List<CredentialsMaintainer> credentialsMaintainers() { return credentialsMaintainers; diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java index 25cdff4b726..de3ae4cff98 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java @@ -7,8 +7,10 @@ import com.yahoo.vespa.hosted.node.admin.container.image.Image; import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixUser; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; +import java.nio.file.Path; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -66,8 +68,29 @@ public class ContainerEngineMock implements ContainerEngine { } @Override - public void createContainer(NodeAgentContext context, ContainerData containerData, ContainerResources containerResources) { + public ContainerData createContainer(NodeAgentContext context, ContainerResources containerResources) { addContainer(createContainer(context, PartialContainer.State.created, containerResources)); + return new ContainerData() { + @Override + public void addFile(ContainerPath path, String data) { + throw new UnsupportedOperationException("addFile not implemented"); + } + + @Override + public void addFile(ContainerPath path, String data, String permissions) { + throw new UnsupportedOperationException("addFile not implemented"); + } + + @Override + public void addDirectory(ContainerPath path, String... permissions) { + throw new UnsupportedOperationException("addDirectory not implemented"); + } + + @Override + public void createSymlink(ContainerPath symlink, Path target) { + throw new UnsupportedOperationException("createSymlink not implemented"); + } + }; } @Override diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java index a2dbfe0db4b..5649da7c4ea 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java @@ -2,8 +2,8 @@ package com.yahoo.vespa.hosted.node.admin.integration; import com.yahoo.config.provision.DockerImage; -import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; +import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.test.file.TestFileSystem; @@ -36,13 +36,13 @@ public class ContainerFailTest { NodeAgentContext context = NodeAgentContextImpl.builder(nodeSpec).fileSystem(TestFileSystem.create()).build(); - tester.inOrder(tester.containerOperations).createContainer(containerMatcher(containerName), any(), any()); + tester.inOrder(tester.containerOperations).createContainer(containerMatcher(containerName), any()); tester.inOrder(tester.containerOperations).resumeNode(containerMatcher(containerName)); tester.containerOperations.removeContainer(context, tester.containerOperations.getContainer(context).get()); tester.inOrder(tester.containerOperations).removeContainer(containerMatcher(containerName), any()); - tester.inOrder(tester.containerOperations).createContainer(containerMatcher(containerName), any(), any()); + tester.inOrder(tester.containerOperations).createContainer(containerMatcher(containerName), any()); tester.inOrder(tester.containerOperations).resumeNode(containerMatcher(containerName)); verify(tester.nodeRepository, never()).updateNodeAttributes(any(), any()); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/MultiContainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/MultiContainerTest.java index 5bb279323e9..79546d8cf78 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/MultiContainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/MultiContainerTest.java @@ -48,7 +48,7 @@ public class MultiContainerTest { tester.addChildNodeRepositoryNode(nodeSpec); ContainerName containerName = ContainerName.fromHostname(hostName); - tester.inOrder(tester.containerOperations).createContainer(containerMatcher(containerName), any(), any()); + tester.inOrder(tester.containerOperations).createContainer(containerMatcher(containerName), any()); tester.inOrder(tester.containerOperations).resumeNode(containerMatcher(containerName)); tester.inOrder(tester.nodeRepository).updateNodeAttributes(eq(hostName), any()); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RebootTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RebootTest.java index 4d95991dd29..f3635be1b4b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RebootTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RebootTest.java @@ -2,8 +2,8 @@ package com.yahoo.vespa.hosted.node.admin.integration; import com.yahoo.config.provision.DockerImage; -import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; +import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdater; import org.junit.Test; @@ -31,7 +31,7 @@ public class RebootTest { tester.addChildNodeRepositoryNode(NodeSpec.Builder.testSpec(hostname).wantedDockerImage(dockerImage).build()); ContainerName host1 = new ContainerName("host1"); - tester.inOrder(tester.containerOperations).createContainer(containerMatcher(host1), any(), any()); + tester.inOrder(tester.containerOperations).createContainer(containerMatcher(host1), any()); tester.setWantedState(NodeAdminStateUpdater.State.SUSPENDED); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RestartTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RestartTest.java index 13b3db1c55c..0675626c0e2 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RestartTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/RestartTest.java @@ -2,9 +2,9 @@ package com.yahoo.vespa.hosted.node.admin.integration; import com.yahoo.config.provision.DockerImage; -import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; +import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import org.junit.Test; import java.util.List; @@ -29,7 +29,7 @@ public class RestartTest { tester.addChildNodeRepositoryNode(nodeSpec); ContainerName host1 = new ContainerName("host1"); - tester.inOrder(tester.containerOperations).createContainer(containerMatcher(host1), any(), any()); + tester.inOrder(tester.containerOperations).createContainer(containerMatcher(host1), any()); tester.inOrder(tester.nodeRepository).updateNodeAttributes( eq(hostname), eq(new NodeAttributes().withDockerImage(dockerImage).withVespaVersion(dockerImage.tagAsVersion()))); 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 28ef19d4065..2eff2c64cec 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 @@ -154,7 +154,7 @@ public class NodeAgentImplTest { nodeAgent.stopForHostSuspension(context); nodeAgent.doConverge(context); - inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any(), any()); + inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any()); inOrder.verify(containerOperations, times(1)).startContainer(eq(context)); inOrder.verify(containerOperations, times(0)).startServices(eq(context)); // done as part of startContainer inOrder.verify(containerOperations, times(1)).resumeNode(eq(context)); @@ -181,7 +181,7 @@ public class NodeAgentImplTest { final InOrder inOrder = inOrder(containerOperations, orchestrator, nodeRepository, aclMaintainer, healthChecker); inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(any(), eq(dockerImage), any()); - inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any(), any()); + inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any()); inOrder.verify(containerOperations, times(1)).startContainer(eq(context)); inOrder.verify(aclMaintainer, times(1)).converge(eq(context)); inOrder.verify(containerOperations, times(1)).resumeNode(eq(context)); @@ -310,7 +310,7 @@ public class NodeAgentImplTest { fail("Expected to throw an exception"); } catch (OrchestratorException ignored) { } - verify(containerOperations, never()).createContainer(eq(context), any(), any()); + verify(containerOperations, never()).createContainer(eq(context), any()); verify(containerOperations, never()).startContainer(eq(context)); verify(orchestrator, never()).resume(any(String.class)); verify(nodeRepository, never()).updateNodeAttributes(any(String.class), any(NodeAttributes.class)); @@ -343,7 +343,7 @@ public class NodeAgentImplTest { // First time we fail to resume because health verification fails verify(orchestrator, times(1)).suspend(eq(hostName)); verify(containerOperations, times(1)).removeContainer(eq(context), any()); - verify(containerOperations, times(1)).createContainer(eq(context), any(), any()); + verify(containerOperations, times(1)).createContainer(eq(context), any()); verify(containerOperations, times(1)).startContainer(eq(context)); verify(orchestrator, never()).resume(eq(hostName)); verify(nodeRepository, never()).updateNodeAttributes(any(), any()); @@ -352,7 +352,7 @@ public class NodeAgentImplTest { // Do not reboot the container again verify(containerOperations, times(1)).removeContainer(eq(context), any()); - verify(containerOperations, times(1)).createContainer(eq(context), any(), any()); + verify(containerOperations, times(1)).createContainer(eq(context), any()); verify(orchestrator, times(1)).resume(eq(hostName)); verify(nodeRepository, times(1)).updateNodeAttributes(eq(hostName), eq(new NodeAttributes() .withRebootGeneration(wantedRebootGeneration))); @@ -393,7 +393,7 @@ public class NodeAgentImplTest { // Should only be called once, when we initialize verify(containerOperations, times(1)).getContainer(eq(context)); verify(containerOperations, never()).removeContainer(eq(context), any()); - verify(containerOperations, never()).createContainer(eq(context), any(), any()); + verify(containerOperations, never()).createContainer(eq(context), any()); verify(containerOperations, never()).startContainer(eq(context)); verify(orchestrator, never()).resume(any(String.class)); verify(nodeRepository, never()).updateNodeAttributes(eq(hostName), any()); @@ -458,7 +458,7 @@ public class NodeAgentImplTest { inOrder.verify(storageMaintainer, times(1)).archiveNodeStorage(eq(context)); inOrder.verify(nodeRepository, times(1)).setNodeState(eq(hostName), eq(NodeState.ready)); - verify(containerOperations, never()).createContainer(eq(context), any(), any()); + verify(containerOperations, never()).createContainer(eq(context), any()); verify(containerOperations, never()).startContainer(eq(context)); verify(containerOperations, never()).suspendNode(eq(context)); verify(containerOperations, times(1)).stopServices(eq(context)); @@ -508,7 +508,7 @@ public class NodeAgentImplTest { nodeAgent.doConverge(context); verify(containerOperations, times(1)).removeContainer(eq(context), any()); - verify(containerOperations, times(1)).createContainer(eq(context), any(), any()); + verify(containerOperations, times(1)).createContainer(eq(context), any()); verify(containerOperations, times(1)).startContainer(eq(context)); } @@ -568,7 +568,7 @@ public class NodeAgentImplTest { } catch (RuntimeException ignored) { } verify(containerOperations, never()).removeContainer(eq(context), any()); - verify(containerOperations, times(1)).createContainer(eq(context), any(), any()); + verify(containerOperations, times(1)).createContainer(eq(context), any()); verify(containerOperations, times(1)).startContainer(eq(context)); verify(nodeAgent, never()).resumeNodeIfNeeded(any()); @@ -578,7 +578,7 @@ public class NodeAgentImplTest { nodeAgent.doConverge(context); verify(containerOperations, times(1)).removeContainer(eq(context), any()); - verify(containerOperations, times(2)).createContainer(eq(context), any(), any()); + verify(containerOperations, times(2)).createContainer(eq(context), any()); verify(containerOperations, times(2)).startContainer(eq(context)); verify(nodeAgent, times(1)).resumeNodeIfNeeded(any()); } @@ -605,7 +605,7 @@ public class NodeAgentImplTest { final InOrder inOrder = inOrder(containerOperations, orchestrator, nodeRepository, aclMaintainer); inOrder.verify(containerOperations, times(1)).pullImageAsyncIfNeeded(any(), eq(dockerImage), any()); - inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any(), any()); + inOrder.verify(containerOperations, times(1)).createContainer(eq(context), any()); inOrder.verify(containerOperations, times(1)).startContainer(eq(context)); inOrder.verify(aclMaintainer, times(1)).converge(eq(context)); inOrder.verify(containerOperations, times(1)).resumeNode(eq(context)); @@ -770,10 +770,10 @@ public class NodeAgentImplTest { mockGetContainer(dockerImage, isRunning); doAnswer(invoc -> { NodeAgentContext context = invoc.getArgument(0, NodeAgentContext.class); - ContainerResources resources = invoc.getArgument(2, ContainerResources.class); + ContainerResources resources = invoc.getArgument(1, ContainerResources.class); mockGetContainer(context.node().wantedDockerImage().get(), resources, true); return null; - }).when(containerOperations).createContainer(any(), any(), any()); + }).when(containerOperations).createContainer(any(), any()); doAnswer(invoc -> { NodeAgentContext context = invoc.getArgument(0, NodeAgentContext.class); |