Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: yarn
    • Labels:
      None
    • Target Version/s:

      Description

      For security reasons, we need to ensure that access to the docker daemon and the ability to run docker containers is restricted to privileged users ( i.e users running applications should not have direct access to docker). In order to ensure the node manager can run docker commands, we need to add docker support to the container-executor binary.

      1. YARN-3852-3.patch
        31 kB
        Abin Shahab
      2. YARN-3852-2.patch
        31 kB
        Abin Shahab
      3. YARN-3852-1.patch
        36 kB
        Abin Shahab
      4. YARN-3852.patch
        31 kB
        Abin Shahab

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2216 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2216/)
          YARN-3852. Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2216 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2216/ ) YARN-3852 . Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #267 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/267/)
          YARN-3852. Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2)

          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #267 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/267/ ) YARN-3852 . Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #259 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/259/)
          YARN-3852. Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #259 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/259/ ) YARN-3852 . Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2197 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2197/)
          YARN-3852. Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2)

          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2197 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2197/ ) YARN-3852 . Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #1000 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1000/)
          YARN-3852. Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1000 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1000/ ) YARN-3852 . Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #270 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/270/)
          YARN-3852. Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #270 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/270/ ) YARN-3852 . Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #8227 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8227/)
          YARN-3852. Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #8227 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8227/ ) YARN-3852 . Add docker container support to container-executor. Contributed by Abin Shahab. (vvasudev: rev f36835ff9b878fa20fe58a30f9d1e8c47702d6d2) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
          Hide
          vvasudev Varun Vasudev added a comment -

          Committed to trunk and branch-2. Thanks Abin Shahab!

          Show
          vvasudev Varun Vasudev added a comment - Committed to trunk and branch-2. Thanks Abin Shahab !
          Hide
          vvasudev Varun Vasudev added a comment -

          +1 - I'll commit this on Monday if no one objects.

          Show
          vvasudev Varun Vasudev added a comment - +1 - I'll commit this on Monday if no one objects.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 5m 29s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 45s There were no new javac warning messages.
          +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings.
          +1 whitespace 0m 5s The patch has no lines that end in whitespace.
          +1 install 1m 22s mvn install still works.
          +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
          +1 yarn tests 6m 18s Tests passed in hadoop-yarn-server-nodemanager.
              21m 55s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12746970/YARN-3852-3.patch
          Optional Tests javac unit
          git revision trunk / e202efa
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8644/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8644/testReport/
          Java 1.7.0_55
          uname Linux asf905.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/8644/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 5m 29s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 45s There were no new javac warning messages. +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings. +1 whitespace 0m 5s The patch has no lines that end in whitespace. +1 install 1m 22s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 yarn tests 6m 18s Tests passed in hadoop-yarn-server-nodemanager.     21m 55s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12746970/YARN-3852-3.patch Optional Tests javac unit git revision trunk / e202efa hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8644/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8644/testReport/ Java 1.7.0_55 uname Linux asf905.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-YARN-Build/8644/console This message was automatically generated.
          Hide
          vvasudev Varun Vasudev added a comment -

          Yes. In the current version of launch_container_as_user, we have this call

            // give up root privs
            if (change_user(user_detail->pw_uid, user_detail->pw_gid) != 0) {
              exit_code = SETUID_OPER_FAILED;
              goto cleanup;
            }
          

          After the refactor, this call has been put in an if condition

            if (effective_user == 1) {
              if (change_effective_user(user_detail->pw_uid, user_detail->pw_gid) != 0) {
                fprintf(ERRORFILE, "Could not change to effective users %d, %d\n", user_detail->pw_uid, user_detail->pw_gid);
                fflush(ERRORFILE);
                goto cleanup;
              }
            } else {
             // give up root privs
              if (change_user(user_detail->pw_uid, user_detail->pw_gid) != 0) {
                exit_code = SETUID_OPER_FAILED;
                goto cleanup;
              }
            }
          

          in create_local_dirs

          Show
          vvasudev Varun Vasudev added a comment - Yes. In the current version of launch_container_as_user, we have this call // give up root privs if (change_user(user_detail->pw_uid, user_detail->pw_gid) != 0) { exit_code = SETUID_OPER_FAILED; goto cleanup; } After the refactor, this call has been put in an if condition if (effective_user == 1) { if (change_effective_user(user_detail->pw_uid, user_detail->pw_gid) != 0) { fprintf(ERRORFILE, "Could not change to effective users %d, %d\n" , user_detail->pw_uid, user_detail->pw_gid); fflush(ERRORFILE); goto cleanup; } } else { // give up root privs if (change_user(user_detail->pw_uid, user_detail->pw_gid) != 0) { exit_code = SETUID_OPER_FAILED; goto cleanup; } } in create_local_dirs
          Hide
          ashahab Abin Shahab added a comment -

          So is this happening because of the refactor?

          On Wed, Jul 22, 2015 at 10:28 AM, Varun Vasudev (JIRA) <jira@apache.org>

          Show
          ashahab Abin Shahab added a comment - So is this happening because of the refactor? On Wed, Jul 22, 2015 at 10:28 AM, Varun Vasudev (JIRA) <jira@apache.org>
          Hide
          vvasudev Varun Vasudev added a comment -

          Sigh. My apologies Abin Shahab - I found one more issue. Docker containers are launched as the correct user but the regular process containers are being run as root.

          I suspect the root cause is the call

          exit_code = create_local_dirs(user, app_id, container_id,
              work_dir, script_name, cred_file, local_dirs, log_dirs,
              1, script_file_dest, cred_file_dest,
              container_file_source, cred_file_source);
          

          in launch_container_as_user. The effective_user argument is set to 1 when it should be 0.

          Show
          vvasudev Varun Vasudev added a comment - Sigh. My apologies Abin Shahab - I found one more issue. Docker containers are launched as the correct user but the regular process containers are being run as root. I suspect the root cause is the call exit_code = create_local_dirs(user, app_id, container_id, work_dir, script_name, cred_file, local_dirs, log_dirs, 1, script_file_dest, cred_file_dest, container_file_source, cred_file_source); in launch_container_as_user. The effective_user argument is set to 1 when it should be 0.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 5m 23s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 44s There were no new javac warning messages.
          +1 release audit 0m 19s The applied patch does not increase the total number of release audit warnings.
          +1 whitespace 0m 4s The patch has no lines that end in whitespace.
          +1 install 1m 20s mvn install still works.
          +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
          +1 yarn tests 6m 5s Tests passed in hadoop-yarn-server-nodemanager.
              21m 30s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12746480/YARN-3852-2.patch
          Optional Tests javac unit
          git revision trunk / 94c6a4a
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8603/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8603/testReport/
          Java 1.7.0_55
          uname Linux asf905.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/8603/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 5m 23s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 44s There were no new javac warning messages. +1 release audit 0m 19s The applied patch does not increase the total number of release audit warnings. +1 whitespace 0m 4s The patch has no lines that end in whitespace. +1 install 1m 20s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 yarn tests 6m 5s Tests passed in hadoop-yarn-server-nodemanager.     21m 30s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12746480/YARN-3852-2.patch Optional Tests javac unit git revision trunk / 94c6a4a hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8603/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8603/testReport/ Java 1.7.0_55 uname Linux asf905.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-YARN-Build/8603/console This message was automatically generated.
          Hide
          vvasudev Varun Vasudev added a comment -

          Thanks for the latest patch Abin Shahab. Patch looks good to me, just a couple of minor changes -

          1. In container-executor.c and container-executor.h
            -int check_dir(const char* npath, mode_t st_mode, mode_t desired, int finalComponent) {
            +int check_dir(char* npath, mode_t st_mode, mode_t desired, int finalComponent) {
            

            and

            -int check_dir(const char* npath, mode_t st_mode, mode_t desired,
            +int check_dir(char* npath, mode_t st_mode, mode_t desired,
                int finalComponent);
            
            -int create_validate_dir(const char* npath, mode_t perm, const char* path,
            +int create_validate_dir(char* npath, mode_t perm, char* path,
                int finalComponent);
            

            You've removed the const-ness of npath.

          2. In container-executor.c
            +int create_script_paths(const char *work_dir,
            +                      const char *script_name, const char *cred_file,
            +                 char** script_file_dest, char** cred_file_dest,
            +                 int* container_file_source, int* cred_file_source ) {
            

          The rest of the patch looks good to me.

          Show
          vvasudev Varun Vasudev added a comment - Thanks for the latest patch Abin Shahab . Patch looks good to me, just a couple of minor changes - In container-executor.c and container-executor.h - int check_dir( const char * npath, mode_t st_mode, mode_t desired, int finalComponent) { + int check_dir( char * npath, mode_t st_mode, mode_t desired, int finalComponent) { and - int check_dir( const char * npath, mode_t st_mode, mode_t desired, + int check_dir( char * npath, mode_t st_mode, mode_t desired, int finalComponent); - int create_validate_dir( const char * npath, mode_t perm, const char * path, + int create_validate_dir( char * npath, mode_t perm, char * path, int finalComponent); You've removed the const-ness of npath. In container-executor.c + int create_script_paths( const char *work_dir, + const char *script_name, const char *cred_file, + char ** script_file_dest, char ** cred_file_dest, + int * container_file_source, int * cred_file_source ) { The rest of the patch looks good to me.
          Hide
          ashahab Abin Shahab added a comment -

          The test failures are unrelated to the container-executor.c changes.

          Show
          ashahab Abin Shahab added a comment - The test failures are unrelated to the container-executor.c changes.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 5m 21s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 37s There were no new javac warning messages.
          +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings.
          -1 whitespace 0m 7s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 install 1m 19s mvn install still works.
          +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
          -1 yarn tests 6m 3s Tests failed in hadoop-yarn-server-nodemanager.
              21m 22s  



          Reason Tests
          Failed unit tests hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService
            hadoop.yarn.server.nodemanager.TestDeletionService



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12746124/YARN-3852-1.patch
          Optional Tests javac unit
          git revision trunk / 98c2bc8
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/8586/artifact/patchprocess/whitespace.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8586/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8586/testReport/
          Java 1.7.0_55
          uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/8586/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 5m 21s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 37s There were no new javac warning messages. +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings. -1 whitespace 0m 7s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 19s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. -1 yarn tests 6m 3s Tests failed in hadoop-yarn-server-nodemanager.     21m 22s   Reason Tests Failed unit tests hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService   hadoop.yarn.server.nodemanager.TestDeletionService Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12746124/YARN-3852-1.patch Optional Tests javac unit git revision trunk / 98c2bc8 whitespace https://builds.apache.org/job/PreCommit-YARN-Build/8586/artifact/patchprocess/whitespace.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8586/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8586/testReport/ Java 1.7.0_55 uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-YARN-Build/8586/console This message was automatically generated.
          Hide
          ashahab Abin Shahab added a comment -

          Code review fixes and refactoring

          Show
          ashahab Abin Shahab added a comment - Code review fixes and refactoring
          Hide
          vvasudev Varun Vasudev added a comment -

          Thanks for the patch Abin Shahab. The patch isn't working for me. There are two issues -

          1. No default value for "docker.binary". I think we should assume this to be "docker" and allow it to be overriden.
          2. The docker launch fails due to
            if (change_effective_user(user_uid, user_gid) != 0)
            

            in launch_docker_container_as_user. For docker run to work, the effective user needs to be root(something like change_effective_user(0, user_gid) is probably the right way).

          Some other issues -

          1. -static const char* DEFAULT_BANNED_USERS[] = {"yarn", "mapred", "hdfs", "bin", 0};
            +static const char* DEFAULT_BANNED_USERS[] = {"mapred", "hdfs", "bin", 0};
            

            Why are you removing the yarn user from the banned users? I'm guessing this is due to a branch-2/trunk issue. The yarn user is banned in trunk but not in branch-2

          2. A couple of formatting fixes
            +   fprintf(LOGFILE, "done opening pid\n");
            +        fflush(LOGFILE);
            

            and

            +    fprintf(LOGFILE, "done writing pid to tmp\n");
            +         fflush(LOGFILE);
            
          3. Can we change the error message in the message below to a more descriptive one?
            +     fprintf(ERRORFILE, "Error reading\n");
            +     fflush(ERRORFILE);
            
          4. In parse_docker_command_file
            +  int read;
            

            should we use ssize_t instead or int?

          5. In parse_docker_command_file, we have some exit(1) calls - can we change this to use the error codes in container-executor.h?
          6. In run_docker
            +      free(docker_binary);
            +      free(args);
            +      free(docker_command_with_binary);
            +      free(docker_command);
            +      exit_code = DOCKER_RUN_FAILED;
            +  }
            +  exit_code = 0;
            +  return exit_code;
            

            The exit code from the function will always be 0

          7. Formatting
            +int create_script_paths(const char *work_dir,
            +                      const char *script_name, const char *cred_file,
            +                 char** script_file_dest, char** cred_file_dest,
            +                 int* container_file_source, int* cred_file_source ) {
            
          8. In create_script_paths, we use a bunch of goto's but the goto target doesn't have any special logic or handling. Can we avoid using the goto?
          9. +    //kill me now.
            

            No need for the commentary

          10. In main.c
            +    char * resources = argv[optind++];// key,value pair describing resources
            +    char * resources_key = malloc(strlen(resources));
            +    char * resources_value = malloc(strlen(resources));
            

          Can we move the declarations of resources, resources_key and resources_value out of the case block(since the same variables are used in two case blocks)?

          Show
          vvasudev Varun Vasudev added a comment - Thanks for the patch Abin Shahab . The patch isn't working for me. There are two issues - No default value for "docker.binary". I think we should assume this to be "docker" and allow it to be overriden. The docker launch fails due to if (change_effective_user(user_uid, user_gid) != 0) in launch_docker_container_as_user. For docker run to work, the effective user needs to be root(something like change_effective_user(0, user_gid) is probably the right way). Some other issues - - static const char * DEFAULT_BANNED_USERS[] = { "yarn" , "mapred" , "hdfs" , "bin" , 0}; + static const char * DEFAULT_BANNED_USERS[] = { "mapred" , "hdfs" , "bin" , 0}; Why are you removing the yarn user from the banned users? I'm guessing this is due to a branch-2/trunk issue. The yarn user is banned in trunk but not in branch-2 A couple of formatting fixes + fprintf(LOGFILE, "done opening pid\n" ); + fflush(LOGFILE); and + fprintf(LOGFILE, "done writing pid to tmp\n" ); + fflush(LOGFILE); Can we change the error message in the message below to a more descriptive one? + fprintf(ERRORFILE, "Error reading\n" ); + fflush(ERRORFILE); In parse_docker_command_file + int read; should we use ssize_t instead or int? In parse_docker_command_file, we have some exit(1) calls - can we change this to use the error codes in container-executor.h? In run_docker + free(docker_binary); + free(args); + free(docker_command_with_binary); + free(docker_command); + exit_code = DOCKER_RUN_FAILED; + } + exit_code = 0; + return exit_code; The exit code from the function will always be 0 Formatting + int create_script_paths( const char *work_dir, + const char *script_name, const char *cred_file, + char ** script_file_dest, char ** cred_file_dest, + int * container_file_source, int * cred_file_source ) { In create_script_paths, we use a bunch of goto's but the goto target doesn't have any special logic or handling. Can we avoid using the goto? + //kill me now. No need for the commentary In main.c + char * resources = argv[optind++]; // key,value pair describing resources + char * resources_key = malloc(strlen(resources)); + char * resources_value = malloc(strlen(resources)); Can we move the declarations of resources, resources_key and resources_value out of the case block(since the same variables are used in two case blocks)?
          Hide
          sidharta-s Sidharta Seethana added a comment -

          One of the test failures ( TestPrivilegedOperationExecutor ) is unrelated to this patch. Please see the update to YARN-2194

          Show
          sidharta-s Sidharta Seethana added a comment - One of the test failures ( TestPrivilegedOperationExecutor ) is unrelated to this patch. Please see the update to YARN-2194
          Hide
          ashahab Abin Shahab added a comment -

          Varun Vasudev I'm looking at the test failures. However, can you review the patch?

          Show
          ashahab Abin Shahab added a comment - Varun Vasudev I'm looking at the test failures. However, can you review the patch?
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 5m 32s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 39s There were no new javac warning messages.
          +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings.
          -1 whitespace 0m 4s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 install 1m 19s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          -1 yarn tests 5m 59s Tests failed in hadoop-yarn-server-nodemanager.
              21m 31s  



          Reason Tests
          Failed unit tests hadoop.yarn.server.nodemanager.TestDeletionService
            hadoop.yarn.server.nodemanager.containermanager.linux.privileged.TestPrivilegedOperationExecutor



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12744358/YARN-3852.patch
          Optional Tests javac unit
          git revision trunk / 2e3d83f
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/8463/artifact/patchprocess/whitespace.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8463/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8463/testReport/
          Java 1.7.0_55
          uname Linux asf905.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/8463/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 5m 32s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 39s There were no new javac warning messages. +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings. -1 whitespace 0m 4s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 19s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. -1 yarn tests 5m 59s Tests failed in hadoop-yarn-server-nodemanager.     21m 31s   Reason Tests Failed unit tests hadoop.yarn.server.nodemanager.TestDeletionService   hadoop.yarn.server.nodemanager.containermanager.linux.privileged.TestPrivilegedOperationExecutor Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12744358/YARN-3852.patch Optional Tests javac unit git revision trunk / 2e3d83f whitespace https://builds.apache.org/job/PreCommit-YARN-Build/8463/artifact/patchprocess/whitespace.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8463/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8463/testReport/ Java 1.7.0_55 uname Linux asf905.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-YARN-Build/8463/console This message was automatically generated.
          Hide
          ashahab Abin Shahab added a comment -

          Changes to container executor for running docker from LCE

          Show
          ashahab Abin Shahab added a comment - Changes to container executor for running docker from LCE

            People

            • Assignee:
              ashahab Abin Shahab
              Reporter:
              sidharta-s Sidharta Seethana
            • Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development