YARN-4579. Allow DefaultContainerExecutor container log directory permissions to be configurable (rchiang via rkanter)

(cherry picked from commit d7fdec1e6b)

hadoop-yarn-project/CHANGES.txt

@@ -794,6 +794,9 @@ Release 2.8.0 - UNRELEASED
     YARN-4682. AMRM client to log when AMRM token updated.
     (Prabhu Joseph via stevel)
 
+    YARN-4579. Allow DefaultContainerExecutor container log directory
+    permissions to be configurable (rchiang via rkanter)
+
   OPTIMIZATIONS
 
     YARN-3339. TestDockerContainerExecutor should pull a single image and not

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java

@@ -764,6 +764,13 @@ public class YarnConfiguration extends Configuration {
       NM_PREFIX + "logaggregation.threadpool-size-max";
   public static final int DEFAULT_NM_LOG_AGGREGATION_THREAD_POOL_SIZE = 100;
 
+  /** Default permissions for container logs. */
+  public static final String NM_DEFAULT_CONTAINER_EXECUTOR_PREFIX =
+      NM_PREFIX + "default-container-executor.";
+  public static final String NM_DEFAULT_CONTAINER_EXECUTOR_LOG_DIRS_PERMISSIONS =
+      NM_DEFAULT_CONTAINER_EXECUTOR_PREFIX + "log-dirs.permissions";
+  public static final String NM_DEFAULT_CONTAINER_EXECUTOR_LOG_DIRS_PERMISSIONS_DEFAULT = "710";
+
   public static final String NM_RESOURCEMANAGER_MINIMUM_VERSION =
       NM_PREFIX + "resourcemanager.minimum.version";
   public static final String DEFAULT_NM_RESOURCEMANAGER_MINIMUM_VERSION = "NONE";

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml

@@ -1072,6 +1072,16 @@
     <value>${yarn.log.dir}/userlogs</value>
   </property>
 
+  <property>
+    <description>
+      The permissions settings used for the creation of container
+      directories when using DefaultContainerExecutor.  This follows
+      standard user/group/all permissions format.
+    </description>
+    <name>yarn.nodemanager.default-container-executor.log-dirs.permissions</name>
+    <value>710</value>
+  </property>
+
   <property>
     <description>Whether to enable log aggregation. Log aggregation collects
       each container's logs and moves these logs onto a file-system, for e.g.

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java

@@ -74,6 +74,8 @@ public class DefaultContainerExecutor extends ContainerExecutor {
 
   protected final FileContext lfs;
 
+  private String logDirPermissions = null;
+
   public DefaultContainerExecutor() {
     try {
       this.lfs = FileContext.getLocalFSFileContext();
@@ -509,9 +511,6 @@ public class DefaultContainerExecutor extends ContainerExecutor {
   /** Permissions for user app dir.
    * $local.dir/usercache/$user/appcache/$appId */
   static final short APPDIR_PERM = (short)0710;
-  /** Permissions for user log dir.
-   * $logdir/$user/$appId */
-  static final short LOGDIR_PERM = (short)0710;
 
   private long getDiskFreeSpace(Path base) throws IOException {
     return lfs.getFsStatus(base).getRemaining();
@@ -702,7 +701,8 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       throws IOException {
 
     boolean appLogDirStatus = false;
-    FsPermission appLogDirPerms = new FsPermission(LOGDIR_PERM);
+    FsPermission appLogDirPerms = new
+        FsPermission(getLogDirPermissions());
     for (String rootLogDir : logDirs) {
       // create $log.dir/$appid
       Path appLogDir = new Path(rootLogDir, appId);
@@ -727,7 +727,8 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       List<String> logDirs, String user) throws IOException {
 
     boolean containerLogDirStatus = false;
-    FsPermission containerLogDirPerms = new FsPermission(LOGDIR_PERM);
+    FsPermission containerLogDirPerms = new
+        FsPermission(getLogDirPermissions());
     for (String rootLogDir : logDirs) {
       // create $log.dir/$appid/$containerid
       Path appLogDir = new Path(rootLogDir, appId);
@@ -749,6 +750,27 @@ public class DefaultContainerExecutor extends ContainerExecutor {
     }
   }
 
+  /**
+   * Return default container log directory permissions.
+   */
+  @VisibleForTesting
+  public String getLogDirPermissions() {
+    if (this.logDirPermissions==null) {
+      this.logDirPermissions = getConf().get(
+          YarnConfiguration.NM_DEFAULT_CONTAINER_EXECUTOR_LOG_DIRS_PERMISSIONS,
+          YarnConfiguration.NM_DEFAULT_CONTAINER_EXECUTOR_LOG_DIRS_PERMISSIONS_DEFAULT);
+    }
+    return this.logDirPermissions;
+  }
+
+  /**
+   * Clear the internal variable for repeatable testing.
+   */
+  @VisibleForTesting
+  public void clearLogDirPermissions() {
+    this.logDirPermissions = null;
+  }
+
   /**
    * @return the list of paths of given local directories
    */

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDefaultContainerExecutor.java

@@ -168,8 +168,7 @@ public class TestDefaultContainerExecutor {
         DefaultContainerExecutor.FILECACHE_PERM);
     final FsPermission appDirPerm = new FsPermission(
         DefaultContainerExecutor.APPDIR_PERM);
-    final FsPermission logDirPerm = new FsPermission(
-        DefaultContainerExecutor.LOGDIR_PERM);
+
     List<String> localDirs = new ArrayList<String>();
     localDirs.add(new Path(BASE_TMP_PATH, "localDirA").toString());
     localDirs.add(new Path(BASE_TMP_PATH, "localDirB").toString());
@@ -181,6 +180,7 @@ public class TestDefaultContainerExecutor {
     conf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "077");
     FileContext lfs = FileContext.getLocalFSFileContext(conf);
     DefaultContainerExecutor executor = new DefaultContainerExecutor(lfs);
+    executor.setConf(conf);
     executor.init();
 
     try {
@@ -208,11 +208,20 @@ public class TestDefaultContainerExecutor {
         Assert.assertEquals(appDirPerm, stats.getPermission());
       }
 
-      executor.createAppLogDirs(appId, logDirs, user);
+      String[] permissionsArray = { "000", "111", "555", "710", "777" };
 
-      for (String dir : logDirs) {
-        FileStatus stats = lfs.getFileStatus(new Path(dir, appId));
-        Assert.assertEquals(logDirPerm, stats.getPermission());
+      for (String perm : permissionsArray ) {
+        conf.set(YarnConfiguration.NM_DEFAULT_CONTAINER_EXECUTOR_LOG_DIRS_PERMISSIONS, perm);
+        executor.clearLogDirPermissions();
+        FsPermission logDirPerm = new FsPermission(
+            executor.getLogDirPermissions());
+        executor.createAppLogDirs(appId, logDirs, user);
+
+        for (String dir : logDirs) {
+          FileStatus stats = lfs.getFileStatus(new Path(dir, appId));
+          Assert.assertEquals(logDirPerm, stats.getPermission());
+          lfs.delete(new Path(dir, appId), true);
+        }
       }
     } finally {
       deleteTmpFiles();