Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-5793

Trim configuration values in DockerLinuxContainerRuntime

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha1
    • Fix Version/s: 2.9.0, 3.0.0-alpha2
    • Component/s: nodemanager
    • Labels:
      None
    • Flags:
      Patch

      Description

      The current implementation of DockerLinuxContainerRuntime does not follow the practice of trimming configuration values. This leads to errors if users set values containing space or newline.

      see the following YARN commits as reference:
      YARN-3395. FairScheduler: Trim whitespaces when using username for queuename.
      YARN-2869. CapacityScheduler should trim sub queue names when parse configuration.
      YARN-2843. Fixed NodeLabelsManager to trim inputs for hosts and labels so as to make them work correctly.

      and many other Hadoop/HDFS commits (just list a few):
      HDFS-9708. FSNamesystem.initAuditLoggers() doesn't trim classnames
      HDFS-2799. Trim fs.checkpoint.dir values.
      HADOOP-6578. Configuration should trim whitespace around a lot of value types
      HADOOP-6534. Trim whitespace from directory lists initializing

      Patch is available against trunk

      DockerLinuxContainerRuntime.java
      @@ -219,9 +219,9 @@ public void initialize(Configuration conf)
           dockerClient = new DockerClient(conf);
           allowedNetworks.clear();
           allowedNetworks.addAll(Arrays.asList(
      -        conf.getStrings(YarnConfiguration.NM_DOCKER_ALLOWED_CONTAINER_NETWORKS,
      +        conf.getTrimmedStrings(YarnConfiguration.NM_DOCKER_ALLOWED_CONTAINER_NETWORKS,
                   YarnConfiguration.DEFAULT_NM_DOCKER_ALLOWED_CONTAINER_NETWORKS)));
      -    defaultNetwork = conf.get(
      +    defaultNetwork = conf.getTrimmed(
               YarnConfiguration.NM_DOCKER_DEFAULT_CONTAINER_NETWORK,
               YarnConfiguration.DEFAULT_NM_DOCKER_DEFAULT_CONTAINER_NETWORK);
       
      @@ -237,7 +237,7 @@ public void initialize(Configuration conf)
             throw new ContainerExecutionException(message);
           }
       
      -    privilegedContainersAcl = new AccessControlList(conf.get(
      +    privilegedContainersAcl = new AccessControlList(conf.getTrimmed(
               YarnConfiguration.NM_DOCKER_PRIVILEGED_CONTAINERS_ACL,
               YarnConfiguration.DEFAULT_NM_DOCKER_PRIVILEGED_CONTAINERS_ACL));
         }
      @@ -439,7 +439,7 @@ public void launchContainer(ContainerRuntimeContext ctx)
               LOCALIZED_RESOURCES);
           @SuppressWarnings("unchecked")
           List<String> userLocalDirs = ctx.getExecutionAttribute(USER_LOCAL_DIRS);
      -    Set<String> capabilities = new HashSet<>(Arrays.asList(conf.getStrings(
      +    Set<String> capabilities = new HashSet<>(Arrays.asList(conf.getTrimmedStrings(
               YarnConfiguration.NM_DOCKER_CONTAINER_CAPABILITIES,
               YarnConfiguration.DEFAULT_NM_DOCKER_CONTAINER_CAPABILITIES)));
      
      1. YARN-5793.0001.patch
        3 kB
        Tianyin Xu
      2. YARN-5793.0000.patch
        2 kB
        Tianyin Xu

        Activity

        Hide
        tianyin Tianyin Xu added a comment -

        patch against trunk

        Show
        tianyin Tianyin Xu added a comment - patch against trunk
        Hide
        templedf Daniel Templeton added a comment -

        Makes sense to me. Wanna go ahead with posting those changes as a patch?

        Show
        templedf Daniel Templeton added a comment - Makes sense to me. Wanna go ahead with posting those changes as a patch?
        Hide
        tianyin Tianyin Xu added a comment -

        patch is attached, and tested on my local repo. perhaps wait for Hudson's saying.

        Show
        tianyin Tianyin Xu added a comment - patch is attached, and tested on my local repo. perhaps wait for Hudson's saying.
        Hide
        templedf Daniel Templeton added a comment -

        Sorry, I jumped the gun.

        Show
        templedf Daniel Templeton added a comment - Sorry, I jumped the gun.
        Hide
        templedf Daniel Templeton added a comment -

        +1 pending Jenkins.

        Show
        templedf Daniel Templeton added a comment - +1 pending Jenkins.
        Hide
        tianyin Tianyin Xu added a comment -

        thanks, Daniel!

        Show
        tianyin Tianyin Xu added a comment - thanks, Daniel!
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 22s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 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 mvninstall 8m 3s trunk passed
        +1 compile 0m 28s trunk passed
        +1 checkstyle 0m 18s trunk passed
        +1 mvnsite 0m 29s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 0m 43s trunk passed
        +1 javadoc 0m 18s trunk passed
        +1 mvninstall 0m 30s the patch passed
        +1 compile 0m 25s the patch passed
        +1 javac 0m 25s the patch passed
        -0 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 2 new + 4 unchanged - 0 fixed = 6 total (was 4)
        +1 mvnsite 0m 31s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 52s the patch passed
        +1 javadoc 0m 18s the patch passed
        -1 unit 15m 20s hadoop-yarn-server-nodemanager in the patch failed.
        -1 asflicense 16m 11s The patch generated 4 ASF License warnings.
        46m 43s



        Reason Tests
        Failed junit tests hadoop.yarn.server.nodemanager.containermanager.queuing.TestQueuingContainerManager



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue YARN-5793
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12835688/YARN-5793.0000.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux c4bff9e7cbc8 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 5877f20
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13615/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13615/artifact/patchprocess/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/13615/testReport/
        asflicense https://builds.apache.org/job/PreCommit-YARN-Build/13615/artifact/patchprocess/patch-asflicense-problems.txt
        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/13615/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 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 mvninstall 8m 3s trunk passed +1 compile 0m 28s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 29s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 43s trunk passed +1 javadoc 0m 18s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 25s the patch passed +1 javac 0m 25s the patch passed -0 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 2 new + 4 unchanged - 0 fixed = 6 total (was 4) +1 mvnsite 0m 31s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 52s the patch passed +1 javadoc 0m 18s the patch passed -1 unit 15m 20s hadoop-yarn-server-nodemanager in the patch failed. -1 asflicense 16m 11s The patch generated 4 ASF License warnings. 46m 43s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.queuing.TestQueuingContainerManager Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5793 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12835688/YARN-5793.0000.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c4bff9e7cbc8 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5877f20 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13615/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13615/artifact/patchprocess/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/13615/testReport/ asflicense https://builds.apache.org/job/PreCommit-YARN-Build/13615/artifact/patchprocess/patch-asflicense-problems.txt 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/13615/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        tianyin Tianyin Xu added a comment -

        fix the style issue (80 character per line)

        the failed test (org.apache.hadoop.yarn.server.nodemanager.containermanager.queuing.testKillMultipleOpportunisticContainers) seems has nothing to do with the patch and I've tested on my local repo and it passed this test successfully.

        Show
        tianyin Tianyin Xu added a comment - fix the style issue (80 character per line) the failed test (org.apache.hadoop.yarn.server.nodemanager.containermanager.queuing.testKillMultipleOpportunisticContainers) seems has nothing to do with the patch and I've tested on my local repo and it passed this test successfully.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 14s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 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 mvninstall 8m 3s trunk passed
        +1 compile 0m 28s trunk passed
        +1 checkstyle 0m 17s trunk passed
        +1 mvnsite 0m 28s trunk passed
        +1 mvneclipse 0m 15s trunk passed
        +1 findbugs 0m 46s trunk passed
        +1 javadoc 0m 19s trunk passed
        +1 mvninstall 0m 26s the patch passed
        +1 compile 0m 28s the patch passed
        +1 javac 0m 28s the patch passed
        -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 1 new + 4 unchanged - 0 fixed = 5 total (was 4)
        +1 mvnsite 0m 29s the patch passed
        +1 mvneclipse 0m 11s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 52s the patch passed
        +1 javadoc 0m 16s the patch passed
        -1 unit 15m 11s hadoop-yarn-server-nodemanager in the patch failed.
        -1 asflicense 16m 4s The patch generated 4 ASF License warnings.
        46m 24s



        Reason Tests
        Failed junit tests hadoop.yarn.server.nodemanager.containermanager.queuing.TestQueuingContainerManager



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue YARN-5793
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12835705/YARN-5793.0001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 1d10bcc7c169 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / b62bc2b
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13622/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13622/artifact/patchprocess/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/13622/testReport/
        asflicense https://builds.apache.org/job/PreCommit-YARN-Build/13622/artifact/patchprocess/patch-asflicense-problems.txt
        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/13622/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 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 mvninstall 8m 3s trunk passed +1 compile 0m 28s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 46s trunk passed +1 javadoc 0m 19s trunk passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 28s the patch passed +1 javac 0m 28s the patch passed -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 1 new + 4 unchanged - 0 fixed = 5 total (was 4) +1 mvnsite 0m 29s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 52s the patch passed +1 javadoc 0m 16s the patch passed -1 unit 15m 11s hadoop-yarn-server-nodemanager in the patch failed. -1 asflicense 16m 4s The patch generated 4 ASF License warnings. 46m 24s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.queuing.TestQueuingContainerManager Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5793 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12835705/YARN-5793.0001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1d10bcc7c169 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b62bc2b Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13622/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13622/artifact/patchprocess/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/13622/testReport/ asflicense https://builds.apache.org/job/PreCommit-YARN-Build/13622/artifact/patchprocess/patch-asflicense-problems.txt 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/13622/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        tianyin Tianyin Xu added a comment -

        Daniel Templeton, I'm puzzled by the Jenkins report. The failed test, org.apache.hadoop.yarn.server.nodemanager.containermanager.queuing.testKillMultipleOpportunisticContainers, comes from the waitForNMContainerState call, which seems have nothing to do with the patch. Also, I did the test on my local repo with the newest code base, and the tests pass without any problem.

        Is it possible for you to take a look and point me what I'm missing?

        Thanks a lot in advance!
        ~t

        Show
        tianyin Tianyin Xu added a comment - Daniel Templeton , I'm puzzled by the Jenkins report. The failed test, org.apache.hadoop.yarn.server.nodemanager.containermanager.queuing.testKillMultipleOpportunisticContainers, comes from the waitForNMContainerState call, which seems have nothing to do with the patch. Also, I did the test on my local repo with the newest code base, and the tests pass without any problem. Is it possible for you to take a look and point me what I'm missing? Thanks a lot in advance! ~t
        Hide
        templedf Daniel Templeton added a comment -

        Looks like the test failure may be YARN-5377. The license warnings are from HADOOP-10075.

        If no one else has any comments, I'll commit this patch on Monday.

        Show
        templedf Daniel Templeton added a comment - Looks like the test failure may be YARN-5377 . The license warnings are from HADOOP-10075 . If no one else has any comments, I'll commit this patch on Monday.
        Hide
        tianyin Tianyin Xu added a comment -

        I see. Thanks a lot, Daniel Templeton!!

        btw, I just took a second look at the current code base and find there're other places using untrimmed configs. Back in 2014-2015, many folks systematically trimmed all the configs, but the practice seems not followed after that... do you think it's worth reporting/fixing these issues?

        Show
        tianyin Tianyin Xu added a comment - I see. Thanks a lot, Daniel Templeton !! btw, I just took a second look at the current code base and find there're other places using untrimmed configs. Back in 2014-2015, many folks systematically trimmed all the configs, but the practice seems not followed after that... do you think it's worth reporting/fixing these issues?
        Hide
        templedf Daniel Templeton added a comment -

        Thanks for the patch, Tianyin Xu. Committed to trunk and branch-2.

        It's probably not a bad idea to file JIRAs for the other places where untrimmed properties are used.

        Show
        templedf Daniel Templeton added a comment - Thanks for the patch, Tianyin Xu . Committed to trunk and branch-2. It's probably not a bad idea to file JIRAs for the other places where untrimmed properties are used.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10735 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10735/)
        YARN-5793. Trim configuration values in DockerLinuxContainerRuntime (templedf: rev f3eb4c3c738204e099cbaa03471497c46530efbf)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10735 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10735/ ) YARN-5793 . Trim configuration values in DockerLinuxContainerRuntime (templedf: rev f3eb4c3c738204e099cbaa03471497c46530efbf) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java
        Hide
        tianyin Tianyin Xu added a comment -

        thanks a lot, Daniel Templeton!

        Show
        tianyin Tianyin Xu added a comment - thanks a lot, Daniel Templeton !

          People

          • Assignee:
            tianyin Tianyin Xu
            Reporter:
            tianyin Tianyin Xu
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development