Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-3611 Support Docker Containers In LinuxContainerExecutor
  3. YARN-7626

Allow regular expression matching in container-executor.cfg for devices and named docker volumes mount

Details

    • Sub-task
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • 3.1.0
    • None

    Description

      Currently when we config some of the GPU devices related fields (like ) in container-executor.cfg, these fields are generated based on different driver versions or GPU device names. We want to enable regular expression matching so that user don't need to manually set up these fields when config container-executor.cfg,

      Attachments

        1. YARN-7626.001.patch
          13 kB
          Zian Chen
        2. YARN-7626.002.patch
          12 kB
          Zian Chen
        3. YARN-7626.003.patch
          12 kB
          Zian Chen
        4. YARN-7626.004.patch
          12 kB
          Zian Chen
        5. YARN-7626.005.patch
          12 kB
          Zian Chen
        6. YARN-7626.006.patch
          12 kB
          Zian Chen
        7. YARN-7626.007.patch
          15 kB
          Zian Chen
        8. YARN-7626.008.patch
          13 kB
          Zian Chen
        9. YARN-7626.009.patch
          13 kB
          Zian Chen
        10. YARN-7626.010.patch
          13 kB
          Zian Chen
        11. YARN-7626.011.patch
          14 kB
          Zian Chen

        Activity

          Zian Chen Zian Chen added a comment -

          Submit the first patch for Jenkins test.

          Zian Chen Zian Chen added a comment - Submit the first patch for Jenkins test.
          genericqa genericqa added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 16m 23s trunk passed
          +1 compile 0m 48s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 shadedclient 26m 47s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 28s the patch passed
          +1 compile 0m 42s the patch passed
          +1 cc 0m 42s the patch passed
          +1 javac 0m 42s the patch passed
          +1 mvnsite 0m 28s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 10m 19s patch has no errors when building and testing our client artifacts.
                Other Tests
          -1 unit 19m 35s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          59m 18s



          Reason Tests
          Failed junit tests TEST-cetest



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12906484/YARN-7626.001.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux 1223163621b1 4.4.0-64-generic #85-Ubuntu SMP Mon Feb 20 11:50:30 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6e42d05
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          unit https://builds.apache.org/job/PreCommit-YARN-Build/19301/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19301/testReport/
          Max. process+thread count 409 (vs. ulimit of 5000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19301/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 16m 23s trunk passed +1 compile 0m 48s trunk passed +1 mvnsite 0m 30s trunk passed +1 shadedclient 26m 47s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 28s the patch passed +1 compile 0m 42s the patch passed +1 cc 0m 42s the patch passed +1 javac 0m 42s the patch passed +1 mvnsite 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 10m 19s patch has no errors when building and testing our client artifacts.       Other Tests -1 unit 19m 35s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 59m 18s Reason Tests Failed junit tests TEST-cetest Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12906484/YARN-7626.001.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 1223163621b1 4.4.0-64-generic #85-Ubuntu SMP Mon Feb 20 11:50:30 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 6e42d05 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 unit https://builds.apache.org/job/PreCommit-YARN-Build/19301/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19301/testReport/ Max. process+thread count 409 (vs. ulimit of 5000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19301/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          genericqa genericqa added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 16m 32s trunk passed
          +1 compile 0m 51s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 shadedclient 27m 49s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 32s the patch passed
          +1 compile 0m 46s the patch passed
          +1 cc 0m 46s the patch passed
          +1 javac 0m 46s the patch passed
          +1 mvnsite 0m 29s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 11m 8s patch has no errors when building and testing our client artifacts.
                Other Tests
          -1 unit 19m 10s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          60m 51s



          Reason Tests
          Failed junit tests TEST-cetest



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907245/YARN-7626.002.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux 095e05e3611c 3.13.0-135-generic #184-Ubuntu SMP Wed Oct 18 11:55:51 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6347b22
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          unit https://builds.apache.org/job/PreCommit-YARN-Build/19390/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19390/testReport/
          Max. process+thread count 306 (vs. ulimit of 5000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19390/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 16m 32s trunk passed +1 compile 0m 51s trunk passed +1 mvnsite 0m 34s trunk passed +1 shadedclient 27m 49s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 32s the patch passed +1 compile 0m 46s the patch passed +1 cc 0m 46s the patch passed +1 javac 0m 46s the patch passed +1 mvnsite 0m 29s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 11m 8s patch has no errors when building and testing our client artifacts.       Other Tests -1 unit 19m 10s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 60m 51s Reason Tests Failed junit tests TEST-cetest Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907245/YARN-7626.002.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 095e05e3611c 3.13.0-135-generic #184-Ubuntu SMP Wed Oct 18 11:55:51 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 6347b22 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 unit https://builds.apache.org/job/PreCommit-YARN-Build/19390/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19390/testReport/ Max. process+thread count 306 (vs. ulimit of 5000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19390/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          genericqa genericqa added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 17m 16s trunk passed
          +1 compile 0m 50s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 shadedclient 28m 29s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 40s the patch passed
          +1 compile 0m 56s the patch passed
          +1 cc 0m 56s the patch passed
          +1 javac 0m 56s the patch passed
          +1 mvnsite 0m 35s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 12m 0s patch has no errors when building and testing our client artifacts.
                Other Tests
          -1 unit 18m 55s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          62m 30s



          Reason Tests
          Failed junit tests TEST-cetest



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907366/YARN-7626.003.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux e8775e29b5d8 3.13.0-133-generic #182-Ubuntu SMP Tue Sep 19 15:49:21 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / a72cdcc
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          unit https://builds.apache.org/job/PreCommit-YARN-Build/19409/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19409/testReport/
          Max. process+thread count 379 (vs. ulimit of 5000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19409/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 17m 16s trunk passed +1 compile 0m 50s trunk passed +1 mvnsite 0m 34s trunk passed +1 shadedclient 28m 29s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 40s the patch passed +1 compile 0m 56s the patch passed +1 cc 0m 56s the patch passed +1 javac 0m 56s the patch passed +1 mvnsite 0m 35s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 12m 0s patch has no errors when building and testing our client artifacts.       Other Tests -1 unit 18m 55s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 62m 30s Reason Tests Failed junit tests TEST-cetest Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907366/YARN-7626.003.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux e8775e29b5d8 3.13.0-133-generic #182-Ubuntu SMP Tue Sep 19 15:49:21 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / a72cdcc maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 unit https://builds.apache.org/job/PreCommit-YARN-Build/19409/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19409/testReport/ Max. process+thread count 379 (vs. ulimit of 5000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19409/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Zian Chen Zian Chen added a comment -

          Update patch 004 to fix failed cases

          Zian Chen Zian Chen added a comment - Update patch 004 to fix failed cases
          genericqa genericqa added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 17m 40s trunk passed
          +1 compile 0m 51s trunk passed
          +1 mvnsite 0m 35s trunk passed
          +1 shadedclient 29m 18s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 42s the patch passed
          +1 compile 1m 1s the patch passed
          +1 cc 1m 1s the patch passed
          +1 javac 1m 1s the patch passed
          +1 mvnsite 0m 39s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 12m 8s patch has no errors when building and testing our client artifacts.
                Other Tests
          -1 unit 18m 56s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 31s The patch does not generate ASF License warnings.
          63m 48s



          Reason Tests
          Failed junit tests hadoop.yarn.server.nodemanager.containermanager.linux.runtime.TestDockerContainerRuntime



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907587/YARN-7626.004.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux 0a0f9ed7bed6 3.13.0-133-generic #182-Ubuntu SMP Tue Sep 19 15:49:21 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0c559b2
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          unit https://builds.apache.org/job/PreCommit-YARN-Build/19443/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19443/testReport/
          Max. process+thread count 329 (vs. ulimit of 5000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19443/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 17m 40s trunk passed +1 compile 0m 51s trunk passed +1 mvnsite 0m 35s trunk passed +1 shadedclient 29m 18s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 42s the patch passed +1 compile 1m 1s the patch passed +1 cc 1m 1s the patch passed +1 javac 1m 1s the patch passed +1 mvnsite 0m 39s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 12m 8s patch has no errors when building and testing our client artifacts.       Other Tests -1 unit 18m 56s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 31s The patch does not generate ASF License warnings. 63m 48s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.linux.runtime.TestDockerContainerRuntime Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907587/YARN-7626.004.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 0a0f9ed7bed6 3.13.0-133-generic #182-Ubuntu SMP Tue Sep 19 15:49:21 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 0c559b2 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 unit https://builds.apache.org/job/PreCommit-YARN-Build/19443/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19443/testReport/ Max. process+thread count 329 (vs. ulimit of 5000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19443/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Zian Chen Zian Chen added a comment - - edited

          Tested patch 005 on cluster with GPU. Looks good. Upload the patch. 

          Zian Chen Zian Chen added a comment - - edited Tested patch 005 on cluster with GPU. Looks good. Upload the patch. 
          genericqa genericqa added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 23m 19s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 15m 52s trunk passed
          +1 compile 0m 51s trunk passed
          +1 mvnsite 0m 33s trunk passed
          +1 shadedclient 26m 26s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 46s the patch passed
          +1 cc 0m 46s the patch passed
          +1 javac 0m 46s the patch passed
          +1 mvnsite 0m 29s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 10m 11s patch has no errors when building and testing our client artifacts.
                Other Tests
          -1 unit 20m 2s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          82m 25s



          Reason Tests
          Failed junit tests hadoop.yarn.server.nodemanager.containermanager.TestContainerManager



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907780/YARN-7626.005.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux 27841858be2a 4.4.0-89-generic #112-Ubuntu SMP Mon Jul 31 19:38:41 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 16be42d
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          unit https://builds.apache.org/job/PreCommit-YARN-Build/19479/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19479/testReport/
          Max. process+thread count 408 (vs. ulimit of 5000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19479/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 23m 19s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 15m 52s trunk passed +1 compile 0m 51s trunk passed +1 mvnsite 0m 33s trunk passed +1 shadedclient 26m 26s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 30s the patch passed +1 compile 0m 46s the patch passed +1 cc 0m 46s the patch passed +1 javac 0m 46s the patch passed +1 mvnsite 0m 29s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 10m 11s patch has no errors when building and testing our client artifacts.       Other Tests -1 unit 20m 2s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 82m 25s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.TestContainerManager Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12907780/YARN-7626.005.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 27841858be2a 4.4.0-89-generic #112-Ubuntu SMP Mon Jul 31 19:38:41 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 16be42d maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 unit https://builds.apache.org/job/PreCommit-YARN-Build/19479/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19479/testReport/ Max. process+thread count 408 (vs. ulimit of 5000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19479/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          leftnoteasy Wangda Tan added a comment -

          Thanks Zian Chen for working on this issue and testing. I just updated the title a bit to better reflect what the patch does:

          ebadger/miklos.szegedi@cloudera.com/shanekumpf@gmail.com, could you help to review the patch?

          leftnoteasy Wangda Tan added a comment - Thanks Zian Chen for working on this issue and testing. I just updated the title a bit to better reflect what the patch does: ebadger / miklos.szegedi@cloudera.com / shanekumpf@gmail.com , could you help to review the patch?
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Zian Chen, thank you for the patch.

          76	static int is_regex(const char *str) {
          77	const char *regex_str = "^[\\^].*[\\$]$";
          78	return execute_regex_match(regex_str, str);
          79	}

          You could just do a simple size_t len=strlen(str); return !(len>2 && str[0]=='^' && str[len-1]=='$'); It is probably more efficient than compiling the regex, etc.

          // Iterate each permitted values.

          There is a missing 'through' here in the iterate through each value message.

          137	            if (is_regex(permitted_values[j]) == 0) {
          138	              ret = validate_volume_name_with_argument(values[i], permitted_values[j]);
          139	            }
          

          I would put ret = strncmp(values[i], permitted_values[j], tmp_ptr - values[i]); into the else of this block.

          // if it's a valid REGEX return; for user mount, we need to strictly check
          

          Is not there a contradiction with the code? 853 if (validate_volume_name(mount) == 0) {

          925	    // if (permitted_mounts[i] is a REGEX): use REGEX to compare; return
          926	    if (is_regex(permitted_mounts[i]) == 0 &&
          927	    validate_volume_name_with_argument(normalized_path, permitted_mounts[i]) == 0) {
          928	      ret = 1;
          929	      break;
          930	    }
          

          Similarly, is not there a contradiction between the code and the comment? If the comment is right, this check should be before if (strcmp(normalized_path, permitted_mounts[i]) == 0) { and break in case of the regex, regardless of match or not.

          979	    ret = normalize_mounts(permitted_ro_mounts, -1);
          

          If isUserMount is a boolean, I would use 0 or 1. -1 might be misleading to some folks.

          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Zian Chen , thank you for the patch. 76 static int is_regex( const char *str) { 77 const char *regex_str = "^[\\^].*[\\$]$" ; 78 return execute_regex_match(regex_str, str); 79 } You could just do a simple size_t len=strlen(str); return !(len>2 && str [0] =='^' && str [len-1] =='$'); It is probably more efficient than compiling the regex, etc. // Iterate each permitted values. There is a missing 'through' here in the iterate through each value message. 137 if (is_regex(permitted_values[j]) == 0) { 138 ret = validate_volume_name_with_argument(values[i], permitted_values[j]); 139 } I would put ret = strncmp(values [i] , permitted_values [j] , tmp_ptr - values [i] ); into the else of this block. // if it's a valid REGEX return ; for user mount, we need to strictly check Is not there a contradiction with the code? 853 if (validate_volume_name(mount) == 0) { 925 // if (permitted_mounts[i] is a REGEX): use REGEX to compare; return 926 if (is_regex(permitted_mounts[i]) == 0 && 927 validate_volume_name_with_argument(normalized_path, permitted_mounts[i]) == 0) { 928 ret = 1; 929 break ; 930 } Similarly, is not there a contradiction between the code and the comment? If the comment is right, this check should be before if (strcmp(normalized_path, permitted_mounts [i] ) == 0) { and break in case of the regex, regardless of match or not. 979 ret = normalize_mounts(permitted_ro_mounts, -1); If isUserMount is a boolean, I would use 0 or 1. -1 might be misleading to some folks.
          Zian Chen Zian Chen added a comment - - edited

          Hi miklos.szegedi@cloudera.com  , thank you so much for your comments, for is_regex function, missing "through" for iteration and isUserMount boolean value change, I'm totally agreed with your suggestions. I'll update the patch according to your comments.

          For those two contradictions, I would like to explain more in details, line 853 and line 925-930 are inside function check_mount_permitted, this function accepts two params as input, permitted_mounts are mounts which are specified by admin which can be permitted for checking the user mounts, and requested are user mounts. in line

          char *normalized_path = normalize_mount(requested, 0);

          we call function normalize_mount which will lead to line 853, in this function we use "isUsermount" to distinguish the input mount is user mount or permitted mount, here, in this case, it's user mount, then this part of the code will not be executed for user mount,

          // we only allow permitted mount to be REGEX, for permitted mount, we check
          // if it's a valid REGEX return; for user mount, we need to strictly check
          if (isUserMount != 0) {
            if (is_regex(mount) == 0) {
              return strdup(mount);
            }
          }
          

          we will follow the original logic which checks if the user mount is a real path, if not, verify if it's a validate volume name, then do proper actions and return normalized_path for user mount.

          Then line 925-930 is used to check if permitted mounts are a regular expression. If it is, we need to use regex matching to see if current permitted_mounts can be matched with normalized_path of the user mount, that's what line 925-930 trying to do. Let me give you an example for this,

          for example one of the permitted_mounts is "/usr/local/.$" which is a regex and the user mount is "//usr/local/bin/", when we call function check_mount_permitted, we will call function normalize_mount for user mount "//usr/local/bin/", check if it's a real path, it is, then the rest of code will normalize the user mount into "/usr/local/bin/" then return as normalized_path, then we go back to function check_mount_permitted and loop permitted_mounts, no other can match "/usr/local/bin/" except regex "/usr/local/.$", and this is because we use line 925-930 check permitted_mounts is a regex and it can match "/usr/local/bin/" using regex matching.

          However, function does not always get user mount as input, like when we call function normalize_mounts in these two lines inside function add_mounts,

          ret = normalize_mounts(permitted_ro_mounts, 1);
          ret |= normalize_mounts(permitted_rw_mounts, 1);

          normalize_mount will get permitted_mounts which let the code logic enter into this part,

          // we only allow permitted mount to be REGEX, for permitted mount, we check
          // if it's a valid REGEX return; for user mount, we need to strictly check
          if (isUserMount != 0) {
            if (is_regex(mount) == 0) {
              return strdup(mount);
            }
          }

          in this situation, we only accept permitted_mounts as regex when it's not a real path, otherwise, we don't allow it as a valid permitted_mount.

          For the second contradiction, "this check should be before if (strcmp(normalized_path, permitted_mounts[i]) == 0)", the order of the first check and second check is not matter, because inside permitted_mounts array, we have regex format permitted_mount as well as non-regex ones, we try to use every possible permitted_mouunt to see if we could match current normalized_path, no matter the permitted_mount is regex or non-regex, if it's non-regex, we use strcmp to match, if it's regex, we use function validate_volume_name_with_argument to match.

          Hope my explanation helps with the understanding of this part of the code, further comments are highly welcomed. leftnoteasy What's your opinion

           

           

           

          Zian Chen Zian Chen added a comment - - edited Hi miklos.szegedi@cloudera.com  , thank you so much for your comments, for is_regex function, missing "through" for iteration and isUserMount boolean value change, I'm totally agreed with your suggestions. I'll update the patch according to your comments. For those two contradictions, I would like to explain more in details, line 853 and line 925-930 are inside function check_mount_permitted, this function accepts two params as input, permitted_mounts are mounts which are specified by admin which can be permitted for checking the user mounts, and requested are user mounts. in line char *normalized_path = normalize_mount(requested, 0); we call function normalize_mount which will lead to line 853, in this function we use "isUsermount" to distinguish the input mount is user mount or permitted mount, here, in this case, it's user mount, then this part of the code will not be executed for user mount, // we only allow permitted mount to be REGEX, for permitted mount, we check // if it's a valid REGEX return ; for user mount, we need to strictly check if (isUserMount != 0) { if (is_regex(mount) == 0) { return strdup(mount); } } we will follow the original logic which checks if the user mount is a real path, if not, verify if it's a validate volume name, then do proper actions and return normalized_path for user mount. Then line 925-930 is used to check if permitted mounts are a regular expression. If it is, we need to use regex matching to see if current permitted_mounts can be matched with normalized_path of the user mount, that's what line 925-930 trying to do. Let me give you an example for this, for example one of the permitted_mounts is "/usr/local/.$" which is a regex and the user mount is "//usr/local/bin/", when we call function check_mount_permitted, we will call function normalize_mount for user mount "//usr/local/bin/", check if it's a real path, it is, then the rest of code will normalize the user mount into "/usr/local/bin/" then return as normalized_path, then we go back to function check_mount_permitted and loop permitted_mounts, no other can match "/usr/local/bin/" except regex "/usr/local/.$", and this is because we use line 925-930 check permitted_mounts is a regex and it can match "/usr/local/bin/" using regex matching. However, function does not always get user mount as input, like when we call function normalize_mounts in these two lines inside function add_mounts, ret = normalize_mounts(permitted_ro_mounts, 1); ret |= normalize_mounts(permitted_rw_mounts, 1); normalize_mount will get permitted_mounts which let the code logic enter into this part, // we only allow permitted mount to be REGEX, for permitted mount, we check // if it's a valid REGEX return ; for user mount, we need to strictly check if (isUserMount != 0) { if (is_regex(mount) == 0) { return strdup(mount); } } in this situation, we only accept permitted_mounts as regex when it's not a real path, otherwise, we don't allow it as a valid permitted_mount. For the second contradiction, "this check should be before if (strcmp(normalized_path, permitted_mounts [i] ) == 0)", the order of the first check and second check is not matter, because inside permitted_mounts array, we have regex format permitted_mount as well as non-regex ones, we try to use every possible permitted_mouunt to see if we could match current normalized_path, no matter the permitted_mount is regex or non-regex, if it's non-regex, we use strcmp to match, if it's regex, we use function validate_volume_name_with_argument to match. Hope my explanation helps with the understanding of this part of the code, further comments are highly welcomed. leftnoteasy What's your opinion      
          Zian Chen Zian Chen added a comment -

          Update patch 006 per Miklos's suggestions. leftnoteasy ,sunilg , could you please help review the latest patch? Thanks!

          Zian Chen Zian Chen added a comment - Update patch 006 per Miklos's suggestions. leftnoteasy , sunilg , could you please help review the latest patch? Thanks!
          genericqa genericqa added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 9m 42s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 17m 37s trunk passed
          +1 compile 0m 50s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 shadedclient 29m 0s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 32s the patch passed
          +1 compile 0m 47s the patch passed
          +1 cc 0m 47s the patch passed
          +1 javac 0m 47s the patch passed
          +1 mvnsite 0m 30s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 11m 22s patch has no errors when building and testing our client artifacts.
                Other Tests
          +1 unit 18m 19s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          70m 57s



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12909040/YARN-7626.006.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux 375642c5c406 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5072388
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19586/testReport/
          Max. process+thread count 380 (vs. ulimit of 5500)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19586/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 9m 42s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 17m 37s trunk passed +1 compile 0m 50s trunk passed +1 mvnsite 0m 34s trunk passed +1 shadedclient 29m 0s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 32s the patch passed +1 compile 0m 47s the patch passed +1 cc 0m 47s the patch passed +1 javac 0m 47s the patch passed +1 mvnsite 0m 30s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 11m 22s patch has no errors when building and testing our client artifacts.       Other Tests +1 unit 18m 19s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 70m 57s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12909040/YARN-7626.006.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 375642c5c406 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 5072388 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19586/testReport/ Max. process+thread count 380 (vs. ulimit of 5500) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19586/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Zian Chen Zian Chen added a comment -

          For the latest patch, I tested the regex expression matching on a GPU cluster for several feature requirements.,

          1. test if regex format permitted mounts can be normalized or not 
          2. test if user input devices can be matched using permitted regex devices
          3. test if user input ro_mounts can be matched using permitted regex mounts 

          All these test scenarios look good. 

           

          Zian Chen Zian Chen added a comment - For the latest patch, I tested the regex expression matching on a GPU cluster for several feature requirements., test if regex format permitted mounts can be normalized or not  test if user input devices can be matched using permitted regex devices test if user input ro_mounts can be matched using permitted regex mounts  All these test scenarios look good.   
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thank you, Zian Chen for the reply and update. Your latest patch does not seem to apply to trunk. Could you verify and rebase?

          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thank you, Zian Chen for the reply and update. Your latest patch does not seem to apply to trunk. Could you verify and rebase?
          genericqa genericqa added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 4s YARN-7626 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          This message was automatically generated.

          genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 4s YARN-7626 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12910313/YARN-7626.007.patch Console output https://builds.apache.org/job/PreCommit-YARN-Build/19667/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Zian Chen Zian Chen added a comment -

          miklos.szegedi@cloudera.com , Thanks Miklos for the comments. Submitted the rebased patch.

          Zian Chen Zian Chen added a comment - miklos.szegedi@cloudera.com , Thanks Miklos for the comments. Submitted the rebased patch.
          genericqa genericqa added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 18m 26s trunk passed
          +1 compile 0m 52s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 shadedclient 30m 13s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 34s the patch passed
          +1 compile 0m 51s the patch passed
          +1 cc 0m 51s the patch passed
          +1 javac 0m 51s the patch passed
          +1 mvnsite 0m 31s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 11m 39s patch has no errors when building and testing our client artifacts.
                Other Tests
          +1 unit 18m 34s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 25s The patch does not generate ASF License warnings.
          63m 30s



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12910347/YARN-7626.008.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux 9b042fad9c96 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0c5d7d7
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19671/testReport/
          Max. process+thread count 341 (vs. ulimit of 5500)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19671/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 18m 26s trunk passed +1 compile 0m 52s trunk passed +1 mvnsite 0m 34s trunk passed +1 shadedclient 30m 13s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 34s the patch passed +1 compile 0m 51s the patch passed +1 cc 0m 51s the patch passed +1 javac 0m 51s the patch passed +1 mvnsite 0m 31s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 11m 39s patch has no errors when building and testing our client artifacts.       Other Tests +1 unit 18m 34s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 63m 30s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12910347/YARN-7626.008.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 9b042fad9c96 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 0c5d7d7 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19671/testReport/ Max. process+thread count 341 (vs. ulimit of 5500) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19671/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          leftnoteasy Wangda Tan added a comment -

          miklos.szegedi@cloudera.com, could you help to check the latest patch?

          leftnoteasy Wangda Tan added a comment - miklos.szegedi@cloudera.com , could you help to check the latest patch?
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thank you for the patch Zian Chen.

          return !(len > 2 && str[0] == '^' && str[len-1] == '$');

          Optional: I think this is still misleading. is_regex should return one on success and 0 on failure.

          // Iterate each permitted values.

          Optional: I think it would be better to write 'Iterate through each permitted value'

          '/dev/nvidia1:/dev/nvidia1'
                    if (prefix == 0) {
                      ret = strcmp(values[i], permitted_values[j]);
                    } else {
                      // If permitted-Values[j] is a REGEX, use REGEX to compare
                      if (is_regex(permitted_values[j]) == 0) {
                        ret = validate_volume_name_with_argument(values[i], permitted_values[j]);
                      } else {
                        ret = strncmp(values[i], permitted_values[j], tmp_ptr - values[i]);
                      }
                    }
          

          Technically the code, where prefix is not null including the regex match, should check only the characters before the prefix :. It is checking now the whole value[i], you should apply the regex only to [values[i] ... tmp_ptr].

          /**
           * Helper function to help normalize mounts for checking if mounts are
           * permitted. The function does the following -
           * 1. Find the canonical path for mount using realpath
           * 2. If the path is a directory, add a '/' at the end (if not present)
           * 3. Return a copy of the canonicalised path(to be freed by the caller)
           * @param mount path to be canonicalised
           * @return pointer to canonicalised path, NULL on error
           */
          static char* normalize_mount(const char* mount, int isUserMount) {
          

          There is no @param documentation for isUserMount, in fact I would name it isRegexAllowed to avoid confusion.

          const char *container_executor_cfg_path = normalize_mount(get_config_path(""), 1);

          I do not understand why the config path could be a regex.

          tmp_path_buffer[0] = normalize_mount(mount_src, 1);

          Should not this be 0, too?

          I have a few conceptual issues with the latest patch.

          1. First of all, normalize_mounts, walks through the permitted mounts and it resolves symlinks but it does not resolve symlinks, if isUserMount (isRegex) is 1. What if the regex resolves to a symlink? I think it would probably be more future proof, if normalize_mounts applied the regex to the directory tree and then called the original normalize_mount on the resulting file names, that returns the real path for each. This would eliminate the need for passing isUserMount all the way through the call structure. It would also help to avoid issues that appear with invalid regexes, etc.
          2. Technically a regex without the ^$ pair is a valid regex. It would be more precise and future proof to mark regexes with a prefix like regex:/dev/device[0-9]+. In this case we would not need to use just a subset for matching.
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thank you for the patch Zian Chen . return !(len > 2 && str[0] == '^' && str[len-1] == '$' ); Optional: I think this is still misleading. is_regex should return one on success and 0 on failure. // Iterate each permitted values. Optional: I think it would be better to write 'Iterate through each permitted value' '/dev/nvidia1:/dev/nvidia1' if (prefix == 0) { ret = strcmp(values[i], permitted_values[j]); } else { // If permitted-Values[j] is a REGEX, use REGEX to compare if (is_regex(permitted_values[j]) == 0) { ret = validate_volume_name_with_argument(values[i], permitted_values[j]); } else { ret = strncmp(values[i], permitted_values[j], tmp_ptr - values[i]); } } Technically the code, where prefix is not null including the regex match, should check only the characters before the prefix :. It is checking now the whole value [i] , you should apply the regex only to [values [i] ... tmp_ptr]. /** * Helper function to help normalize mounts for checking if mounts are * permitted. The function does the following - * 1. Find the canonical path for mount using realpath * 2. If the path is a directory, add a '/' at the end ( if not present) * 3. Return a copy of the canonicalised path(to be freed by the caller) * @param mount path to be canonicalised * @ return pointer to canonicalised path, NULL on error */ static char * normalize_mount( const char * mount, int isUserMount) { There is no @param documentation for isUserMount, in fact I would name it isRegexAllowed to avoid confusion. const char *container_executor_cfg_path = normalize_mount(get_config_path(""), 1); I do not understand why the config path could be a regex. tmp_path_buffer[0] = normalize_mount(mount_src, 1); Should not this be 0, too? I have a few conceptual issues with the latest patch. First of all, normalize_mounts, walks through the permitted mounts and it resolves symlinks but it does not resolve symlinks, if isUserMount (isRegex) is 1. What if the regex resolves to a symlink? I think it would probably be more future proof, if normalize_mounts applied the regex to the directory tree and then called the original normalize_mount on the resulting file names, that returns the real path for each. This would eliminate the need for passing isUserMount all the way through the call structure. It would also help to avoid issues that appear with invalid regexes, etc. Technically a regex without the ^$ pair is a valid regex. It would be more precise and future proof to mark regexes with a prefix like  regex:/dev/device [0-9] + . In this case we would not need to use just a subset for matching.
          Zian Chen Zian Chen added a comment -

          Hi miklos.szegedi@cloudera.com , really appreciate your comments for the latest patch. I agree with most of the suggestions. Just have two quick questions here for the conceptual issues No. 2,

          1. Since current trunk code base uses ^$ pair as the judgment criteria for regex, could you kindly give us an example that there is any other type of regex that is not like ^$ pair which is useful to us in docker volume mount scenario?
          2. I'm also concerned using regex: as a prefix instead of ^$ pair for regex may introduce security holes, what if hackers input user mount like regex+ as a prefix?  

          Could you please advice? Thank you so much!

           

           

          Zian Chen Zian Chen added a comment - Hi miklos.szegedi@cloudera.com , really appreciate your comments for the latest patch. I agree with most of the suggestions. Just have two quick questions here for the conceptual issues No. 2, Since current trunk code base uses ^$ pair as the judgment criteria for regex, could you kindly give us an example that there is any other type of regex that is not like ^$ pair which is useful to us in docker volume mount scenario? I'm also concerned using regex: as a prefix instead of ^$ pair for regex may introduce security holes, what if hackers input user mount like regex+ as a prefix?   Could you please advice? Thank you so much!    
          Zian Chen Zian Chen added a comment -

          Hi leftnoteasy , could you also share your thoughts on this? Thanks!

          Zian Chen Zian Chen added a comment - Hi leftnoteasy , could you also share your thoughts on this? Thanks!
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Indeed there is not there any substantial difference other than saying regex:/dev/nvidia.*. I think this latter is a bit more robust in case we try to configure regexes for other purposes in the future. This is just an opinion I let you decide.

          what if hackers input user mount like regex+ as a prefix?

          Regex+ won't be considered valid. What if they put ^.*$? I do not think there is a difference at least not from this point of view. But you just raised an important point. The regex has to be properly validated.

          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Indeed there is not there any substantial difference other than saying regex:/dev/nvidia.* . I think this latter is a bit more robust in case we try to configure regexes for other purposes in the future. This is just an opinion I let you decide. what if hackers input user mount like regex+ as a prefix? Regex+ won't be considered valid. What if they put ^.*$? I do not think there is a difference at least not from this point of view. But you just raised an important point. The regex has to be properly validated.
          Zian Chen Zian Chen added a comment -

          I'm changing the patch according to the comments and find some memory leak over there, I'm working on it now, will update the patch once this issue gets resolved. Thanks

          Zian Chen Zian Chen added a comment - I'm changing the patch according to the comments and find some memory leak over there, I'm working on it now, will update the patch once this issue gets resolved. Thanks
          genericqa genericqa added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 17m 20s trunk passed
          +1 compile 0m 54s trunk passed
          +1 mvnsite 0m 35s trunk passed
          +1 shadedclient 29m 4s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 32s the patch passed
          +1 compile 0m 48s the patch passed
          +1 cc 0m 48s the patch passed
          +1 javac 0m 48s the patch passed
          +1 mvnsite 0m 30s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 11m 22s patch has no errors when building and testing our client artifacts.
                Other Tests
          -1 unit 18m 24s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 24s The patch does not generate ASF License warnings.
          61m 53s



          Reason Tests
          Failed junit tests TEST-cetest



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12912513/YARN-7626.009.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux 18e0514fdbda 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 315f48e
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          unit https://builds.apache.org/job/PreCommit-YARN-Build/19847/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19847/testReport/
          Max. process+thread count 302 (vs. ulimit of 10000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19847/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 17m 20s trunk passed +1 compile 0m 54s trunk passed +1 mvnsite 0m 35s trunk passed +1 shadedclient 29m 4s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 32s the patch passed +1 compile 0m 48s the patch passed +1 cc 0m 48s the patch passed +1 javac 0m 48s the patch passed +1 mvnsite 0m 30s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 11m 22s patch has no errors when building and testing our client artifacts.       Other Tests -1 unit 18m 24s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 24s The patch does not generate ASF License warnings. 61m 53s Reason Tests Failed junit tests TEST-cetest Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12912513/YARN-7626.009.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 18e0514fdbda 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 315f48e maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 unit https://builds.apache.org/job/PreCommit-YARN-Build/19847/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19847/testReport/ Max. process+thread count 302 (vs. ulimit of 10000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19847/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Zian Chen Zian Chen added a comment -

          Address failed test cases and rebuild the patch

          Zian Chen Zian Chen added a comment - Address failed test cases and rebuild the patch
          genericqa genericqa added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 28s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 15m 7s trunk passed
          +1 compile 0m 47s trunk passed
          +1 mvnsite 0m 29s trunk passed
          +1 shadedclient 24m 58s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 44s the patch passed
          +1 cc 0m 44s the patch passed
          +1 javac 0m 44s the patch passed
          +1 mvnsite 0m 28s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 9m 41s patch has no errors when building and testing our client artifacts.
                Other Tests
          +1 unit 19m 10s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          56m 38s



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12912683/YARN-7626.010.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux a55fa8e8d64c 4.4.0-89-generic #112-Ubuntu SMP Mon Jul 31 19:38:41 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 29233c3
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19864/testReport/
          Max. process+thread count 473 (vs. ulimit of 10000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19864/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 28s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 15m 7s trunk passed +1 compile 0m 47s trunk passed +1 mvnsite 0m 29s trunk passed +1 shadedclient 24m 58s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 30s the patch passed +1 compile 0m 44s the patch passed +1 cc 0m 44s the patch passed +1 javac 0m 44s the patch passed +1 mvnsite 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 9m 41s patch has no errors when building and testing our client artifacts.       Other Tests +1 unit 19m 10s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 56m 38s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12912683/YARN-7626.010.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux a55fa8e8d64c 4.4.0-89-generic #112-Ubuntu SMP Mon Jul 31 19:38:41 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 29233c3 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19864/testReport/ Max. process+thread count 473 (vs. ulimit of 10000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19864/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Zian Chen Zian Chen added a comment - - edited

          Hi miklos.szegedi@cloudera.com , could you help me check the latest patch? Thanks!

          Zian Chen Zian Chen added a comment - - edited Hi miklos.szegedi@cloudera.com , could you help me check the latest patch? Thanks!
          leftnoteasy Wangda Tan added a comment -

          Thanks Zian Chen. +1, LGTM, miklos.szegedi@cloudera.com, do you want to take a look at the latest patch?

          leftnoteasy Wangda Tan added a comment - Thanks Zian Chen . +1, LGTM, miklos.szegedi@cloudera.com , do you want to take a look at the latest patch?
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thank you for the patch Zian Chen and for the review leftnoteasy.

          Optional: I have one style issue with the latest patch. When you refer to 6 in your patch like the one below, you should probably do sizeof("regex:"). This helps to better understand the code and it is more future proof.

          132	return is_volume_name(requested) && (execute_regex_match(pattern + 6, requested) == 0);
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thank you for the patch Zian Chen and for the review leftnoteasy . Optional: I have one style issue with the latest patch. When you refer to 6 in your patch like the one below, you should probably do sizeof("regex:"). This helps to better understand the code and it is more future proof. 132 return is_volume_name(requested) && (execute_regex_match(pattern + 6, requested) == 0);
          Zian Chen Zian Chen added a comment -

          Sure miklos.szegedi@cloudera.com totally agree with that style change. Will update the patch shortly. Thank you leftnoteasy for the review as well.

          Zian Chen Zian Chen added a comment - Sure miklos.szegedi@cloudera.com totally agree with that style change. Will update the patch shortly. Thank you  leftnoteasy for the review as well.
          genericqa genericqa added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 33s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 15m 42s trunk passed
          +1 compile 0m 49s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 shadedclient 26m 20s branch has no errors when building and testing our client artifacts.
                Patch Compile Tests
          +1 mvninstall 0m 29s the patch passed
          +1 compile 0m 45s the patch passed
          +1 cc 0m 45s the patch passed
          +1 javac 0m 45s the patch passed
          +1 mvnsite 0m 28s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 shadedclient 10m 5s patch has no errors when building and testing our client artifacts.
                Other Tests
          +1 unit 19m 55s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          59m 18s



          Subsystem Report/Notes
          Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:d4cc50f
          JIRA Issue YARN-7626
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12913126/YARN-7626.011.patch
          Optional Tests asflicense compile cc mvnsite javac unit
          uname Linux fadf7a03e941 4.4.0-64-generic #85-Ubuntu SMP Mon Feb 20 11:50:30 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/patchprocess/precommit/personality/provided.sh
          git revision trunk / 4971276
          maven version: Apache Maven 3.3.9
          Default Java 1.8.0_151
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19894/testReport/
          Max. process+thread count 435 (vs. ulimit of 10000)
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/19894/console
          Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          genericqa genericqa added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 33s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 15m 42s trunk passed +1 compile 0m 49s trunk passed +1 mvnsite 0m 34s trunk passed +1 shadedclient 26m 20s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 0m 29s the patch passed +1 compile 0m 45s the patch passed +1 cc 0m 45s the patch passed +1 javac 0m 45s the patch passed +1 mvnsite 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 10m 5s patch has no errors when building and testing our client artifacts.       Other Tests +1 unit 19m 55s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 59m 18s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:d4cc50f JIRA Issue YARN-7626 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12913126/YARN-7626.011.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux fadf7a03e941 4.4.0-64-generic #85-Ubuntu SMP Mon Feb 20 11:50:30 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / 4971276 maven version: Apache Maven 3.3.9 Default Java 1.8.0_151 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/19894/testReport/ Max. process+thread count 435 (vs. ulimit of 10000) modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/19894/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Zian Chen Zian Chen added a comment -

          leftnoteasy , I think after changing the style issue according to Miklos's latest comments, the latest patch should be good. Could you help me do a final review and commit the patch if no other issue? Really appreciate your help!

          miklos.szegedi@cloudera.com Really appreciates your help with detailed suggestions for the code changes!

          Zian Chen Zian Chen added a comment - leftnoteasy , I think after changing the style issue according to Miklos's latest comments, the latest patch should be good. Could you help me do a final review and commit the patch if no other issue? Really appreciate your help! miklos.szegedi@cloudera.com Really appreciates your help with detailed suggestions for the code changes!
          leftnoteasy Wangda Tan added a comment -

          Thanks miklos.szegedi@cloudera.com/Zian Chen, will commit the patch by tomorrow if no objections.

          leftnoteasy Wangda Tan added a comment - Thanks miklos.szegedi@cloudera.com / Zian Chen , will commit the patch by tomorrow if no objections.
          leftnoteasy Wangda Tan added a comment - - edited

          Committed to trunk, thanks Zian Chen. And thanks thorough reviews from miklos.szegedi@cloudera.com.

          leftnoteasy Wangda Tan added a comment - - edited Committed to trunk, thanks Zian Chen . And thanks thorough reviews from miklos.szegedi@cloudera.com .
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thank you for the contribution Zian Chen and for the commit leftnoteasy.

          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thank you for the contribution Zian Chen and for the commit leftnoteasy .
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #13789 (See https://builds.apache.org/job/Hadoop-trunk-Commit/13789/)
          YARN-7626. Allow regular expression matching in container-executor.cfg (wangda: rev 037d7834833df2d1e60f5015b60d42550b1ddce6)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #13789 (See https://builds.apache.org/job/Hadoop-trunk-Commit/13789/ ) YARN-7626 . Allow regular expression matching in container-executor.cfg (wangda: rev 037d7834833df2d1e60f5015b60d42550b1ddce6) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c

          People

            Zian Chen Zian Chen
            Zian Chen Zian Chen
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: