[ https://issues.apache.org/jira/browse/MAPREDUCE-7375?focusedWorklogId=796195&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-796195 ]
ASF GitHub Bot logged work on MAPREDUCE-7375: --------------------------------------------- Author: ASF GitHub Bot Created on: 28/Jul/22 21:38 Start Date: 28/Jul/22 21:38 Worklog Time Spent: 10m Work Description: cnauroth commented on code in PR #4237: URL: https://github.com/apache/hadoop/pull/4237#discussion_r932699216 ########## hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestJobSubmissionFiles.java: ########## @@ -139,4 +141,19 @@ public void testGetStagingDirWhenShortFileOwnerNameAndShortUserName() assertEquals(stagingPath, JobSubmissionFiles.getStagingDir(cluster, conf, user)); } + + @Test + public void testDirPermission() throws Exception { + Cluster cluster = mock(Cluster.class); + HdfsConfiguration CONF = new HdfsConfiguration(); + MiniDFSCluster cluster1 = new MiniDFSCluster.Builder(CONF).numDataNodes(2).build(); + FileSystem fs = cluster1.getFileSystem(); + UserGroupInformation user = UserGroupInformation + .createUserForTesting(USER_1_SHORT_NAME, GROUP_NAMES); + Path stagingPath = new Path(fs.getUri().toString() + "/testDirPermission"); + + when(cluster.getStagingAreaDir()).thenReturn(stagingPath); + Path res = JobSubmissionFiles.getStagingDir(cluster, CONF, user); + assertEquals(new FsPermission(0700),fs.getFileStatus(res).getPermission()); Review Comment: Nitpick: please add a single space after the comma. ########## hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestJobSubmissionFiles.java: ########## @@ -139,4 +141,19 @@ public void testGetStagingDirWhenShortFileOwnerNameAndShortUserName() assertEquals(stagingPath, JobSubmissionFiles.getStagingDir(cluster, conf, user)); } + + @Test + public void testDirPermission() throws Exception { + Cluster cluster = mock(Cluster.class); + HdfsConfiguration CONF = new HdfsConfiguration(); + MiniDFSCluster cluster1 = new MiniDFSCluster.Builder(CONF).numDataNodes(2).build(); Review Comment: Suggestion: different variable name for greater clarity, e.g. `dfsCluster`. Also, please use a `finally` block to guarantee calling `MiniDFSCluster#shutdown()` at the end of the test. ########## hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestJobSubmissionFiles.java: ########## @@ -139,4 +141,19 @@ public void testGetStagingDirWhenShortFileOwnerNameAndShortUserName() assertEquals(stagingPath, JobSubmissionFiles.getStagingDir(cluster, conf, user)); } + + @Test + public void testDirPermission() throws Exception { Review Comment: Does this test actually cover the bug? Should the test be configuring umask to 700 to simulate the bug more accurately? Ideally, this should be a test that 1) fails before applying the `JobSubmissionFiles` patch, and 2) starts passing after applying the `JobSubmissionFiles` patch. ########## hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestJobSubmissionFiles.java: ########## @@ -139,4 +141,19 @@ public void testGetStagingDirWhenShortFileOwnerNameAndShortUserName() assertEquals(stagingPath, JobSubmissionFiles.getStagingDir(cluster, conf, user)); } + + @Test + public void testDirPermission() throws Exception { + Cluster cluster = mock(Cluster.class); + HdfsConfiguration CONF = new HdfsConfiguration(); Review Comment: Nitpick: lower-case `conf` for local variables. Issue Time Tracking ------------------- Worklog Id: (was: 796195) Time Spent: 2h (was: 1h 50m) > JobSubmissionFiles don't set right permission after mkdirs > ---------------------------------------------------------- > > Key: MAPREDUCE-7375 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-7375 > Project: Hadoop Map/Reduce > Issue Type: Bug > Components: mrv2 > Affects Versions: 3.3.2 > Reporter: Zhang Dongsheng > Priority: Major > Labels: pull-request-available > Attachments: MAPREDUCE-7375.patch > > Time Spent: 2h > Remaining Estimate: 0h > > JobSubmissionFiles provide getStagingDir to get Staging Directory.If > stagingArea missing, method will create new directory with this. > {quote}fs.mkdirs(stagingArea, new FsPermission(JOB_DIR_PERMISSION));{quote} > It seems create new directory with JOB_DIR_PERMISSION,but this permission > will be apply by umask.If umask too strict , this permission may be 000(if > umask is 700).So we should change permission after create. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: mapreduce-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: mapreduce-issues-h...@hadoop.apache.org