Details

      Description

      Added some tests for LinuxContainerExecuror
      YARN-186-branch-0.23.patch patch for branch-0.23
      YARN-186-branch-2.patch patch for branch-2
      ARN-186-trunk.patch patch for trank

      1. YARN-186-branch-0.23.patch
        7 kB
        Aleksey Gorshkov
      2. YARN-186-branch-2.patch
        7 kB
        Aleksey Gorshkov
      3. YARN-186-trunk.patch
        6 kB
        Aleksey Gorshkov

        Activity

        Hide
        Robert Joseph Evans added a comment -

        Submitting patch to kick build

        Show
        Robert Joseph Evans added a comment - Submitting patch to kick build
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12551354/YARN-186-trunk.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:

        org.apache.hadoop.yarn.server.nodemanager.TestLinuxContainerExecutorExtension

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/128//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/128//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12551354/YARN-186-trunk.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: org.apache.hadoop.yarn.server.nodemanager.TestLinuxContainerExecutorExtension +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/128//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/128//console This message is automatically generated.
        Hide
        Aleksey Gorshkov added a comment -

        fixed error

        Show
        Aleksey Gorshkov added a comment - fixed error
        Hide
        Robert Joseph Evans added a comment -

        The tests look good. but I have a few comments.

        1. There are also two existing tests for the LinuxContainerExecutor. TestLinuxContainerExecutor is more of an integration test and requires some manual setup with the actual binary to make it work. TestLinuxContainerExecutorWithMocks tries to test the code by mocking out parts under it. The tests here look like they should be added into TestLinuxContainerExecutorWithMocks instead of adding in a new file.
        2. In setup() a LinuxContainerExecutor exec is created, but it is only used in one of the tests, testErrorLauncher. It would probably be better to have the initialization happen in the test instead, which if you combine with TestLinuxContainerExecutorWithMocks this becomes a bit of a noop, because it is doing almost the exact same thing already for other tests.
        3. At a minimum some documentation about what writeScriptFile does would be good. Having it write a script that will echo the arguments into a file named after the last argument seems kind of confusing. Especially when to read those arguments you need to open a file named '--checksetup'. I personally would like to see it write the arguments out to a consistently named file, so that if the arguments are wrong you get an assertion indicating that instead of a FileNotFoundException. Which again if you combine with TestLinuxContainerExecutorWithMocks the writeScriptFile goes away because there is already code in the test to do something very similar.
        4. I would also like to see testInitException explained a little bit better what it is trying to test. It is confusing why only setting the executor path to /bin/sh causes init() to fail but in setup() you are setting it to /bin/bash, and you skip calling the init() method.

        Also if your patches are identical you don't really need to post separate patches for each branch. One patch for trunk that applies to all of the branches is simples to review.

        Show
        Robert Joseph Evans added a comment - The tests look good. but I have a few comments. There are also two existing tests for the LinuxContainerExecutor. TestLinuxContainerExecutor is more of an integration test and requires some manual setup with the actual binary to make it work. TestLinuxContainerExecutorWithMocks tries to test the code by mocking out parts under it. The tests here look like they should be added into TestLinuxContainerExecutorWithMocks instead of adding in a new file. In setup() a LinuxContainerExecutor exec is created, but it is only used in one of the tests, testErrorLauncher. It would probably be better to have the initialization happen in the test instead, which if you combine with TestLinuxContainerExecutorWithMocks this becomes a bit of a noop, because it is doing almost the exact same thing already for other tests. At a minimum some documentation about what writeScriptFile does would be good. Having it write a script that will echo the arguments into a file named after the last argument seems kind of confusing. Especially when to read those arguments you need to open a file named '--checksetup'. I personally would like to see it write the arguments out to a consistently named file, so that if the arguments are wrong you get an assertion indicating that instead of a FileNotFoundException. Which again if you combine with TestLinuxContainerExecutorWithMocks the writeScriptFile goes away because there is already code in the test to do something very similar. I would also like to see testInitException explained a little bit better what it is trying to test. It is confusing why only setting the executor path to /bin/sh causes init() to fail but in setup() you are setting it to /bin/bash, and you skip calling the init() method. Also if your patches are identical you don't really need to post separate patches for each branch. One patch for trunk that applies to all of the branches is simples to review.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12552293/YARN-186-trunk.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 2 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:

        org.apache.hadoop.yarn.server.nodemanager.TestLinuxContainerExecutorWithMocks

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/133//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/133//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12552293/YARN-186-trunk.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: org.apache.hadoop.yarn.server.nodemanager.TestLinuxContainerExecutorWithMocks +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/133//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/133//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12552459/YARN-186-trunk.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 2 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/137//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/137//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12552459/YARN-186-trunk.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/137//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/137//console This message is automatically generated.
        Hide
        Robert Joseph Evans added a comment -

        The patch looks good, and so do the tests. Thanks.

        My only question is why were the following lines added to setup?

        conf.set(YarnConfiguration.NM_LOCAL_DIRS, "file:///bin/echo");
        conf.set(YarnConfiguration.NM_LOG_DIRS, "file:///dev/null");
        

        The test seems to pass just fine without them, and I did not understand their need.

        Show
        Robert Joseph Evans added a comment - The patch looks good, and so do the tests. Thanks. My only question is why were the following lines added to setup? conf.set(YarnConfiguration.NM_LOCAL_DIRS, "file: ///bin/echo" ); conf.set(YarnConfiguration.NM_LOG_DIRS, "file: ///dev/ null " ); The test seems to pass just fine without them, and I did not understand their need.
        Hide
        Aleksey Gorshkov added a comment -

        I agree. I've cleaned files.

        Show
        Aleksey Gorshkov added a comment - I agree. I've cleaned files.
        Hide
        Robert Joseph Evans added a comment -

        Thanks Aleksey,

        I pulled this into trunk, branch-2, and branch-0.23

        Show
        Robert Joseph Evans added a comment - Thanks Aleksey, I pulled this into trunk, branch-2, and branch-0.23
        Hide
        Hudson added a comment -

        Integrated in Hadoop-trunk-Commit #2980 (See https://builds.apache.org/job/Hadoop-trunk-Commit/2980/)
        YARN-186. Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Show
        Hudson added a comment - Integrated in Hadoop-trunk-Commit #2980 (See https://builds.apache.org/job/Hadoop-trunk-Commit/2980/ ) YARN-186 . Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Yarn-trunk #31 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/31/)
        YARN-186. Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Show
        Hudson added a comment - Integrated in Hadoop-Yarn-trunk #31 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/31/ ) YARN-186 . Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #430 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/430/)
        svn merge -c 1407171 FIXES: YARN-186. Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407173)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407173
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #430 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/430/ ) svn merge -c 1407171 FIXES: YARN-186 . Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407173) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407173 Files : /hadoop/common/branches/branch-0.23/hadoop-yarn-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1221 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1221/)
        YARN-186. Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1221 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1221/ ) YARN-186 . Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1251 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1251/)
        YARN-186. Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171)

        Result = FAILURE
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1251 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1251/ ) YARN-186 . Coverage fixing LinuxContainerExecutor (Aleksey Gorshkov via bobby) (Revision 1407171) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1407171 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/resources/mock-container-executer-with-error

          People

          • Assignee:
            Aleksey Gorshkov
            Reporter:
            Aleksey Gorshkov
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development