Hadoop YARN
  1. Hadoop YARN
  2. YARN-106

Nodemanager needs to set permissions of local directories

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.3, 2.0.0-alpha
    • Fix Version/s: 2.0.2-alpha, 0.23.4, 3.0.0
    • Component/s: nodemanager
    • Labels:
      None

      Description

      If the nodemanager process is running with a restrictive default umask (e.g.: 0077) then it will create its local directories with permissions that are too restrictive to allow containers from other users to run.

      1. YARN-106.patch
        8 kB
        Jason Lowe
      2. YARN-106.patch
        7 kB
        Jason Lowe
      3. YARN-106.patch
        9 kB
        Jason Lowe
      4. YARN-106.patch
        10 kB
        Jason Lowe

        Activity

        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1211 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1211/)
        YARN-106. Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649)

        Result = FAILURE
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1211 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1211/ ) YARN-106 . Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1180 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1180/)
        YARN-106. Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1180 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1180/ ) YARN-106 . Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #389 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/389/)
        svn merge -c 1391649 FIXES: YARN-106. Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391656)

        Result = UNSTABLE
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391656
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #389 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/389/ ) svn merge -c 1391649 FIXES: YARN-106 . Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391656) Result = UNSTABLE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391656 Files : /hadoop/common/branches/branch-0.23/hadoop-yarn-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java
        Hide
        Robert Joseph Evans added a comment -

        Thanks Jason,

        I put this into trunk, branch-2, and branch-0.23

        Show
        Robert Joseph Evans added a comment - Thanks Jason, I put this into trunk, branch-2, and branch-0.23
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2847 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2847/)
        YARN-106. Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2847 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2847/ ) YARN-106 . Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #2784 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2784/)
        YARN-106. Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2784 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2784/ ) YARN-106 . Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2806 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2806/)
        YARN-106. Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649)

        Result = FAILURE
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2806 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2806/ ) YARN-106 . Nodemanager needs to set permissions of local directories (jlowe via bobby) (Revision 1391649) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1391649 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LocalDirsHandlerService.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDirectoryCollection.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestContainerLogsPage.java
        Hide
        Siddharth Seth added a comment -

        Thanks Jason and Bobby. Looks good to me as well. The absolute URI shouldn't have existed ... I missed that in YARN-22.

        Show
        Siddharth Seth added a comment - Thanks Jason and Bobby. Looks good to me as well. The absolute URI shouldn't have existed ... I missed that in YARN-22 .
        Hide
        Robert Joseph Evans added a comment -

        The changes look good to me +1. I'll check this in.

        Show
        Robert Joseph Evans added a comment - The changes look good to me +1. I'll check this in.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12546875/YARN-106.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 2 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/54//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/54//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12546875/YARN-106.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/54//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/54//console This message is automatically generated.
        Hide
        Jason Lowe added a comment -

        Failure was due to a bogus URI being used by TestContainerLogsPage that should have failed originally, but URIs in local/log dirs aren't handled properly. See YARN-33.

        Updated the TestContainerLogsPage to fix the URI so it points to a path that can be created. YARN-33 will handle cleaning up DirectoryCollection and/or LocalDirsHandlerService to deal with URIs in the local/log dir config.

        Show
        Jason Lowe added a comment - Failure was due to a bogus URI being used by TestContainerLogsPage that should have failed originally, but URIs in local/log dirs aren't handled properly. See YARN-33 . Updated the TestContainerLogsPage to fix the URI so it points to a path that can be created. YARN-33 will handle cleaning up DirectoryCollection and/or LocalDirsHandlerService to deal with URIs in the local/log dir config.
        Hide
        Jason Lowe added a comment -

        Looks like the test failure is related.

        Show
        Jason Lowe added a comment - Looks like the test failure is related.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12546600/YARN-106.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:

        org.apache.hadoop.yarn.server.nodemanager.webapp.TestContainerLogsPage

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/53//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/53//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12546600/YARN-106.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: org.apache.hadoop.yarn.server.nodemanager.webapp.TestContainerLogsPage +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/53//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/53//console This message is automatically generated.
        Hide
        Jason Lowe added a comment -

        Yep, you're right, since we explicitly remove the directory checkDirs() won't handle it so we need to handle explicitly.

        Show
        Jason Lowe added a comment - Yep, you're right, since we explicitly remove the directory checkDirs() won't handle it so we need to handle explicitly.
        Hide
        Siddharth Seth added a comment -

        Unless I'm missing something, the checkDirs() call after may not attempt to check these dirs since the directory creation failed, and they no longer exist in the localDirs collection.

        The patch to add the explicit check looks good. +1. Need someone to kick jenkins before committing it.

        Show
        Siddharth Seth added a comment - Unless I'm missing something, the checkDirs() call after may not attempt to check these dirs since the directory creation failed, and they no longer exist in the localDirs collection. The patch to add the explicit check looks good. +1. Need someone to kick jenkins before committing it.
        Hide
        Jason Lowe added a comment -

        Thanks for taking another look. Normally the checkDirs() call right afterwards will encounter the same error and update the configuration's directory list, but I agree it should explicitly handle it just in case. I updated the patch accordingly.

        Show
        Jason Lowe added a comment - Thanks for taking another look. Normally the checkDirs() call right afterwards will encounter the same error and update the configuration's directory list, but I agree it should explicitly handle it just in case. I updated the patch accordingly.
        Hide
        Siddharth Seth added a comment -

        Thanks for updating the patch. Looks good except for one minor fix. After attempting to create the dirs in LocalDirsHandlerService, the dir set needs to be updated back into the configuration, especially in case of a failure.

        Show
        Siddharth Seth added a comment - Thanks for updating the patch. Looks good except for one minor fix. After attempting to create the dirs in LocalDirsHandlerService, the dir set needs to be updated back into the configuration, especially in case of a failure.
        Hide
        Jason Lowe added a comment -

        OK I'm convinced, thanks for the clarification. Attaching a new patch where it will create missing local/log directories as mode 0755 but will leave existing directories untouched.

        Show
        Jason Lowe added a comment - OK I'm convinced, thanks for the clarification. Attaching a new patch where it will create missing local/log directories as mode 0755 but will leave existing directories untouched.
        Hide
        Siddharth Seth added a comment -

        Not the best example - but a single user setup does not need 0755 for the public cache. I don't think a non LCE cluster needs this either.
        If admins happen to have pre-created the dirs with specific permissions - I don't think we should try changing them
        1) admins are aware of these directories since they've been explicitly configured. If they exist, they've likely been created with whatever permissions are required for the cluster.
        2) the YARN user may not have privileges to change permission (may still be able to write to the dir) - in which case the dir is marked as unusable.

        Show
        Siddharth Seth added a comment - Not the best example - but a single user setup does not need 0755 for the public cache. I don't think a non LCE cluster needs this either. If admins happen to have pre-created the dirs with specific permissions - I don't think we should try changing them 1) admins are aware of these directories since they've been explicitly configured. If they exist, they've likely been created with whatever permissions are required for the cluster. 2) the YARN user may not have privileges to change permission (may still be able to write to the dir) - in which case the dir is marked as unusable.
        Hide
        Jason Lowe added a comment -

        How will public files in the distributed cache work properly if the local directory doesn't have world access? We're already locking down access to the directories within these top-level directories for files that aren't public, so even if they are left at 755 I'm not sure there is a real need to set them otherwise. Maybe there's a valid use-case I'm missing?

        I agree I'd rather avoid adding yet more configs, especially since it would be easy to configure a broken setup (e.g.: files put in the public cache but are inaccessible to many jobs or logs can't be served/aggregated by the nodemanager).

        Show
        Jason Lowe added a comment - How will public files in the distributed cache work properly if the local directory doesn't have world access? We're already locking down access to the directories within these top-level directories for files that aren't public, so even if they are left at 755 I'm not sure there is a real need to set them otherwise. Maybe there's a valid use-case I'm missing? I agree I'd rather avoid adding yet more configs, especially since it would be easy to configure a broken setup (e.g.: files put in the public cache but are inaccessible to many jobs or logs can't be served/aggregated by the nodemanager).
        Hide
        Siddharth Seth added a comment -

        This will run into issues if users want to control the permission on their local / log dirs. The patch has these set to 0755 - which I think is ok if the dirs don't already exist. If they do exist however, I don't think we should be attempting to change the permissions. Alternately, maybe configure LOG/LOCAL dir permissions ? - which could also be used to ensure the dirs have the correct permissions. Would prefer not adding additional configs though.

        Show
        Siddharth Seth added a comment - This will run into issues if users want to control the permission on their local / log dirs. The patch has these set to 0755 - which I think is ok if the dirs don't already exist. If they do exist however, I don't think we should be attempting to change the permissions. Alternately, maybe configure LOG/LOCAL dir permissions ? - which could also be used to ensure the dirs have the correct permissions. Would prefer not adding additional configs though.
        Hide
        Jason Lowe added a comment -

        Patch to change DirectoryCollection so it also takes the permissions for directories and updating LocalDirsHandlerService to pass appropriate permissions.

        Show
        Jason Lowe added a comment - Patch to change DirectoryCollection so it also takes the permissions for directories and updating LocalDirsHandlerService to pass appropriate permissions.
        Hide
        Jason Lowe added a comment -

        The nodemanager should be using the permission forms of DiskChecker.checkDir like the datanode does.

        Show
        Jason Lowe added a comment - The nodemanager should be using the permission forms of DiskChecker.checkDir like the datanode does.

          People

          • Assignee:
            Jason Lowe
            Reporter:
            Jason Lowe
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development