This is an automated email from the ASF dual-hosted git repository.
sammichen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 786e53b128 HDDS-6610. Remove support for recursive volume list/delete
using ozone fs command (#5264)
786e53b128 is described below
commit 786e53b1287015c367713f9b0e07a486f16933e8
Author: ashishkumar50 <[email protected]>
AuthorDate: Thu Sep 14 11:03:45 2023 +0530
HDDS-6610. Remove support for recursive volume list/delete using ozone fs
command (#5264)
---
.../hadoop/fs/ozone/TestRootedDDSWithFSO.java | 5 +--
.../hadoop/fs/ozone/TestRootedOzoneFileSystem.java | 38 ++++++++++----------
.../fs/ozone/TestRootedOzoneFileSystemWithFSO.java | 3 +-
.../rooted/ITestRootedOzoneContractRootDir.java | 23 ++++++++++++
.../hadoop/ozone/shell/TestOzoneShellHA.java | 42 ++++++++++++++++------
.../fs/ozone/BasicRootedOzoneFileSystem.java | 32 +++++++++--------
6 files changed, 95 insertions(+), 48 deletions(-)
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedDDSWithFSO.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedDDSWithFSO.java
index 6e6abb6e77..e3cd9b6cf0 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedDDSWithFSO.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedDDSWithFSO.java
@@ -129,7 +129,7 @@ public class TestRootedDDSWithFSO {
Path root = new Path("/");
FileStatus[] fileStatuses = fs.listStatus(root);
for (FileStatus fileStatus : fileStatuses) {
- fs.delete(fileStatus.getPath(), true);
+ fs.delete(fileStatus.getPath(), false);
}
} catch (IOException ex) {
fail("Failed to cleanup files.");
@@ -191,7 +191,8 @@ public class TestRootedDDSWithFSO {
OMMetrics omMetrics = cluster.getOzoneManager().getMetrics();
long prevDeletes = omMetrics.getNumKeyDeletes();
- Assert.assertTrue(fs.delete(volumePath, true));
+ Assert.assertTrue(fs.delete(bucketPath, true));
+ Assert.assertTrue(fs.delete(volumePath, false));
long deletes = omMetrics.getNumKeyDeletes();
Assert.assertEquals(prevDeletes + 1, deletes);
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
index 98927db428..bcd1d0e41c 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
@@ -198,7 +198,8 @@ public class TestRootedOzoneFileSystem {
@After
public void cleanup() throws IOException {
- fs.delete(volumePath, true);
+ fs.delete(bucketPath, true);
+ fs.delete(volumePath, false);
}
public static FileSystem getFs() {
@@ -546,9 +547,9 @@ public class TestRootedOzoneFileSystem {
@Test
public void testListStatusIteratorWithPathNotFound() throws Exception {
- Path root = new Path("/test");
+ Path path = new Path("/test/test1/test2");
try {
- ofs.listStatusIterator(root);
+ ofs.listStatusIterator(path);
Assert.fail("Should have thrown OMException");
} catch (OMException omEx) {
Assert.assertEquals("Volume test is not found",
@@ -1514,14 +1515,6 @@ public class TestRootedOzoneFileSystem {
Assert.assertTrue(fs.delete(volumePath3, false));
// Verify the volume is deleted
Assert.assertFalse(volumeExist(volumeStr3));
-
- // Test recursively delete volume
- // Create test volume, bucket and key
- fs.mkdirs(dirPath3);
- // Delete volume recursively
- Assert.assertTrue(fs.delete(volumePath3, true));
- // Verify the volume is deleted
- Assert.assertFalse(volumeExist(volumeStr3));
}
private void createSymlinkSrcDestPaths(String srcVol,
@@ -1561,7 +1554,7 @@ public class TestRootedOzoneFileSystem {
try (GenericTestUtils.SystemOutCapturer capture =
new GenericTestUtils.SystemOutCapturer()) {
String linkPathStr = rootPath + destVolume;
- ToolRunner.run(shell, new String[]{"-ls", "-R", linkPathStr});
+ ToolRunner.run(shell, new String[]{"-ls", linkPathStr});
Assert.assertTrue(capture.getOutput().contains("drwxrwxrwx"));
Assert.assertTrue(capture.getOutput().contains(linkPathStr +
OZONE_URI_DELIMITER + srcBucket));
@@ -1599,8 +1592,10 @@ public class TestRootedOzoneFileSystem {
// due to bug - HDDS-7884
fs.delete(new Path(OZONE_URI_DELIMITER + destVolume +
OZONE_URI_DELIMITER + srcBucket));
- fs.delete(new Path(OZONE_URI_DELIMITER + srcVolume), true);
- fs.delete(new Path(OZONE_URI_DELIMITER + destVolume), true);
+ fs.delete(new Path(OZONE_URI_DELIMITER + srcVolume +
+ OZONE_URI_DELIMITER + srcBucket));
+ fs.delete(new Path(OZONE_URI_DELIMITER + srcVolume), false);
+ fs.delete(new Path(OZONE_URI_DELIMITER + destVolume), false);
}
}
@@ -1707,8 +1702,10 @@ public class TestRootedOzoneFileSystem {
// due to bug - HDDS-7884
fs.delete(new Path(OZONE_URI_DELIMITER + destVolume + OZONE_URI_DELIMITER
+ srcBucket));
- fs.delete(new Path(OZONE_URI_DELIMITER + srcVolume), true);
- fs.delete(new Path(OZONE_URI_DELIMITER + destVolume), true);
+ fs.delete(new Path(OZONE_URI_DELIMITER + srcVolume + OZONE_URI_DELIMITER
+ + srcBucket));
+ fs.delete(new Path(OZONE_URI_DELIMITER + srcVolume), false);
+ fs.delete(new Path(OZONE_URI_DELIMITER + destVolume), false);
}
}
@@ -1741,8 +1738,8 @@ public class TestRootedOzoneFileSystem {
// confirm non recursive delete of volume with link fails
deleteNonRecursivelyAndFail(linkVolumePath);
- // confirm recursive delete of volume with link works
- fs.delete(linkVolumePath, true);
+ fs.delete(linkPath, true);
+ fs.delete(linkVolumePath, false);
// confirm vol1 data is unaffected
Assert.assertTrue(dir1Status.equals(fs.getFileStatus(dirPath1)));
@@ -1753,7 +1750,8 @@ public class TestRootedOzoneFileSystem {
assertTrue(exception.getMessage().contains("File not found."));
// Cleanup
- fs.delete(volumePath1, true);
+ fs.delete(bucketPath1, true);
+ fs.delete(volumePath1, false);
}
@@ -2289,7 +2287,7 @@ public class TestRootedOzoneFileSystem {
assertThrows(OMException.class,
() -> ofs.getFileStatus(new Path(volume, bucketNameLocal)));
// Cleanup
- ofs.delete(volume, true);
+ ofs.delete(volume, false);
}
@Test
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java
index 8a7c3c453b..d923f41a75 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java
@@ -240,7 +240,8 @@ public class TestRootedOzoneFileSystemWithFSO
*/
long prevDeletes = getOMMetrics().getNumKeyDeletes();
- Assert.assertTrue(getFs().delete(volumePath1, true));
+ Assert.assertTrue(getFs().delete(bucketPath2, true));
+ Assert.assertTrue(getFs().delete(volumePath1, false));
long deletes = getOMMetrics().getNumKeyDeletes();
Assert.assertTrue(deletes == prevDeletes + 1);
}
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
index 5d7c7a9485..f4e27df2cd 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
@@ -57,4 +57,27 @@ public class ITestRootedOzoneContractRootDir extends
public void testRmNonEmptyRootDirNonRecursive() {
// OFS doesn't support creating files directly under root
}
+
+ @Override
+ public void testRmEmptyRootDirNonRecursive() {
+ // Internally test deletes volume recursively
+ // Which is not supported
+ }
+
+ @Override
+ public void testListEmptyRootDirectory() {
+ // Internally test deletes volume recursively
+ // Which is not supported
+ }
+
+ @Override
+ public void testSimpleRootListing() {
+ // Recursive list is not supported
+ }
+
+ @Override
+ public void testMkDirDepth1() {
+ // Internally test deletes volume recursively
+ // Which is not supported
+ }
}
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
index ffb67ba38f..bd68c1501e 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
@@ -659,7 +659,7 @@ public class TestOzoneShellHA {
getClientConfForOFS(hostPrefix, cluster.getConf());
OzoneFsShell shell = new OzoneFsShell(clientConf);
- try (FileSystem fs = FileSystem.get(clientConf)) {
+ try {
int res;
// Test orphan link bucket when source volume removed
res = ToolRunner.run(shell, new String[]{"-mkdir", "-p",
@@ -675,17 +675,29 @@ public class TestOzoneShellHA {
"/linkvol/linkbuck"};
execute(ozoneShell, args);
- res = ToolRunner.run(shell, new String[]{"-rm", "-R", "-f",
- "-skipTrash", hostPrefix + "/vol1"});
- assertEquals(0, res);
+ args =
+ new String[] {"volume", "delete", "vol1", "-r", "--yes"};
+ execute(ozoneShell, args);
+ out.reset();
+ OMException omExecution = assertThrows(OMException.class,
+ () -> client.getObjectStore().getVolume("vol1"));
+ assertEquals(VOLUME_NOT_FOUND, omExecution.getResult());
- res = ToolRunner.run(shell, new String[]{"-ls", "-R",
+ res = ToolRunner.run(shell, new String[]{"-ls",
hostPrefix + "/linkvol"});
assertEquals(0, res);
- res = ToolRunner.run(shell, new String[]{"-rm", "-R", "-f",
- "-skipTrash", hostPrefix + "/linkvol"});
- assertEquals(0, res);
+ args = new String[] {"bucket", "delete", "linkvol"
+ + OZONE_URI_DELIMITER + "linkbuck"};
+ execute(ozoneShell, args);
+ out.reset();
+
+ args = new String[] {"volume", "delete", "linkvol"};
+ execute(ozoneShell, args);
+ out.reset();
+ omExecution = assertThrows(OMException.class,
+ () -> client.getObjectStore().getVolume("linkvol"));
+ assertEquals(VOLUME_NOT_FOUND, omExecution.getResult());
// Test orphan link bucket when only source bucket removed
res = ToolRunner.run(shell, new String[]{"-mkdir", "-p",
@@ -704,9 +716,17 @@ public class TestOzoneShellHA {
"-skipTrash", hostPrefix + "/vol1/bucket1"});
assertEquals(0, res);
- res = ToolRunner.run(shell, new String[]{"-rm", "-R", "-f",
- "-skipTrash", hostPrefix + "/linkvol"});
- assertEquals(0, res);
+ args = new String[] {"bucket", "delete", "linkvol"
+ + OZONE_URI_DELIMITER + "linkbuck"};
+ execute(ozoneShell, args);
+ out.reset();
+
+ args = new String[] {"volume", "delete", "linkvol"};
+ execute(ozoneShell, args);
+ out.reset();
+ omExecution = assertThrows(OMException.class,
+ () -> client.getObjectStore().getVolume("linkvol"));
+ assertEquals(VOLUME_NOT_FOUND, omExecution.getResult());
} finally {
shell.close();
}
diff --git
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
index 62f1b908dd..3ed415e931 100644
---
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
+++
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
@@ -681,7 +681,14 @@ public class BasicRootedOzoneFileSystem extends FileSystem
{
// Handle delete volume
if (ofsPath.isVolume()) {
- return deleteVolume(f, recursive, ofsPath);
+ if (recursive) {
+ LOG.warn("Recursive volume delete using ofs is not supported");
+ throw new IOException("Recursive volume delete using " +
+ "ofs is not supported. " +
+ "Instead use 'ozone sh volume delete -r -skipTrash " +
+ "-id <OM_SERVICE_ID> <Volume_URI>' command");
+ }
+ return deleteVolume(f, ofsPath);
}
// delete bucket
@@ -793,7 +800,7 @@ public class BasicRootedOzoneFileSystem extends FileSystem {
}
}
- private boolean deleteVolume(Path f, boolean recursive, OFSPath ofsPath)
+ private boolean deleteVolume(Path f, OFSPath ofsPath)
throws IOException {
// verify volume exist
try {
@@ -804,18 +811,6 @@ public class BasicRootedOzoneFileSystem extends FileSystem
{
}
String volumeName = ofsPath.getVolumeName();
- if (recursive) {
- // Delete all buckets first
- OzoneVolume volume =
- adapterImpl.getObjectStore().getVolume(volumeName);
- Iterator<? extends OzoneBucket> it = volume.listBuckets("");
- String prefixVolumePathStr = addTrailingSlashIfNeeded(f.toString());
- while (it.hasNext()) {
- OzoneBucket bucket = it.next();
- String nextBucket = prefixVolumePathStr + bucket.getName();
- delete(new Path(nextBucket), true);
- }
- }
try {
adapterImpl.getObjectStore().deleteVolume(volumeName);
return true;
@@ -1139,6 +1134,15 @@ public class BasicRootedOzoneFileSystem extends
FileSystem {
@Override
public RemoteIterator<FileStatus> listStatusIterator(Path f)
throws IOException {
+ OFSPath ofsPath = new OFSPath(f,
+ OzoneConfiguration.of(getConfSource()));
+ if (ofsPath.isRoot() || ofsPath.isVolume()) {
+ LOG.warn("Recursive root/volume list using ofs is not supported");
+ throw new IOException("Recursive list root/volume " +
+ "using ofs is not supported. " +
+ "Instead use 'ozone sh key list " +
+ "<Volume_URI>' command");
+ }
return new OzoneFileStatusIterator<>(f);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]