Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDFS-16791. Add getEnclosingRoot() API to filesystem interface and implementations #6198

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1643,7 +1643,7 @@ public MultipartUploaderBuilder createMultipartUploader(Path basePath)
* The enclosing root path is a common ancestor that should be used for temp and staging dirs
* as well as within encryption zones and other restricted directories.
*
* Call makeQualified on the param path to ensure the param path to ensure its part of the correct filesystem
* Call makeQualified on the param path to ensure its part of the correct filesystem
*
* @param path file path to find the enclosing root path for
* @return a path to the enclosing root
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4949,7 +4949,7 @@ public CompletableFuture<FSDataInputStream> build() throws IOException {
* The enclosing root path is a common ancestor that should be used for temp and staging dirs
* as well as within encryption zones and other restricted directories.
*
mccormickt12 marked this conversation as resolved.
Show resolved Hide resolved
* Call makeQualified on the param path to ensure the param path to ensure its part of the correct filesystem.
* Call makeQualified on the param path to ensure its part of the correct filesystem.
*
* @param path file path to find the enclosing root path for
* @return a path to the enclosing root
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1940,7 +1940,8 @@ public Path getEnclosingRoot(Path path) throws IOException {
try {
res = fsState.resolve((path.toString()), true);
} catch (FileNotFoundException ex) {
throw new NotInMountpointException(path, String.format("getEnclosingRoot - %s", ex.getMessage()));
throw new NotInMountpointException(path,
String.format("getEnclosingRoot - %s", ex.getMessage()));
mccormickt12 marked this conversation as resolved.
Show resolved Hide resolved
}
Path fullPath = new Path(res.resolvedPath);
Path enclosingPath = res.targetFileSystem.getEnclosingRoot(path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1484,7 +1484,8 @@ public Path getEnclosingRoot(Path path) throws IOException {
try {
res = fsState.resolve((path.toString()), true);
} catch (FileNotFoundException ex) {
throw new NotInMountpointException(path, String.format("getEnclosingRoot - %s", ex.getMessage()));
throw new NotInMountpointException(path,
String.format("getEnclosingRoot - %s", ex.getMessage()));
}
Path fullPath = new Path(res.resolvedPath);
Path enclosingPath = res.targetFileSystem.getEnclosingRoot(path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,18 @@ public void testEnclosingRootEquivalence() throws IOException {
root, fs.getEnclosingRoot(foobar));
assertEquals("Ensure getEnclosingRoot called on itself returns the root directory",
root, fs.getEnclosingRoot(fs.getEnclosingRoot(foobar)));
assertEquals("Ensure getEnclosingRoot for different paths in the same enclosing root returns the same path",
assertEquals(
"Ensure getEnclosingRoot for different paths in the same enclosing root returns the same path",
fs.getEnclosingRoot(root), fs.getEnclosingRoot(foobar));
assertEquals("Ensure getEnclosingRoot on a path returns the root directory",
root, fs.getEnclosingRoot(methodPath()));
assertEquals("Ensure getEnclosingRoot called on itself on a path returns the root directory",
root, fs.getEnclosingRoot(fs.getEnclosingRoot(methodPath())));
assertEquals("Ensure getEnclosingRoot for different paths in the same enclosing root returns the same path", fs.getEnclosingRoot(root), fs.getEnclosingRoot(methodPath()));
assertEquals(
"Ensure getEnclosingRoot for different paths in the same enclosing root "
+ "returns the same path",
fs.getEnclosingRoot(root),
fs.getEnclosingRoot(methodPath()));
}


Expand All @@ -57,7 +62,8 @@ public void testEnclosingRootPathExists() throws Exception {
Path foobar = methodPath();
fs.mkdirs(foobar);

assertEquals("Ensure getEnclosingRoot returns the root directory when the root directory exists",
assertEquals(
"Ensure getEnclosingRoot returns the root directory when the root directory exists",
root, fs.getEnclosingRoot(foobar));
assertEquals("Ensure getEnclosingRoot returns the root directory when the directory exists",
root, fs.getEnclosingRoot(foobar));
Expand All @@ -70,9 +76,11 @@ public void testEnclosingRootPathDNE() throws Exception {
Path root = path("/");

// .
assertEquals("Ensure getEnclosingRoot returns the root directory even when the path does not exist",
assertEquals(
"Ensure getEnclosingRoot returns the root directory even when the path does not exist",
root, fs.getEnclosingRoot(foobar));
assertEquals("Ensure getEnclosingRoot returns the root directory even when the path does not exist",
assertEquals(
"Ensure getEnclosingRoot returns the root directory even when the path does not exist",
root, fs.getEnclosingRoot(methodPath()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,34 +193,34 @@ Path getTrashRootInFallBackFS() throws IOException {
@Test
public void testTrashRootsAfterEncryptionZoneDeletion() throws Exception {
try {
final Path zone = new Path("/EZ");
fsTarget.mkdirs(zone);
final Path zone1 = new Path("/EZ/zone1");
fsTarget.mkdirs(zone1);

DFSTestUtil.createKey("test_key", cluster, CONF);
HdfsAdmin hdfsAdmin = new HdfsAdmin(cluster.getURI(0), CONF);
final EnumSet<CreateEncryptionZoneFlag> provisionTrash =
EnumSet.of(CreateEncryptionZoneFlag.PROVISION_TRASH);
hdfsAdmin.createEncryptionZone(zone1, "test_key", provisionTrash);

final Path encFile = new Path(zone1, "encFile");
DFSTestUtil.createFile(fsTarget, encFile, 10240, (short) 1, 0xFEED);

Configuration clientConf = new Configuration(CONF);
clientConf.setLong(FS_TRASH_INTERVAL_KEY, 1);
clientConf.set("fs.default.name", fsTarget.getUri().toString());
FsShell shell = new FsShell(clientConf);

//Verify file deletion within EZ
DFSTestUtil.verifyDelete(shell, fsTarget, encFile, true);
assertTrue("ViewFileSystem trash roots should include EZ file trash",
(fsView.getTrashRoots(true).size() == 1));

//Verify deletion of EZ
DFSTestUtil.verifyDelete(shell, fsTarget, zone, true);
assertTrue("ViewFileSystem trash roots should include EZ zone trash",
(fsView.getTrashRoots(true).size() == 2));
final Path zone = new Path("/EZ");
fsTarget.mkdirs(zone);
final Path zone1 = new Path("/EZ/zone1");
fsTarget.mkdirs(zone1);

DFSTestUtil.createKey("test_key", cluster, CONF);
HdfsAdmin hdfsAdmin = new HdfsAdmin(cluster.getURI(0), CONF);
final EnumSet<CreateEncryptionZoneFlag> provisionTrash =
EnumSet.of(CreateEncryptionZoneFlag.PROVISION_TRASH);
hdfsAdmin.createEncryptionZone(zone1, "test_key", provisionTrash);

final Path encFile = new Path(zone1, "encFile");
DFSTestUtil.createFile(fsTarget, encFile, 10240, (short) 1, 0xFEED);

Configuration clientConf = new Configuration(CONF);
clientConf.setLong(FS_TRASH_INTERVAL_KEY, 1);
clientConf.set("fs.default.name", fsTarget.getUri().toString());
FsShell shell = new FsShell(clientConf);

//Verify file deletion within EZ
DFSTestUtil.verifyDelete(shell, fsTarget, encFile, true);
assertTrue("ViewFileSystem trash roots should include EZ file trash",
(fsView.getTrashRoots(true).size() == 1));

//Verify deletion of EZ
DFSTestUtil.verifyDelete(shell, fsTarget, zone, true);
assertTrue("ViewFileSystem trash roots should include EZ zone trash",
(fsView.getTrashRoots(true).size() == 2));
} finally {
DFSTestUtil.deleteKey("test_key", cluster);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this in a separate JIRA?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i recall correctly I needed to add this because a test was failing. My new tests also use test keys, and i don't think this cleanup is correct between tests.
I can try and remove it thought and double check if you prefer (It just takes a long time for tests to run), but happy to do it

}
Expand Down