Create factory method for ContainerFileSystem

5 files changed, 33 insertions, 38 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java
index 393137f795e..495b72e4554 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java
@@ -17,7 +17,7 @@ public class ContainerFileSystem extends FileSystem {
 
     private final ContainerFileSystemProvider containerFsProvider;
 
-    public ContainerFileSystem(ContainerFileSystemProvider containerFsProvider) {
+    ContainerFileSystem(ContainerFileSystemProvider containerFsProvider) {
         this.containerFsProvider = containerFsProvider;
     }
 
@@ -81,4 +81,7 @@ public class ContainerFileSystem extends FileSystem {
         throw new UnsupportedOperationException();
     }
 
+    public static ContainerFileSystem create(Path containerStorageRoot, int uidOffset, int gidOffset) {
+        return new ContainerFileSystemProvider(containerStorageRoot, uidOffset, gidOffset).getFileSystem(null);
+    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java
index 79214334c55..73cd3f8cfc5 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java
@@ -37,14 +37,12 @@ import static com.yahoo.yolean.Exceptions.uncheck;
 /**
  * @author valerijf
  */
-public class ContainerFileSystemProvider extends FileSystemProvider {
-
+class ContainerFileSystemProvider extends FileSystemProvider {
     private final ContainerFileSystem containerFs;
     private final ContainerUserPrincipalLookupService userPrincipalLookupService;
     private final Path containerRootOnHost;
 
-
-    public ContainerFileSystemProvider(Path containerRootOnHost, int uidOffset, int gidOffset) {
+    ContainerFileSystemProvider(Path containerRootOnHost, int uidOffset, int gidOffset) {
         this.containerFs = new ContainerFileSystem(this);
         this.userPrincipalLookupService = new ContainerUserPrincipalLookupService(
                 containerRootOnHost.getFileSystem().getUserPrincipalLookupService(), uidOffset, gidOffset);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPath.java
index e967806dc55..15295ffd087 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPath.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPath.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.task.util.fs;
 
 import java.io.IOException;
 import java.net.URI;
-import java.nio.file.FileSystem;
 import java.nio.file.LinkOption;
 import java.nio.file.Path;
 import java.nio.file.WatchEvent;
@@ -17,6 +16,8 @@ import java.util.Objects;
 import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerFileSystemProvider.toContainerPath;
 
 /**
+ * Represents a path in container that is mapped in from the host. ContainerPaths are always normalized and absolute.
+ *
  * @author valerijf
  */
 public class ContainerPath implements Path {
@@ -36,12 +37,11 @@ public class ContainerPath implements Path {
             throw new IllegalArgumentException("Path on host (" + pathOnHost + ") must start with container root on host (" + containerRootOnHost + ")");
     }
 
-    public Path pathOnHost() {
-        return pathOnHost;
-    }
+    public Path pathOnHost() { return pathOnHost; }
+    public String pathInContainer() { return '/' + String.join("/", parts); }
 
     @Override
-    public FileSystem getFileSystem() {
+    public ContainerFileSystem getFileSystem() {
         return containerFs;
     }
 
@@ -83,15 +83,9 @@ public class ContainerPath implements Path {
         return Path.of(parts[beginIndex], rest);
     }
 
-    @Override
-    public ContainerPath resolve(Path other) {
-        return resolve(containerFs, parts, other);
-    }
-
-    @Override
-    public ContainerPath resolveSibling(String other) {
-        return resolve(Path.of("..", other));
-    }
+    @Override public ContainerPath resolve(Path other) { return resolve(containerFs, parts, other); }
+    @Override public ContainerPath resolve(String other) { return resolve(Path.of(other)); }
+    @Override public ContainerPath resolveSibling(String other) { return resolve(Path.of("..", other)); }
 
     @Override
     public boolean startsWith(Path other) {
@@ -179,7 +173,7 @@ public class ContainerPath implements Path {
 
     @Override
     public String toString() {
-        return '/' + String.join("/", parts);
+        return containerFs.provider().containerRootOnHost().getFileName() + ":" + pathInContainer();
     }
 
     private static ContainerPath resolve(ContainerFileSystem containerFs, String[] currentParts, Path other) {
@@ -199,25 +193,25 @@ public class ContainerPath implements Path {
                 parts.toArray(String[]::new));
     }
 
-    static ContainerPath fromPathInContainer(ContainerFileSystem containerFs, Path pathInContainer) {
+    public static ContainerPath fromPathInContainer(ContainerFileSystem containerFs, Path pathInContainer) {
         if (!pathInContainer.isAbsolute())
             throw new IllegalArgumentException("Path in container must be absolute: " + pathInContainer);
         return resolve(containerFs, new String[0], pathInContainer);
     }
 
-static ContainerPath fromPathOnHost(ContainerFileSystem containerFs, Path pathOnHost) {
-    pathOnHost = pathOnHost.normalize();
-    Path containerRootOnHost = containerFs.provider().containerRootOnHost();
-    Path pathUnderContainerStorage = containerRootOnHost.relativize(pathOnHost);
+    public static ContainerPath fromPathOnHost(ContainerFileSystem containerFs, Path pathOnHost) {
+        pathOnHost = pathOnHost.normalize();
+        Path containerRootOnHost = containerFs.provider().containerRootOnHost();
+        Path pathUnderContainerStorage = containerRootOnHost.relativize(pathOnHost);
 
-    if (pathUnderContainerStorage.getNameCount() == 0 || pathUnderContainerStorage.getName(0).toString().isEmpty())
-        return new ContainerPath(containerFs, pathOnHost, new String[0]);
-    if (pathUnderContainerStorage.getName(0).toString().equals(".."))
-        throw new IllegalArgumentException("Path " + pathOnHost + " is not under container root " + containerRootOnHost);
+        if (pathUnderContainerStorage.getNameCount() == 0 || pathUnderContainerStorage.getName(0).toString().isEmpty())
+            return new ContainerPath(containerFs, pathOnHost, new String[0]);
+        if (pathUnderContainerStorage.getName(0).toString().equals(".."))
+            throw new IllegalArgumentException("Path " + pathOnHost + " is not under container root " + containerRootOnHost);
 
-    List<String> parts = new ArrayList<>();
-    for (int i = 0; i < pathUnderContainerStorage.getNameCount(); i++)
-        parts.add(pathUnderContainerStorage.getName(i).toString());
-    return new ContainerPath(containerFs, pathOnHost, parts.toArray(String[]::new));
-}
+        List<String> parts = new ArrayList<>();
+        for (int i = 0; i < pathUnderContainerStorage.getNameCount(); i++)
+            parts.add(pathUnderContainerStorage.getName(i).toString());
+        return new ContainerPath(containerFs, pathOnHost, parts.toArray(String[]::new));
+    }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java
index 38c1e2720c3..eb15450b756 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java
@@ -21,7 +21,7 @@ class ContainerFileSystemTest {
 
     private final FileSystem fileSystem = TestFileSystem.create();
     private final UnixPath containerRootOnHost = new UnixPath(fileSystem.getPath("/data/storage/ctr1"));
-    private final ContainerFileSystem containerFs = new ContainerFileSystemProvider(containerRootOnHost.createDirectories().toPath(), 10_000, 11_000).getFileSystem(null);
+    private final ContainerFileSystem containerFs = ContainerFileSystem.create(containerRootOnHost.createDirectories().toPath(), 10_000, 11_000);
 
     @Test
     public void creates_files_and_directories_with_container_root_as_owner() throws IOException {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java
index ead4ad7ecde..ebbbaf3b525 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java
@@ -25,7 +25,7 @@ import java.nio.file.Path;
 class ContainerPathTest {
 
     private final FileSystem baseFs = TestFileSystem.create();
-    private final ContainerFileSystem containerFs = new ContainerFileSystemProvider(baseFs.getPath("/data/storage/ctr1"), 0, 0).getFileSystem(null);
+    private final ContainerFileSystem containerFs = ContainerFileSystem.create(baseFs.getPath("/data/storage/ctr1"), 0, 0);
 
     @Test
     public void create_new_container_path() {
@@ -62,7 +62,7 @@ class ContainerPathTest {
         assertFalse(parent.startsWith(path));
         assertFalse(path.startsWith(Path.of(path.toString())));
 
-        assertTrue(path.endsWith(Path.of(path.toString())));
+        assertTrue(path.endsWith(Path.of(path.pathInContainer())));
         assertTrue(path.endsWith(Path.of("logs/file")));
         assertFalse(path.endsWith(Path.of("/logs/file")));
     }
@@ -102,7 +102,7 @@ class ContainerPathTest {
 
     private static void assertPaths(ContainerPath actual, String expectedPathOnHost, String expectedPathInContainer) {
         assertEquals(expectedPathOnHost, actual.pathOnHost().toString());
-        assertEquals(expectedPathInContainer, actual.toString());
+        assertEquals(expectedPathInContainer, actual.pathInContainer());
     }
 
     private static void assertThrows(Executable executable, String expectedMsg) {