From c3e26735a662e478005c8c75b0909797a22cd640 Mon Sep 17 00:00:00 2001 From: Chris Nauroth Date: Thu, 26 Jun 2014 05:36:54 +0000 Subject: [PATCH] HADOOP-9705. FsShell cp -p does not preserve directory attibutes. Contributed by Akira AJISAKA. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1605672 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../fs/shell/CommandWithDestination.java | 92 +++++----- .../hadoop/fs/shell/TestCopyPreserveFlag.java | 20 +++ .../org/apache/hadoop/hdfs/TestDFSShell.java | 161 ++++++++++++++++++ 4 files changed, 238 insertions(+), 38 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index caf8d04f6d..c0fe2095a3 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -618,6 +618,9 @@ Release 2.5.0 - UNRELEASED HADOOP-10746. TestSocketIOWithTimeout#testSocketIOWithTimeout fails on Power PC. (Jinghui Wang via Arpit Agarwal) + HADOOP-9705. FsShell cp -p does not preserve directory attibutes. + (Akira AJISAKA via cnauroth) + BREAKDOWN OF HADOOP-10514 SUBTASKS AND RELATED JIRAS HADOOP-10520. Extended attributes definition and FileSystem APIs for diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java index 66b3629834..ac3b1e6c09 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java @@ -267,6 +267,9 @@ abstract class CommandWithDestination extends FsCommand { dst.refreshStatus(); // need to update stat to know it exists now } super.recursePath(src); + if (dst.stat.isDirectory()) { + preserveAttributes(src, dst); + } } finally { dst = savedDst; } @@ -298,44 +301,7 @@ abstract class CommandWithDestination extends FsCommand { try { in = src.fs.open(src.path); copyStreamToTarget(in, target); - if (shouldPreserve(FileAttribute.TIMESTAMPS)) { - target.fs.setTimes( - target.path, - src.stat.getModificationTime(), - src.stat.getAccessTime()); - } - if (shouldPreserve(FileAttribute.OWNERSHIP)) { - target.fs.setOwner( - target.path, - src.stat.getOwner(), - src.stat.getGroup()); - } - if (shouldPreserve(FileAttribute.PERMISSION) || - shouldPreserve(FileAttribute.ACL)) { - target.fs.setPermission( - target.path, - src.stat.getPermission()); - } - if (shouldPreserve(FileAttribute.ACL)) { - FsPermission perm = src.stat.getPermission(); - if (perm.getAclBit()) { - List srcEntries = - src.fs.getAclStatus(src.path).getEntries(); - List srcFullEntries = - AclUtil.getAclFromPermAndEntries(perm, srcEntries); - target.fs.setAcl(target.path, srcFullEntries); - } - } - if (shouldPreserve(FileAttribute.XATTR)) { - Map srcXAttrs = src.fs.getXAttrs(src.path); - if (srcXAttrs != null) { - Iterator> iter = srcXAttrs.entrySet().iterator(); - while (iter.hasNext()) { - Entry entry = iter.next(); - target.fs.setXAttr(target.path, entry.getKey(), entry.getValue()); - } - } - } + preserveAttributes(src, target); } finally { IOUtils.closeStream(in); } @@ -365,6 +331,56 @@ abstract class CommandWithDestination extends FsCommand { } } + /** + * Preserve the attributes of the source to the target. + * The method calls {@link #shouldPreserve(FileAttribute)} to check what + * attribute to preserve. + * @param src source to preserve + * @param target where to preserve attributes + * @throws IOException if fails to preserve attributes + */ + protected void preserveAttributes(PathData src, PathData target) + throws IOException { + if (shouldPreserve(FileAttribute.TIMESTAMPS)) { + target.fs.setTimes( + target.path, + src.stat.getModificationTime(), + src.stat.getAccessTime()); + } + if (shouldPreserve(FileAttribute.OWNERSHIP)) { + target.fs.setOwner( + target.path, + src.stat.getOwner(), + src.stat.getGroup()); + } + if (shouldPreserve(FileAttribute.PERMISSION) || + shouldPreserve(FileAttribute.ACL)) { + target.fs.setPermission( + target.path, + src.stat.getPermission()); + } + if (shouldPreserve(FileAttribute.ACL)) { + FsPermission perm = src.stat.getPermission(); + if (perm.getAclBit()) { + List srcEntries = + src.fs.getAclStatus(src.path).getEntries(); + List srcFullEntries = + AclUtil.getAclFromPermAndEntries(perm, srcEntries); + target.fs.setAcl(target.path, srcFullEntries); + } + } + if (shouldPreserve(FileAttribute.XATTR)) { + Map srcXAttrs = src.fs.getXAttrs(src.path); + if (srcXAttrs != null) { + Iterator> iter = srcXAttrs.entrySet().iterator(); + while (iter.hasNext()) { + Entry entry = iter.next(); + target.fs.setXAttr(target.path, entry.getKey(), entry.getValue()); + } + } + } + } + // Helper filter filesystem that registers created files as temp files to // be deleted on exit unless successfully renamed private static class TargetFileSystem extends FilterFileSystem { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCopyPreserveFlag.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCopyPreserveFlag.java index 9bc399e88c..ecfb5a5c33 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCopyPreserveFlag.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCopyPreserveFlag.java @@ -74,6 +74,8 @@ public class TestCopyPreserveFlag { output.close(); fs.setTimes(FROM, MODIFICATION_TIME, 0); fs.setPermission(FROM, PERMISSIONS); + fs.setTimes(new Path("d1"), MODIFICATION_TIME, 0); + fs.setPermission(new Path("d1"), PERMISSIONS); } @After @@ -132,4 +134,22 @@ public class TestCopyPreserveFlag { run(new Cp(), FROM.toString(), TO.toString()); assertAttributesChanged(); } + + @Test(timeout = 10000) + public void testDirectoryCpWithP() throws Exception { + run(new Cp(), "-p", "d1", "d3"); + assertEquals(fs.getFileStatus(new Path("d1")).getModificationTime(), + fs.getFileStatus(new Path("d3")).getModificationTime()); + assertEquals(fs.getFileStatus(new Path("d1")).getPermission(), + fs.getFileStatus(new Path("d3")).getPermission()); + } + + @Test(timeout = 10000) + public void testDirectoryCpWithoutP() throws Exception { + run(new Cp(), "d1", "d4"); + assertTrue(fs.getFileStatus(new Path("d1")).getModificationTime() != + fs.getFileStatus(new Path("d4")).getModificationTime()); + assertTrue(!fs.getFileStatus(new Path("d1")).getPermission() + .equals(fs.getFileStatus(new Path("d4")).getPermission())); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java index 286be84c4b..8788a2564b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java @@ -56,6 +56,7 @@ import org.junit.Test; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY; import static org.apache.hadoop.fs.permission.AclEntryScope.ACCESS; +import static org.apache.hadoop.fs.permission.AclEntryScope.DEFAULT; import static org.apache.hadoop.fs.permission.AclEntryType.*; import static org.apache.hadoop.fs.permission.FsAction.*; import static org.apache.hadoop.hdfs.server.namenode.AclTestHelpers.aclEntry; @@ -1778,6 +1779,166 @@ public class TestDFSShell { } } + // verify cp -ptopxa option will preserve directory attributes. + @Test (timeout = 120000) + public void testCopyCommandsToDirectoryWithPreserveOption() + throws Exception { + Configuration conf = new Configuration(); + conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_XATTRS_ENABLED_KEY, true); + conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY, true); + MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1) + .format(true).build(); + FsShell shell = null; + FileSystem fs = null; + final String testdir = + "/tmp/TestDFSShell-testCopyCommandsToDirectoryWithPreserveOption-" + + counter.getAndIncrement(); + final Path hdfsTestDir = new Path(testdir); + try { + fs = cluster.getFileSystem(); + fs.mkdirs(hdfsTestDir); + Path srcDir = new Path(hdfsTestDir, "srcDir"); + fs.mkdirs(srcDir); + + fs.setAcl(srcDir, Lists.newArrayList( + aclEntry(ACCESS, USER, ALL), + aclEntry(ACCESS, USER, "foo", ALL), + aclEntry(ACCESS, GROUP, READ_EXECUTE), + aclEntry(DEFAULT, GROUP, "bar", READ_EXECUTE), + aclEntry(ACCESS, OTHER, EXECUTE))); + // set sticky bit + fs.setPermission(srcDir, + new FsPermission(ALL, READ_EXECUTE, EXECUTE, true)); + + // Create a file in srcDir to check if modification time of + // srcDir to be preserved after copying the file. + // If cp -p command is to preserve modification time and then copy child + // (srcFile), modification time will not be preserved. + Path srcFile = new Path(srcDir, "srcFile"); + fs.create(srcFile).close(); + + FileStatus status = fs.getFileStatus(srcDir); + final long mtime = status.getModificationTime(); + final long atime = status.getAccessTime(); + final String owner = status.getOwner(); + final String group = status.getGroup(); + final FsPermission perm = status.getPermission(); + + fs.setXAttr(srcDir, "user.a1", new byte[]{0x31, 0x32, 0x33}); + fs.setXAttr(srcDir, "trusted.a1", new byte[]{0x31, 0x31, 0x31}); + + shell = new FsShell(conf); + + // -p + Path targetDir1 = new Path(hdfsTestDir, "targetDir1"); + String[] argv = new String[] { "-cp", "-p", srcDir.toUri().toString(), + targetDir1.toUri().toString() }; + int ret = ToolRunner.run(shell, argv); + assertEquals("cp -p is not working", SUCCESS, ret); + FileStatus targetStatus = fs.getFileStatus(targetDir1); + assertEquals(mtime, targetStatus.getModificationTime()); + assertEquals(atime, targetStatus.getAccessTime()); + assertEquals(owner, targetStatus.getOwner()); + assertEquals(group, targetStatus.getGroup()); + FsPermission targetPerm = targetStatus.getPermission(); + assertTrue(perm.equals(targetPerm)); + Map xattrs = fs.getXAttrs(targetDir1); + assertTrue(xattrs.isEmpty()); + List acls = fs.getAclStatus(targetDir1).getEntries(); + assertTrue(acls.isEmpty()); + assertFalse(targetPerm.getAclBit()); + + // -ptop + Path targetDir2 = new Path(hdfsTestDir, "targetDir2"); + argv = new String[] { "-cp", "-ptop", srcDir.toUri().toString(), + targetDir2.toUri().toString() }; + ret = ToolRunner.run(shell, argv); + assertEquals("cp -ptop is not working", SUCCESS, ret); + targetStatus = fs.getFileStatus(targetDir2); + assertEquals(mtime, targetStatus.getModificationTime()); + assertEquals(atime, targetStatus.getAccessTime()); + assertEquals(owner, targetStatus.getOwner()); + assertEquals(group, targetStatus.getGroup()); + targetPerm = targetStatus.getPermission(); + assertTrue(perm.equals(targetPerm)); + xattrs = fs.getXAttrs(targetDir2); + assertTrue(xattrs.isEmpty()); + acls = fs.getAclStatus(targetDir2).getEntries(); + assertTrue(acls.isEmpty()); + assertFalse(targetPerm.getAclBit()); + + // -ptopx + Path targetDir3 = new Path(hdfsTestDir, "targetDir3"); + argv = new String[] { "-cp", "-ptopx", srcDir.toUri().toString(), + targetDir3.toUri().toString() }; + ret = ToolRunner.run(shell, argv); + assertEquals("cp -ptopx is not working", SUCCESS, ret); + targetStatus = fs.getFileStatus(targetDir3); + assertEquals(mtime, targetStatus.getModificationTime()); + assertEquals(atime, targetStatus.getAccessTime()); + assertEquals(owner, targetStatus.getOwner()); + assertEquals(group, targetStatus.getGroup()); + targetPerm = targetStatus.getPermission(); + assertTrue(perm.equals(targetPerm)); + xattrs = fs.getXAttrs(targetDir3); + assertEquals(xattrs.size(), 2); + assertArrayEquals(new byte[]{0x31, 0x32, 0x33}, xattrs.get("user.a1")); + assertArrayEquals(new byte[]{0x31, 0x31, 0x31}, xattrs.get("trusted.a1")); + acls = fs.getAclStatus(targetDir3).getEntries(); + assertTrue(acls.isEmpty()); + assertFalse(targetPerm.getAclBit()); + + // -ptopa + Path targetDir4 = new Path(hdfsTestDir, "targetDir4"); + argv = new String[] { "-cp", "-ptopa", srcDir.toUri().toString(), + targetDir4.toUri().toString() }; + ret = ToolRunner.run(shell, argv); + assertEquals("cp -ptopa is not working", SUCCESS, ret); + targetStatus = fs.getFileStatus(targetDir4); + assertEquals(mtime, targetStatus.getModificationTime()); + assertEquals(atime, targetStatus.getAccessTime()); + assertEquals(owner, targetStatus.getOwner()); + assertEquals(group, targetStatus.getGroup()); + targetPerm = targetStatus.getPermission(); + assertTrue(perm.equals(targetPerm)); + xattrs = fs.getXAttrs(targetDir4); + assertTrue(xattrs.isEmpty()); + acls = fs.getAclStatus(targetDir4).getEntries(); + assertFalse(acls.isEmpty()); + assertTrue(targetPerm.getAclBit()); + assertEquals(fs.getAclStatus(srcDir), fs.getAclStatus(targetDir4)); + + // -ptoa (verify -pa option will preserve permissions also) + Path targetDir5 = new Path(hdfsTestDir, "targetDir5"); + argv = new String[] { "-cp", "-ptoa", srcDir.toUri().toString(), + targetDir5.toUri().toString() }; + ret = ToolRunner.run(shell, argv); + assertEquals("cp -ptoa is not working", SUCCESS, ret); + targetStatus = fs.getFileStatus(targetDir5); + assertEquals(mtime, targetStatus.getModificationTime()); + assertEquals(atime, targetStatus.getAccessTime()); + assertEquals(owner, targetStatus.getOwner()); + assertEquals(group, targetStatus.getGroup()); + targetPerm = targetStatus.getPermission(); + assertTrue(perm.equals(targetPerm)); + xattrs = fs.getXAttrs(targetDir5); + assertTrue(xattrs.isEmpty()); + acls = fs.getAclStatus(targetDir5).getEntries(); + assertFalse(acls.isEmpty()); + assertTrue(targetPerm.getAclBit()); + assertEquals(fs.getAclStatus(srcDir), fs.getAclStatus(targetDir5)); + } finally { + if (shell != null) { + shell.close(); + } + if (fs != null) { + fs.delete(hdfsTestDir, true); + fs.close(); + } + cluster.shutdown(); + } + } + // Verify cp -pa option will preserve both ACL and sticky bit. @Test (timeout = 120000) public void testCopyCommandsPreserveAclAndStickyBit() throws Exception {