Hadoop YARN
  1. Hadoop YARN
  2. YARN-88

DefaultContainerExecutor can fail to set proper permissions

    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
    • Component/s: nodemanager
    • Labels:
      None

      Description

      DefaultContainerExecutor can fail to set the proper permissions on its local directories if the cluster has been configured with a restrictive umask, e.g.: fs.permissions.umask-mode=0077. The configured umask ends up defeating the permissions requested by DefaultContainerExecutor when it creates directories.

      1. YARN-88.patch
        12 kB
        Jason Lowe
      2. YARN-88.patch
        13 kB
        Jason Lowe

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1204 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1204/)
          YARN-88. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580)

          Result = FAILURE
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580
          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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1204 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1204/ ) YARN-88 . DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580) Result = FAILURE sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580 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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1173 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1173/)
          YARN-88. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580)

          Result = SUCCESS
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580
          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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1173 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1173/ ) YARN-88 . DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580) Result = SUCCESS sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580 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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #382 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/382/)
          merge YARN-88 from trunk. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388584)

          Result = UNSTABLE
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388584
          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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #382 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/382/ ) merge YARN-88 from trunk. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388584) Result = UNSTABLE sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388584 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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #2775 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2775/)
          YARN-88. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580)

          Result = FAILURE
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580
          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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2775 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2775/ ) YARN-88 . DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580) Result = FAILURE sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580 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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #2753 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2753/)
          YARN-88. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580)

          Result = SUCCESS
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580
          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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2753 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2753/ ) YARN-88 . DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580) Result = SUCCESS sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580 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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2816 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2816/)
          YARN-88. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580)

          Result = SUCCESS
          sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580
          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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2816 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2816/ ) YARN-88 . DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe) (Revision 1388580) Result = SUCCESS sseth : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388580 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/DefaultContainerExecutor.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/TestDefaultContainerExecutor.java
          Hide
          Siddharth Seth added a comment -

          Committed to trunk, branch-2 and branch-23. Thanks Jason.

          Show
          Siddharth Seth added a comment - Committed to trunk, branch-2 and branch-23. Thanks Jason.
          Hide
          Siddharth Seth added a comment -

          Thanks Tom. Committing this.

          Show
          Siddharth Seth added a comment - Thanks Tom. Committing this.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12545775/YARN-88.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 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/46//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/46//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/12545775/YARN-88.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 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/46//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/46//console This message is automatically generated.
          Hide
          Thomas Graves added a comment -

          I kicked jenkins.

          Show
          Thomas Graves added a comment - I kicked jenkins.
          Hide
          Siddharth Seth added a comment -

          Patch looks good. +1 subject to a jenkins run. Need someone with access to start a run though.

          The LCE appears to have the same issue - with a restrictive umask. Also, the LCE uses 750 in most places, instead of the 710 used by the DCE.

          Show
          Siddharth Seth added a comment - Patch looks good. +1 subject to a jenkins run. Need someone with access to start a run though. The LCE appears to have the same issue - with a restrictive umask. Also, the LCE uses 750 in most places, instead of the 710 used by the DCE.
          Hide
          Siddharth Seth added a comment -

          It does make me wonder why we are explicitly granting group directory execute access to the appId directory. What does the nodemanager user need to access in there? Should we instead be locking down the usercache/${user}/appcache/${appId} to 700? If so, then we're OK using default permissions on the container and temp directories since the parent directory is locked down. If it is necessary for the appId directory to be 710, then it seems like the containerId should also be 710 and the temp directory should be 700.

          Good point, 700 does seem to be adequate. Can't think of where the NM may need group access. Needs some looking into.
          For now, will look at and commit the new patch.

          Show
          Siddharth Seth added a comment - It does make me wonder why we are explicitly granting group directory execute access to the appId directory. What does the nodemanager user need to access in there? Should we instead be locking down the usercache/${user}/appcache/${appId} to 700? If so, then we're OK using default permissions on the container and temp directories since the parent directory is locked down. If it is necessary for the appId directory to be 710, then it seems like the containerId should also be 710 and the temp directory should be 700. Good point, 700 does seem to be adequate. Can't think of where the NM may need group access. Needs some looking into. For now, will look at and commit the new patch.
          Hide
          Jason Lowe added a comment -

          Updated the patch to have the container and temp directory use the same permissions as the appId directory. This is what the LinuxContainerExecutor does.

          Show
          Jason Lowe added a comment - Updated the patch to have the container and temp directory use the same permissions as the appId directory. This is what the LinuxContainerExecutor does.
          Hide
          Jason Lowe added a comment -

          Doh, nevermind I got confused. The logs are served up from the logs directory not the container directory, and those permissions are set correctly after this patch.

          It does make me wonder why we are explicitly granting group directory execute access to the appId directory. What does the nodemanager user need to access in there? Should we instead be locking down the usercache/$

          {user}

          /appcache/$

          {appId}

          to 700? If so, then we're OK using default permissions on the container and temp directories since the parent directory is locked down. If it is necessary for the appId directory to be 710, then it seems like the containerId should also be 710 and the temp directory should be 700.

          Show
          Jason Lowe added a comment - Doh, nevermind I got confused. The logs are served up from the logs directory not the container directory, and those permissions are set correctly after this patch. It does make me wonder why we are explicitly granting group directory execute access to the appId directory. What does the nodemanager user need to access in there? Should we instead be locking down the usercache/$ {user} /appcache/$ {appId} to 700? If so, then we're OK using default permissions on the container and temp directories since the parent directory is locked down. If it is necessary for the appId directory to be 710, then it seems like the containerId should also be 710 and the temp directory should be 700.
          Hide
          Jason Lowe added a comment -

          At first I didn't think other things besides the user needed access to the files within the containerId directory, but now that I think about it I believe that's necessary for the nodemanager to serve up the logs of an actively running container. As for the temp directory, only the user should need access to that directory.

          I'll update the patch accordingly.

          Show
          Jason Lowe added a comment - At first I didn't think other things besides the user needed access to the files within the containerId directory, but now that I think about it I believe that's necessary for the nodemanager to serve up the logs of an actively running container. As for the temp directory, only the user should need access to that directory. I'll update the patch accordingly.
          Hide
          Siddharth Seth added a comment -

          Jason, the patch looks good in what it's doing. There are a few more directories created by the DefaultContainerExecutor - for which no explicit permissions are set rightnow. Specifically, usercache/$

          {user}

          /appcache/$

          {appId}

          /$

          {containerId}

          and a temp dir. Should permission for these additional dirs be set via this patch ?

          Show
          Siddharth Seth added a comment - Jason, the patch looks good in what it's doing. There are a few more directories created by the DefaultContainerExecutor - for which no explicit permissions are set rightnow. Specifically, usercache/$ {user} /appcache/$ {appId} /$ {containerId} and a temp dir. Should permission for these additional dirs be set via this patch ?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12545463/YARN-88.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 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/41//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/41//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/12545463/YARN-88.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 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/41//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/41//console This message is automatically generated.
          Hide
          Jason Lowe added a comment -

          Patch to have DefaultContainerExecutor explicitly set the permissions of directories after creating them if the umask is too restrictive.

          Show
          Jason Lowe added a comment - Patch to have DefaultContainerExecutor explicitly set the permissions of directories after creating them if the umask is too restrictive.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development