Merge pull request #10997 from vespa-engine/hakonhall/support-filesystem-in-nodeagentcontextimpl

Support FileSystem in NodeAgentContextImpl

5 files changed, 36 insertions, 19 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java
index f1fd97f6e4c..28764c6a7f2 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java
@@ -11,7 +11,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 
 import java.nio.file.Path;
-import java.nio.file.Paths;
 
 public interface NodeAgentContext extends TaskContext {
 
@@ -52,10 +51,7 @@ public interface NodeAgentContext extends TaskContext {
     Path pathOnHostFromPathInNode(Path pathInNode);
 
     /** @see #pathOnHostFromPathInNode(Path) */
-    default Path pathOnHostFromPathInNode(String pathInNode) {
-        return pathOnHostFromPathInNode(Paths.get(pathInNode));
-    }
-
+    Path pathOnHostFromPathInNode(String pathInNode);
 
     /**
      * This method is the inverse of {@link #pathOnHostFromPathInNode(Path)}
@@ -66,9 +62,7 @@ public interface NodeAgentContext extends TaskContext {
     Path pathInNodeFromPathOnHost(Path pathOnHost);
 
     /** @see #pathOnHostFromPathInNode(Path) */
-    default Path pathInNodeFromPathOnHost(String pathOnHost) {
-        return pathInNodeFromPathOnHost(Paths.get(pathOnHost));
-    }
+    Path pathInNodeFromPathOnHost(String pathOnHost);
 
 
     /**
@@ -78,7 +72,5 @@ public interface NodeAgentContext extends TaskContext {
     Path pathInNodeUnderVespaHome(Path relativePath);
 
     /** @see #pathInNodeUnderVespaHome(Path) */
-    default Path pathInNodeUnderVespaHome(String relativePath) {
-        return pathInNodeUnderVespaHome(Paths.get(relativePath));
-    }
+    Path pathInNodeUnderVespaHome(String relativePath);
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
index b2769ee40c4..304664e541b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
@@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 
 import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Objects;
@@ -106,6 +107,12 @@ public class NodeAgentContextImpl implements NodeAgentContext {
     }
 
     @Override
+    public Path pathOnHostFromPathInNode(String pathInNode) {
+        // Ensure the path is on the proper FileSystem
+        return pathOnHostFromPathInNode(ROOT.getFileSystem().getPath(pathInNode));
+    }
+
+    @Override
     public Path pathInNodeFromPathOnHost(Path pathOnHost) {
         if (! pathOnHost.isAbsolute())
             throw new IllegalArgumentException("Expected an absolute path on the host, got: " + pathOnHost);
@@ -117,6 +124,12 @@ public class NodeAgentContextImpl implements NodeAgentContext {
     }
 
     @Override
+    public Path pathInNodeFromPathOnHost(String pathOnHost) {
+        // Ensure the path is on the proper FileSystem
+        return pathInNodeFromPathOnHost(pathToNodeRootOnHost.getFileSystem().getPath(pathOnHost));
+    }
+
+    @Override
     public Path pathInNodeUnderVespaHome(Path relativePath) {
         if (relativePath.isAbsolute())
             throw new IllegalArgumentException("Expected a relative path to the Vespa home, got: " + relativePath);
@@ -125,6 +138,12 @@ public class NodeAgentContextImpl implements NodeAgentContext {
     }
 
     @Override
+    public Path pathInNodeUnderVespaHome(String relativePath) {
+        // Ensure the path is on the proper FileSystem
+        return pathInNodeUnderVespaHome(pathToVespaHome.getFileSystem().getPath(relativePath));
+    }
+
+    @Override
     public void recordSystemModification(Logger logger, String message) {
         log(logger, message);
     }
@@ -166,6 +185,7 @@ public class NodeAgentContextImpl implements NodeAgentContext {
         private Path pathToVespaHome;
         private String vespaUser;
         private String vespaUserOnHost;
+        private FileSystem fileSystem = FileSystems.getDefault();
 
         public Builder(NodeSpec node) {
             this.nodeSpecBuilder = new NodeSpec.Builder(node);
@@ -209,8 +229,8 @@ public class NodeAgentContextImpl implements NodeAgentContext {
             return this;
         }
 
-        public Builder pathToContainerStorage(Path pathToContainerStorage) {
-            this.pathToContainerStorage = pathToContainerStorage;
+        public Builder pathToContainerStorageFromFileSystem(FileSystem fileSystem) {
+            this.pathToContainerStorage = fileSystem.getPath("/home/docker");
             return this;
         }
 
@@ -229,8 +249,13 @@ public class NodeAgentContextImpl implements NodeAgentContext {
             return this;
         }
 
+        /**
+         * Sets the default file system to use for paths. May be overridden for each path,
+         * e.g. {@link #pathToVespaHome(Path)}  pathToVespaHome()}.
+         */
         public Builder fileSystem(FileSystem fileSystem) {
-            return pathToContainerStorage(fileSystem.getPath("/home/docker"));
+            this.fileSystem = fileSystem;
+            return this;
         }
 
         public NodeAgentContextImpl build() {
@@ -260,8 +285,8 @@ public class NodeAgentContextImpl implements NodeAgentContext {
                             return getId().region().value();
                         }
                     }),
-                    Optional.ofNullable(pathToContainerStorage).orElseGet(() -> Paths.get("/home/docker")),
-                    Optional.ofNullable(pathToVespaHome).orElseGet(() -> Paths.get("/opt/vespa")),
+                    Optional.ofNullable(pathToContainerStorage).orElseGet(() -> fileSystem.getPath("/home/docker")),
+                    Optional.ofNullable(pathToVespaHome).orElseGet(() -> fileSystem.getPath("/opt/vespa")),
                     Optional.ofNullable(vespaUser).orElse("vespa"),
                     Optional.ofNullable(vespaUserOnHost).orElse("container_vespa"));
         }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index 22b3949755f..70b406b3056 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -97,7 +97,7 @@ public class DockerTester implements AutoCloseable {
                 Optional.empty(), Optional.empty(), Optional.empty());
         nodeAdmin = new NodeAdminImpl(nodeAgentFactory, metrics, Clock.systemUTC(), Duration.ofMillis(10), Duration.ZERO);
         NodeAgentContextFactory nodeAgentContextFactory = (nodeSpec, acl) ->
-                new NodeAgentContextImpl.Builder(nodeSpec).acl(acl).fileSystem(fileSystem).build();
+                new NodeAgentContextImpl.Builder(nodeSpec).acl(acl).pathToContainerStorageFromFileSystem(fileSystem).build();
         nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeAgentContextFactory, nodeRepository, orchestrator,
                 nodeAdmin, HOST_HOSTNAME, Clock.systemUTC());
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
index d034d3c1cd0..2a670ba8322 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
@@ -110,7 +110,7 @@ public class StorageMaintainerTest {
 
         private NodeAgentContext createNodeAgentContextAndContainerStorage(FileSystem fileSystem, String containerName) throws IOException {
             NodeAgentContext context = new NodeAgentContextImpl.Builder(containerName + ".domain.tld")
-                    .fileSystem(fileSystem).build();
+                    .pathToContainerStorageFromFileSystem(fileSystem).build();
 
             Path containerVespaHomeOnHost = context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome(""));
             Files.createDirectories(context.pathOnHostFromPathInNode("/etc/something"));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java
index 84f13ed299a..be94846fb65 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java
@@ -13,7 +13,7 @@ import static org.junit.Assert.assertEquals;
 public class NodeAgentContextImplTest {
     private final FileSystem fileSystem = TestFileSystem.create();
     private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-1.domain.tld")
-            .fileSystem(fileSystem).build();
+            .pathToContainerStorageFromFileSystem(fileSystem).build();
 
     @Test
     public void path_on_host_from_path_in_node_test() {