HADOOP-18144. getTrashRoot/s in ViewFileSystem return a viewFS path

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java

@@ -191,8 +191,8 @@ public boolean moveToTrash(Path path) throws IOException {
         cause = e;
       }
     }
-    throw (IOException)
-      new IOException("Failed to move to trash: " + path).initCause(cause);
+    throw (IOException) new IOException(
+        "Failed to move " + path + " to trash " + trashPath).initCause(cause);
   }
 
   @SuppressWarnings("deprecation")

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/Constants.java

@@ -134,8 +134,10 @@ public interface Constants {
       HCFSMountTableConfigLoader.class;
 
   /**
-   * Enable ViewFileSystem to return a trashRoot which is local to mount point.
+   * Enable ViewFileSystem to return a trashRoot which is in the root dir of a
+   * mount point.
    */
-  String CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH = "fs.viewfs.mount.point.local.trash";
-  boolean CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT = false;
+  String CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT =
+      "fs.viewfs.trash.root.under.mount.point.root";
+  boolean CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT = false;
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java

@@ -25,8 +25,8 @@
 import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS;
 import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT;
 import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555;
-import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH;
-import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT;
+import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT;
+import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT;
 
 import java.util.function.Function;
 import java.io.FileNotFoundException;
@@ -1128,51 +1128,58 @@ public Collection<? extends BlockStoragePolicySpi> getAllStoragePolicies()
     return allPolicies;
   }
 
+  // Test whether a path is inside a snapshot or encrypted.
+  boolean isSnapshotEnabledOrEncrypted(Path p) throws IOException {
+    try {
+      FileStatus status = getFileStatus(p);
+      return status.isSnapshotEnabled() || status.isEncrypted();
+    } catch (FileNotFoundException ignored) {
+      // return false if path p does not exist yet.
+      return false;
+    }
+  }
+
   /**
    * Get the trash root directory for current user when the path
    * specified is deleted.
    *
-   * If CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is not set, return
+   * If CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is not set, return
    * the default trash root from targetFS.
    *
-   * When CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is set to true,
-   * 1) If path p is in fallback FS or from the same mount point as the default
-   *    trash root for targetFS, return the default trash root for targetFS.
-   * 2) else, return a trash root in the mounted targetFS
-   *    (/mntpoint/.Trash/{user})
+   * When CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is set to true,
+   * 1) If path p is in a snapshot or encryption zone, or when it is in the
+   *    fallback FS, return the default trash root from targetFS.
+   * 2) else, return a viewFS path for the trash root under the root of the
+   *    mount point (/mntpoint/.Trash/{user}).
    *
    * @param path the trash root of the path to be determined.
    * @return the trash root path.
    */
   @Override
   public Path getTrashRoot(Path path) {
-    boolean useMountPointLocalTrash =
-        config.getBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH,
-            CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT);
+    boolean trashRootUnderMountPointRoot =
+        config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT,
+            CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT);
 
     try {
       InodeTree.ResolveResult<FileSystem> res =
           fsState.resolve(getUriPath(path), true);
 
-      Path trashRoot = res.targetFileSystem.getTrashRoot(res.remainingPath);
-      if (!useMountPointLocalTrash) {
-        return trashRoot;
-      } else {
-        // Path p is either in a mount point or in the fallback FS
+      Path targetFSTrashRoot =
+          res.targetFileSystem.getTrashRoot(res.remainingPath);
 
-        if (ROOT_PATH.equals(new Path(res.resolvedPath))
-            || trashRoot.toUri().getPath().startsWith(res.resolvedPath)) {
-          // Path p is in the fallback FS or targetFileSystem.trashRoot is in
-          // the same mount point as Path p
-          return trashRoot;
-        } else {
-          // targetFileSystem.trashRoot is in a different mount point from
-          // Path p. Return the trash root for the mount point.
-          Path mountPointRoot =
-              res.targetFileSystem.getFileStatus(new Path("/")).getPath();
-          return new Path(mountPointRoot,
-              TRASH_PREFIX + "/" + ugi.getShortUserName());
-        }
+      if (!trashRootUnderMountPointRoot) {
+        return targetFSTrashRoot;
+      }
+
+      // New trash root policy
+      if (isSnapshotEnabledOrEncrypted(path) ||
+          ROOT_PATH.equals(new Path(res.resolvedPath))) {
+        return targetFSTrashRoot;
+      } else {
+        // Return the trash root for the mount point.
+        return new Path(res.resolvedPath,
+            TRASH_PREFIX + "/" + ugi.getShortUserName());
       }
     } catch (IOException | IllegalArgumentException e) {
       throw new NotInMountpointException(path, "getTrashRoot");
@@ -1182,6 +1189,10 @@ public Path getTrashRoot(Path path) {
   /**
    * Get all the trash roots for current user or all users.
    *
+   * When CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is set to true, we
+   * also return trash roots under the root of each mount point, with their
+   * viewFS paths.
+   *
    * @param allUsers return trash roots for all users if true.
    * @return all Trash root directories.
    */
@@ -1192,59 +1203,67 @@ public Collection<FileStatus> getTrashRoots(boolean allUsers) {
       trashRoots.addAll(fs.getTrashRoots(allUsers));
     }
 
-    // Add trash dirs for each mount point
-    boolean useMountPointLocalTrash =
-        config.getBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH,
-            CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH_DEFAULT);
-    if (useMountPointLocalTrash) {
+    boolean trashRootUnderMountPointRoot =
+        config.getBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT,
+            CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT_DEFAULT);
+    // Return trashRoots if CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT is
+    // disabled.
+    if (!trashRootUnderMountPointRoot) {
+      return trashRoots;
+    }
 
-      Set<Path> currentTrashPaths = new HashSet<>();
-      for (FileStatus file : trashRoots) {
-        currentTrashPaths.add(file.getPath());
-      }
+    // We use targetFS Path to check for duplicate paths.
+    Set<Path> uniqueTrashRoots = new HashSet<>();
+    for (FileStatus file : trashRoots) {
+      uniqueTrashRoots.add(file.getPath());
+    }
 
-      MountPoint[] mountPoints = getMountPoints();
-      try {
-        for (int i = 0; i < mountPoints.length; i++) {
-          Path trashRoot = makeQualified(
-              new Path(mountPoints[i].mountedOnPath + "/" + TRASH_PREFIX));
+    List<InodeTree.MountPoint<FileSystem>> mountPoints =
+        fsState.getMountPoints();
+    try {
+      for (InodeTree.MountPoint<FileSystem> mountPoint : mountPoints) {
 
-          // Continue if trashRoot does not exist for this filesystem
-          if (!exists(trashRoot)) {
-            continue;
-          }
+        Path trashRoot = new Path(mountPoint.src + "/" + TRASH_PREFIX);
 
-          InodeTree.ResolveResult<FileSystem> res =
-              fsState.resolve(getUriPath(trashRoot), true);
+        // Continue if trashRoot does not exist for this mount point
+        if (!exists(trashRoot)) {
+          continue;
+        }
 
-          if (!allUsers) {
-            Path userTrash =
+        if (!allUsers) {
+          Path userTrashRoot = new Path(trashRoot, ugi.getShortUserName());
+          if (exists(userTrashRoot)) {
+            Path targetFsTrashPath =
                 new Path("/" + TRASH_PREFIX + "/" + ugi.getShortUserName());
-            try {
-              FileStatus file = res.targetFileSystem.getFileStatus(userTrash);
-              if (!currentTrashPaths.contains(file.getPath())) {
-                trashRoots.add(file);
-                currentTrashPaths.add(file.getPath());
-              }
-            } catch (FileNotFoundException ignored) {
+            FileStatus targetFsTrash = mountPoint.target.getTargetFileSystem()
+                .getFileStatus(targetFsTrashPath);
+            if (!uniqueTrashRoots.contains(targetFsTrash.getPath())) {
+              FileStatus trash = getFileStatus(userTrashRoot);
+              trashRoots.add(trash);
+              uniqueTrashRoots.add(targetFsTrash.getPath());
             }
-          } else {
-            FileStatus[] targetFsTrashRoots =
-                res.targetFileSystem.listStatus(new Path("/" + TRASH_PREFIX));
-            for (FileStatus file : targetFsTrashRoots) {
-              // skip if we already include it in currentTrashPaths
-              if (currentTrashPaths.contains(file.getPath())) {
-                continue;
-              }
-
-              trashRoots.add(file);
-              currentTrashPaths.add(file.getPath());
+          }
+        } else {
+          FileStatus[] mountPointTrashRoots = listStatus(trashRoot);
+          for (FileStatus trash : mountPointTrashRoots) {
+            // Remove the mountPoint to get the targetFS path
+            Path targetFsTrashPath =
+                new Path(trash.getPath().toUri().getPath()
+                    .substring(mountPoint.src.length()));
+            FileStatus targetFsTrash = mountPoint.target.getTargetFileSystem()
+                .getFileStatus(targetFsTrashPath);
+            // skip if we already include it in uniqueTrashRoots
+            if (uniqueTrashRoots.contains(targetFsTrash.getPath())) {
+              continue;
             }
+
+            trashRoots.add(trash);
+            uniqueTrashRoots.add(targetFsTrash.getPath());
           }
         }
-      } catch (IOException e) {
-        LOG.warn("Exception in get all trash roots", e);
       }
+    } catch (IOException e) {
+      LOG.warn("Exception in get all trash roots", e);
     }
 
     return trashRoots;

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java

@@ -69,7 +69,7 @@
 import static org.apache.hadoop.fs.FileSystemTestHelper.*;
 import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_ENABLE_INNER_CACHE;
 import static org.apache.hadoop.fs.viewfs.Constants.PERMISSION_555;
-import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH;
+import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT;
 import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX;
 
 import org.junit.After;
@@ -1105,34 +1105,36 @@ public void testTrashRoot() throws IOException {
   }
 
   /**
-   * Test the localized trash root for getTrashRoot.
+   * Test trash root at the root of mount points
    */
   @Test
-  public void testTrashRootLocalizedTrash() throws IOException {
+  public void testTrashRootRootOfMountPoint() throws IOException {
     UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
     Configuration conf2 = new Configuration(conf);
-    conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true);
+    conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true);
     ConfigUtil.addLinkFallback(conf2, targetTestRoot.toUri());
     FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2);
 
-    // Case 1: path p not in the default FS.
+    // Case 1: path p in the /data mount point.
     // Return a trash root within the mount point.
     Path dataTestPath = new Path("/data/dir/file");
-    Path dataTrashRoot = new Path(targetTestRoot,
-        "data/" + TRASH_PREFIX + "/" + ugi.getShortUserName());
+    Path dataTrashRoot =
+        new Path("/data/" + TRASH_PREFIX + "/" + ugi.getShortUserName());
     Assert.assertEquals(dataTrashRoot, fsView2.getTrashRoot(dataTestPath));
 
     // Case 2: path p not found in mount table, fall back to the default FS
     // Return a trash root in user home dir
     Path nonExistentPath = new Path("/nonExistentDir/nonExistentFile");
-    Path userTrashRoot = new Path(fsTarget.getHomeDirectory(), TRASH_PREFIX);
-    Assert.assertEquals(userTrashRoot, fsView2.getTrashRoot(nonExistentPath));
+    Path targetFsTrashRoot = fsTarget.makeQualified(
+        new Path(fsTarget.getHomeDirectory().toUri().getPath(), TRASH_PREFIX));
+    Assert.assertEquals(targetFsTrashRoot,
+        fsView2.getTrashRoot(nonExistentPath));
 
     // Case 3: turn off the CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH flag.
     // Return a trash root in user home dir.
-    conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, false);
+    conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, false);
     fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2);
-    Assert.assertEquals(userTrashRoot, fsView2.getTrashRoot(dataTestPath));
+    Assert.assertEquals(targetFsTrashRoot, fsView2.getTrashRoot(dataTestPath));
 
     // Case 4: viewFS without fallback. Expect exception for a nonExistent path
     conf2 = new Configuration(conf);
@@ -1141,60 +1143,52 @@ public void testTrashRootLocalizedTrash() throws IOException {
       fsView2.getTrashRoot(nonExistentPath);
     } catch (NotInMountpointException ignored) {
     }
-
-    // Case 5: path p is in the same mount point as targetFS.getTrashRoot().
-    // Return targetFS.getTrashRoot()
-    // Use a new Configuration object, so that we can start with an empty
-    // mount table. This would avoid a conflict between the /user link in
-    // setupMountPoints() and homeDir we will need to setup for this test.
-    // default homeDir for hdfs is /user/.
-    Configuration conf3 = ViewFileSystemTestSetup.createConfig();
-    Path homeDir = fsTarget.getHomeDirectory();
-    String homeParentDir = homeDir.getParent().toUri().getPath();
-    conf3.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true);
-    ConfigUtil.addLink(conf3, homeParentDir,
-        new Path(targetTestRoot, homeParentDir).toUri());
-    Path homeTestPath = new Path(homeDir.toUri().getPath(), "testuser/file");
-    FileSystem fsView3 = FileSystem.get(FsConstants.VIEWFS_URI, conf3);
-    Assert.assertEquals(userTrashRoot, fsView3.getTrashRoot(homeTestPath));
   }
 
   /**
-   * A mocked FileSystem which returns a deep trash dir.
+   * A mocked FileSystem which returns overwrites getFileStatus() to always
+   * return an encrypted FileStatus.
    */
   static class MockTrashRootFS extends MockFileSystem {
     public static final Path TRASH =
-        new Path("/mnt/very/deep/deep/trash/dir/.Trash");
+        new Path("/very/deep/deep/trash/dir/.Trash");
 
     @Override
     public Path getTrashRoot(Path path) {
       return TRASH;
     }
+
+    @Override
+    public FileStatus getFileStatus(Path path) throws IOException {
+      FileStatus status = super.getFileStatus(path);
+      return new FileStatus(0, false, 3, 1024, 0, 0, null, "test", "testg",
+          path, path, false, true, false);
+    }
   }
 
   /**
-   * Test a trash root that is inside a mount point for getTrashRoot
+   * Test a trash root that is encrypted for getTrashRoot
    */
   @Test
-  public void testTrashRootDeepTrashDir() throws IOException {
+  public void testTrashRootEncryptedTrashDir() throws IOException {
 
     Configuration conf2 = ViewFileSystemTestSetup.createConfig();
-    conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true);
+    conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true);
     conf2.setClass("fs.mocktrashfs.impl", MockTrashRootFS.class,
         FileSystem.class);
     ConfigUtil.addLink(conf2, "/mnt", URI.create("mocktrashfs://mnt/path"));
-    Path testPath = new Path(MockTrashRootFS.TRASH, "projs/proj");
+    Path testPath = new Path("/mnt/projs/proj");
     FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2);
     Assert.assertEquals(MockTrashRootFS.TRASH, fsView2.getTrashRoot(testPath));
   }
 
   /**
-   * Test localized trash roots in getTrashRoots() for all users.
+   * Test getTrashRoots() for all users.
    */
   @Test
   public void testTrashRootsAllUsers() throws IOException {
     Configuration conf2 = new Configuration(conf);
-    conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true);
+    conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true);
     FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2);
 
     // Case 1: verify correct trash roots from fsView and fsView2
@@ -1233,14 +1227,14 @@ public void testTrashRootsAllUsers() throws IOException {
   }
 
   /**
-   * Test localized trash roots in getTrashRoots() for current user.
+   * Test getTrashRoots() for current user.
    */
   @Test
   public void testTrashRootsCurrentUser() throws IOException {
     String currentUser =
         UserGroupInformation.getCurrentUser().getShortUserName();
     Configuration conf2 = new Configuration(conf);
-    conf2.setBoolean(CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH, true);
+    conf2.setBoolean(CONFIG_VIEWFS_TRASH_ROOT_UNDER_MOUNT_POINT_ROOT, true);
     FileSystem fsView2 = FileSystem.get(FsConstants.VIEWFS_URI, conf2);
 
     int beforeTrashRootNum = fsView.getTrashRoots(false).size();