YARN-5287. LinuxContainerExecutor fails to set proper permission. Contributed by Ying Zhang

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c

@@ -569,6 +569,15 @@ int create_validate_dir(const char* npath, mode_t perm, const char* path,
       if (check_dir(npath, sb.st_mode, perm, finalComponent) == -1) {
         return -1;
       }
+    } else {
+      // Explicitly set permission after creating the directory in case
+      // umask has been set to a restrictive value, i.e., 0077.
+      if (chmod(npath, perm) != 0) {
+        int permInt = perm & (S_IRWXU | S_IRWXG | S_IRWXO);
+        fprintf(LOGFILE, "Can't chmod %s to the required permission %o - %s\n",
+                npath, permInt, strerror(errno));
+        return -1;
+      }
     }
   } else {
     if (check_dir(npath, sb.st_mode, perm, finalComponent) == -1) {
@@ -599,7 +608,7 @@ int check_dir(const char* npath, mode_t st_mode, mode_t desired, int finalCompon
  * Function to prepare the container directories.
  * It creates the container work and log directories.
  */
-static int create_container_directories(const char* user, const char *app_id,
+int create_container_directories(const char* user, const char *app_id,
     const char *container_id, char* const* local_dir, char* const* log_dir, const char *work_dir) {
   // create dirs as 0750
   const mode_t perms = S_IRWXU | S_IRGRP | S_IXGRP;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h

@@ -279,3 +279,13 @@ int traffic_control_read_stats(char *command_file);
  * Run a docker command passing the command file as an argument
  */
 int run_docker(const char *command_file);
+
+/**
+ * Function to prepare the container directories.
+ * It creates the container work and log directories.
+ */
+int create_container_directories(const char* user, const char *app_id,
+                     const char *container_id, char* const* local_dir,
+                     char* const* log_dir, const char *work_dir);
+
+int create_log_dirs(const char *app_id, char * const * log_dirs);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c

@@ -808,6 +808,44 @@ void test_recursive_unlink_children() {
   }
 }
 
+/**
+ * This test is used to verify that app and container directories can be
+ * created with required permissions when umask has been set to a restrictive
+ * value of 077.
+ */
+void test_dir_permissions() {
+  printf("\nTesting dir permissions\n");
+
+  // Set umask to 077
+  umask(077);
+
+  // Change user to the yarn user. This only takes effect when we're
+  // running as root.
+  if (seteuid(user_detail->pw_uid) != 0) {
+    printf("FAIL: failed to seteuid to user - %s\n", strerror(errno));
+    exit(1);
+  }
+
+  // Create container directories for "app_5"
+  char* container_dir = get_container_work_directory(TEST_ROOT "/local-1",
+                                     yarn_username, "app_5", "container_1");
+  create_log_dirs("app_5", log_dirs);
+  create_container_directories(yarn_username, "app_5", "container_1",
+                               local_dirs, log_dirs, container_dir);
+
+  // Verify directories have been created with required permissions
+  mode_t container_dir_perm = S_IRWXU | S_IRGRP | S_IXGRP;
+  struct stat sb;
+  if (stat(container_dir, &sb) != 0 ||
+      check_dir(container_dir, sb.st_mode, container_dir_perm, 1) != 0) {
+    printf("FAIL: failed to create container directory %s "
+           "with required permissions\n", container_dir);
+    exit(1);
+  }
+
+  free(container_dir);
+}
+
 // This test is expected to be executed either by a regular
 // user or by root. If executed by a regular user it doesn't
 // test all the functions that would depend on changing the
@@ -936,6 +974,10 @@ int main(int argc, char **argv) {
     test_run_container();
   }
 
+  // This test needs to be run in a subshell, so that when it changes umask
+  // and user, it doesn't give up our privs.
+  run_test_in_child("test_dir_permissions", test_dir_permissions);
+
   /*
    * try to seteuid(0).  if it doesn't work, carry on anyway.
    * we're going to capture the return value to get rid of a