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]

Reply via email to