From 7b425137ff3435d2759e4b03199082e5fe59c3c9 Mon Sep 17 00:00:00 2001 From: skysiders Date: Tue, 3 May 2022 19:49:32 +0800 Subject: [PATCH 1/5] ensure mkDirForAM create directory with special permissions --- .../main/java/org/apache/tez/common/TezCommonUtils.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java b/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java index 3163968908..2ad1c25c49 100644 --- a/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java +++ b/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java @@ -291,7 +291,14 @@ public static Path getSummaryRecoveryPath(Path attemptRecoverPath) { * @throws IOException */ public static void mkDirForAM(FileSystem fs, Path dir) throws IOException { - fs.mkdirs(dir, new FsPermission(TEZ_AM_DIR_PERMISSION)); + FsPermission perm = new FsPermission(TEZ_AM_DIR_PERMISSION); + fs.mkdirs(dir, perm); + if(!fs.getFileStatus(dir).getPermission().equals(perm)){ + LOG.warn("Directory " + dir.toString() + " created with unexpected permissions : " + + fs.getFileStatus(dir).getPermission() + ". Fixing permissions to correct value : " + + perm.toString()); + fs.setPermission(dir,perm); + } } /** From 72808529450c5f7018fd30f449984307c3361b36 Mon Sep 17 00:00:00 2001 From: skysiders Date: Sat, 7 May 2022 16:29:57 +0800 Subject: [PATCH 2/5] add UnitTest and fix checkstyle --- .../java/org/apache/tez/common/TezCommonUtils.java | 10 +++++----- .../java/org/apache/tez/common/TestTezCommonUtils.java | 7 +++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java b/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java index 2ad1c25c49..5c2876444c 100644 --- a/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java +++ b/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java @@ -293,11 +293,11 @@ public static Path getSummaryRecoveryPath(Path attemptRecoverPath) { public static void mkDirForAM(FileSystem fs, Path dir) throws IOException { FsPermission perm = new FsPermission(TEZ_AM_DIR_PERMISSION); fs.mkdirs(dir, perm); - if(!fs.getFileStatus(dir).getPermission().equals(perm)){ - LOG.warn("Directory " + dir.toString() + " created with unexpected permissions : " - + fs.getFileStatus(dir).getPermission() + ". Fixing permissions to correct value : " - + perm.toString()); - fs.setPermission(dir,perm); + if (!fs.getFileStatus(dir).getPermission().equals(perm)) { + LOG.warn("Directory " + dir.toString() + " created with unexpected permissions : " + + fs.getFileStatus(dir).getPermission() + ". Fixing permissions to correct value : " + + perm.toString()); + fs.setPermission(dir, perm); } } diff --git a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java index d7bd397386..d3b91cb547 100644 --- a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java +++ b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java @@ -28,6 +28,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.yarn.api.records.LocalResource; import org.apache.hadoop.yarn.api.records.LocalResourceType; @@ -413,4 +414,10 @@ public void testGetDAGSessionTimeout() { } + @Test + public void testMkDirForAM() throws IOException { + Path path = new Path("/tmp/testMkDirForAM"); + TezCommonUtils.mkDirForAM(remoteFs, path); + Assert.assertEquals(TezCommonUtils.TEZ_AM_DIR_PERMISSION, remoteFs.getFileStatus(path).getPermission()); + } } From e1f83742a757048dfa0d905d8c53dbd4fe5b3e82 Mon Sep 17 00:00:00 2001 From: skysiders Date: Sat, 7 May 2022 17:14:17 +0800 Subject: [PATCH 3/5] fix mistake in UnitTest --- .../org/apache/tez/common/TestTezCommonUtils.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java index d3b91cb547..c47447d999 100644 --- a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java +++ b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java @@ -26,6 +26,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; @@ -416,8 +417,14 @@ public void testGetDAGSessionTimeout() { @Test public void testMkDirForAM() throws IOException { - Path path = new Path("/tmp/testMkDirForAM"); - TezCommonUtils.mkDirForAM(remoteFs, path); - Assert.assertEquals(TezCommonUtils.TEZ_AM_DIR_PERMISSION, remoteFs.getFileStatus(path).getPermission()); + Configuration remoteConf = new Configuration(); + remoteConf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR); + remoteConf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "777"); + MiniDFSCluster miniDFS = new MiniDFSCluster.Builder(remoteConf).numDataNodes(3).format(true).racks(null) + .build(); + FileSystem remoteFileSystem = miniDFS.getFileSystem(); + Path path = new Path(TEST_ROOT_DIR + "/testMkDirForAM"); + TezCommonUtils.mkDirForAM(remoteFileSystem, path); + Assert.assertEquals(TezCommonUtils.TEZ_AM_DIR_PERMISSION, remoteFileSystem.getFileStatus(path).getPermission()); } } From 85039e38aa9f84cf22cdb036c838f7765cf2efeb Mon Sep 17 00:00:00 2001 From: skysiders Date: Sat, 7 May 2022 20:29:51 +0800 Subject: [PATCH 4/5] fix mistake in UnitTest --- .../src/test/java/org/apache/tez/common/TestTezCommonUtils.java | 1 - 1 file changed, 1 deletion(-) diff --git a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java index c47447d999..5b2f12684d 100644 --- a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java +++ b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java @@ -29,7 +29,6 @@ import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.yarn.api.records.LocalResource; import org.apache.hadoop.yarn.api.records.LocalResourceType; From af4f8d19fb7496c97a88144755dd647a64e46c69 Mon Sep 17 00:00:00 2001 From: skysiders Date: Mon, 9 May 2022 14:49:19 +0800 Subject: [PATCH 5/5] shutdown miniDFS cluster --- .../src/test/java/org/apache/tez/common/TestTezCommonUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java index 5b2f12684d..d5dc6fd6b5 100644 --- a/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java +++ b/tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java @@ -425,5 +425,6 @@ public void testMkDirForAM() throws IOException { Path path = new Path(TEST_ROOT_DIR + "/testMkDirForAM"); TezCommonUtils.mkDirForAM(remoteFileSystem, path); Assert.assertEquals(TezCommonUtils.TEZ_AM_DIR_PERMISSION, remoteFileSystem.getFileStatus(path).getPermission()); + miniDFS.shutdown(); } }