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

yarn application -list returns a tracking URL for AM that doesn't work in secured and HA environment

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha2
    • Fix Version/s: 2.9.0, 3.0.0-beta1
    • Component/s: amrmproxy
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The tracking URL given at the command line should work secured or not. The tracking URLs are like http://node-2.abc.com:47014 and AM web server supposed to redirect it to a RM address like this http://node-1.abc.com:8088/proxy/application_1494544954891_0002/, but it fails to do that because the connection is rejected when AM is talking to RM admin service to get HA status.

      1. YARN-6625.001.patch
        4 kB
        Yufei Gu
      2. YARN-6625.002.patch
        3 kB
        Yufei Gu
      3. YARN-6625.003.patch
        7 kB
        Yufei Gu
      4. YARN-6625.004.patch
        8 kB
        Yufei Gu
      5. YARN-6625.branch-2.005.patch
        8 kB
        Wangda Tan

        Issue Links

          Activity

          Hide
          yufeigu Yufei Gu added a comment - - edited

          Patch v1 provides a solution to makes RM admin service to support token like RM scheduler service does. Daryn Sharp, can you take a look?

          Show
          yufeigu Yufei Gu added a comment - - edited Patch v1 provides a solution to makes RM admin service to support token like RM scheduler service does. Daryn Sharp , can you take a look?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s 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.
          0 mvndep 0m 48s Maven dependency ordering for branch
          +1 mvninstall 14m 51s trunk passed
          +1 compile 8m 31s trunk passed
          +1 checkstyle 0m 52s trunk passed
          +1 mvnsite 1m 18s trunk passed
          +1 mvneclipse 0m 45s trunk passed
          -1 findbugs 1m 5s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings.
          +1 javadoc 1m 2s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 1s the patch passed
          +1 compile 7m 28s the patch passed
          +1 javac 7m 28s the patch passed
          -0 checkstyle 0m 52s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 32 unchanged - 0 fixed = 33 total (was 32)
          +1 mvnsite 1m 19s the patch passed
          +1 mvneclipse 0m 44s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 0s the patch passed
          +1 javadoc 1m 8s the patch passed
          -1 unit 2m 38s hadoop-yarn-common in the patch failed.
          +1 unit 40m 2s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 42s The patch does not generate ASF License warnings.
          98m 10s



          Reason Tests
          Failed junit tests hadoop.yarn.client.TestClientRMProxy



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-6625
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12868805/YARN-6625.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7c42862cbd81 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 009b9f3
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15970/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15970/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15970/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15970/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15970/console
          Powered by Apache Yetus 0.5.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 17s 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. 0 mvndep 0m 48s Maven dependency ordering for branch +1 mvninstall 14m 51s trunk passed +1 compile 8m 31s trunk passed +1 checkstyle 0m 52s trunk passed +1 mvnsite 1m 18s trunk passed +1 mvneclipse 0m 45s trunk passed -1 findbugs 1m 5s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings. +1 javadoc 1m 2s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 1s the patch passed +1 compile 7m 28s the patch passed +1 javac 7m 28s the patch passed -0 checkstyle 0m 52s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 32 unchanged - 0 fixed = 33 total (was 32) +1 mvnsite 1m 19s the patch passed +1 mvneclipse 0m 44s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 0s the patch passed +1 javadoc 1m 8s the patch passed -1 unit 2m 38s hadoop-yarn-common in the patch failed. +1 unit 40m 2s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 42s The patch does not generate ASF License warnings. 98m 10s Reason Tests Failed junit tests hadoop.yarn.client.TestClientRMProxy Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6625 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12868805/YARN-6625.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7c42862cbd81 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 009b9f3 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15970/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15970/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/15970/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15970/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15970/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment - - edited

          Overall looks good. Here's some comments:

          1. In ClientRMProxy, instead of return new Text(schedulerService + "," + adminService);, let's use Joiner like what's used below it in getTokenService.
          2. ClientRMProxy#getAMRMTokenService is used in a few places. Have you made sure that they're all okay with adding the RM admin address?
          3. I'm not expert on the way our RPCs work, but is HAServiceProtocolPB the right thing to check in AdminSecurityInfo? Just from the name, it seems funny to use an "HA" Protocol here because what happens in a non-HA cluster? In any case, based on the getKerberosInfo above it and the name itself, wouldn't ResourceManagerAdministrationProtocolPB be the right thing to use?
          4. It would also be good to add some kind of test, though that might be tricky
          Show
          rkanter Robert Kanter added a comment - - edited Overall looks good. Here's some comments: In ClientRMProxy , instead of return new Text(schedulerService + "," + adminService); , let's use Joiner like what's used below it in getTokenService . ClientRMProxy#getAMRMTokenService is used in a few places. Have you made sure that they're all okay with adding the RM admin address? I'm not expert on the way our RPCs work, but is HAServiceProtocolPB the right thing to check in AdminSecurityInfo ? Just from the name, it seems funny to use an "HA" Protocol here because what happens in a non-HA cluster? In any case, based on the getKerberosInfo above it and the name itself, wouldn't ResourceManagerAdministrationProtocolPB be the right thing to use? It would also be good to add some kind of test, though that might be tricky
          Hide
          yufeigu Yufei Gu added a comment - - edited

          Thanks Robert Kanter for the review. I uploaded patch v2 as our off-line discussion.
          First, we don't use the solution in patch v1 since it is not a good idea to let AM access the Admin service of RM. Second, AM cannot get RM HA status thought scheduler service since there is no scheduler service in a standby RM.

          Patch v2 provide a solution which try RMs one by one until we find one alive RM based on the assumption that it doesn't matter if RM is active or standby in this situation since if AM redirect to a standby RM, the standby RM will do redirect to active RM anyway. The only concern is that we should make sure the RM is alive.

          Show
          yufeigu Yufei Gu added a comment - - edited Thanks Robert Kanter for the review. I uploaded patch v2 as our off-line discussion. First, we don't use the solution in patch v1 since it is not a good idea to let AM access the Admin service of RM. Second, AM cannot get RM HA status thought scheduler service since there is no scheduler service in a standby RM. Patch v2 provide a solution which try RMs one by one until we find one alive RM based on the assumption that it doesn't matter if RM is active or standby in this situation since if AM redirect to a standby RM, the standby RM will do redirect to active RM anyway. The only concern is that we should make sure the RM is alive.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 2m 30s 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 14m 5s trunk passed
          +1 compile 0m 17s trunk passed
          +1 checkstyle 0m 11s trunk passed
          +1 mvnsite 0m 17s trunk passed
          +1 findbugs 0m 26s trunk passed
          +1 javadoc 0m 14s trunk passed
          +1 mvninstall 0m 17s the patch passed
          +1 compile 0m 15s the patch passed
          +1 javac 0m 15s the patch passed
          -0 checkstyle 0m 9s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy: The patch generated 1 new + 13 unchanged - 0 fixed = 14 total (was 13)
          +1 mvnsite 0m 15s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 30s the patch passed
          +1 javadoc 0m 11s the patch passed
          +1 unit 0m 22s hadoop-yarn-server-web-proxy in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          21m 31s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-6625
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12874148/YARN-6625.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ac8dd4d8289f 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6d116ff
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16225/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-web-proxy.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16225/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16225/console
          Powered by Apache Yetus 0.5.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 2m 30s 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 14m 5s trunk passed +1 compile 0m 17s trunk passed +1 checkstyle 0m 11s trunk passed +1 mvnsite 0m 17s trunk passed +1 findbugs 0m 26s trunk passed +1 javadoc 0m 14s trunk passed +1 mvninstall 0m 17s the patch passed +1 compile 0m 15s the patch passed +1 javac 0m 15s the patch passed -0 checkstyle 0m 9s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy: The patch generated 1 new + 13 unchanged - 0 fixed = 14 total (was 13) +1 mvnsite 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 30s the patch passed +1 javadoc 0m 11s the patch passed +1 unit 0m 22s hadoop-yarn-server-web-proxy in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 21m 31s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6625 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12874148/YARN-6625.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ac8dd4d8289f 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6d116ff Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16225/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-web-proxy.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16225/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy Console output https://builds.apache.org/job/PreCommit-YARN-Build/16225/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          I think the approach in the v2 patch is better and simpler. Here's some feedback:

          • I think we should flip around the logic in isValidUrl. There's a lot of other types of bad response codes we could get (e.g. 500 if there's some kind of unhandled exception) and really only one good type (i.e. 200).
          • Let's use constants defined in HttpURLConnection for the codes instead of magic numbers. http://download.java.net/jdk7/archive/b123/docs/api/java/net/HttpURLConnection.html#HTTP_OK
          • For for-each loops, I think the formatting is usually for (String rmId : rmIds) not for (String rmId: rmIds)
          • Once isValidUrl returns true in the loop in findRedirectUrl, we should be able to break, right? No reason to keep searching.
          • The else and continue in the loop in findRedirectUrl doesn't seem to be needed.
          • This newer solution should be unit testable. You can setup a good and bad (mock) servlet and call findRedirectUrl to see that you get the good one.
          Show
          rkanter Robert Kanter added a comment - I think the approach in the v2 patch is better and simpler. Here's some feedback: I think we should flip around the logic in isValidUrl . There's a lot of other types of bad response codes we could get (e.g. 500 if there's some kind of unhandled exception) and really only one good type (i.e. 200). Let's use constants defined in HttpURLConnection for the codes instead of magic numbers. http://download.java.net/jdk7/archive/b123/docs/api/java/net/HttpURLConnection.html#HTTP_OK For for-each loops, I think the formatting is usually for (String rmId : rmIds) not for (String rmId: rmIds) Once isValidUrl returns true in the loop in findRedirectUrl , we should be able to break , right? No reason to keep searching. The else and continue in the loop in findRedirectUrl doesn't seem to be needed. This newer solution should be unit testable. You can setup a good and bad (mock) servlet and call findRedirectUrl to see that you get the good one.
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Robert Kanter for the review. Uploaded patch v3 for all your comments.

          Show
          yufeigu Yufei Gu added a comment - Thanks Robert Kanter for the review. Uploaded patch v3 for all your comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s 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 12m 47s trunk passed
          +1 compile 0m 14s trunk passed
          +1 checkstyle 0m 13s trunk passed
          +1 mvnsite 0m 17s trunk passed
          +1 findbugs 0m 24s trunk passed
          +1 javadoc 0m 14s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 16s the patch passed
          +1 compile 0m 14s the patch passed
          +1 javac 0m 14s the patch passed
          -0 checkstyle 0m 9s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy: The patch generated 1 new + 19 unchanged - 0 fixed = 20 total (was 19)
          +1 mvnsite 0m 15s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 28s the patch passed
          +1 javadoc 0m 11s the patch passed
                Other Tests
          +1 unit 0m 23s hadoop-yarn-server-web-proxy in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          17m 55s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-6625
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877356/YARN-6625.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 88524ba7e0d2 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 75c0220
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16451/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-web-proxy.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16451/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16451/console
          Powered by Apache Yetus 0.6.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 11s 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 12m 47s trunk passed +1 compile 0m 14s trunk passed +1 checkstyle 0m 13s trunk passed +1 mvnsite 0m 17s trunk passed +1 findbugs 0m 24s trunk passed +1 javadoc 0m 14s trunk passed       Patch Compile Tests +1 mvninstall 0m 16s the patch passed +1 compile 0m 14s the patch passed +1 javac 0m 14s the patch passed -0 checkstyle 0m 9s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy: The patch generated 1 new + 19 unchanged - 0 fixed = 20 total (was 19) +1 mvnsite 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 28s the patch passed +1 javadoc 0m 11s the patch passed       Other Tests +1 unit 0m 23s hadoop-yarn-server-web-proxy in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 17m 55s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6625 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877356/YARN-6625.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 88524ba7e0d2 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 75c0220 Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16451/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-web-proxy.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16451/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy Console output https://builds.apache.org/job/PreCommit-YARN-Build/16451/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          Two last minor things and then we're good to go:

          1. The isValidUrl method can be more simpler/compact - something like this:
            private boolean isValidUrl(String url) {
              boolean isValid = false;
              try {
                HttpURLConnection conn =
                  (HttpURLConnection) new URL(url).openConnection();
                conn.connect();
                isValid = (conn.getResponseCode() == HttpURLConnection.HTTP_OK);
              } catch (Exception e) {
                LOG.debug("Failed to connect to " + url + ": " + e.toString());
              }
              return isValid;
            }
            
          2. Remove the * imports from TestAmFilter
          Show
          rkanter Robert Kanter added a comment - Two last minor things and then we're good to go: The isValidUrl method can be more simpler/compact - something like this: private boolean isValidUrl( String url) { boolean isValid = false ; try { HttpURLConnection conn = (HttpURLConnection) new URL(url).openConnection(); conn.connect(); isValid = (conn.getResponseCode() == HttpURLConnection.HTTP_OK); } catch (Exception e) { LOG.debug( "Failed to connect to " + url + ": " + e.toString()); } return isValid; } Remove the * imports from TestAmFilter
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Robert Kanter. Uploaded patch v4 for your comments.

          Show
          yufeigu Yufei Gu added a comment - Thanks Robert Kanter . Uploaded patch v4 for your comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 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 8s trunk passed
          +1 compile 0m 19s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 20s trunk passed
          +1 findbugs 0m 31s trunk passed
          +1 javadoc 0m 16s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 16s the patch passed
          +1 javac 0m 17s the patch passed
          +1 checkstyle 0m 10s the patch passed
          +1 mvnsite 0m 19s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 38s the patch passed
          +1 javadoc 0m 12s the patch passed
                Other Tests
          +1 unit 0m 28s hadoop-yarn-server-web-proxy in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          21m 10s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-6625
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877391/YARN-6625.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux dfeaf03a6734 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / a29fe10
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16452/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16452/console
          Powered by Apache Yetus 0.6.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 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 8s trunk passed +1 compile 0m 19s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 20s trunk passed +1 findbugs 0m 31s trunk passed +1 javadoc 0m 16s trunk passed       Patch Compile Tests +1 mvninstall 0m 18s the patch passed +1 compile 0m 16s the patch passed +1 javac 0m 17s the patch passed +1 checkstyle 0m 10s the patch passed +1 mvnsite 0m 19s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 38s the patch passed +1 javadoc 0m 12s the patch passed       Other Tests +1 unit 0m 28s hadoop-yarn-server-web-proxy in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 21m 10s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6625 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877391/YARN-6625.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux dfeaf03a6734 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a29fe10 Default Java 1.8.0_131 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16452/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy Console output https://builds.apache.org/job/PreCommit-YARN-Build/16452/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          +1

          Show
          rkanter Robert Kanter added a comment - +1
          Hide
          rkanter Robert Kanter added a comment -

          Thanks Yufei Gu. Committed to trunk!

          Show
          rkanter Robert Kanter added a comment - Thanks Yufei Gu . Committed to trunk!
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Robert Kanter for the review and lots of off-line discussion.

          Show
          yufeigu Yufei Gu added a comment - Thanks Robert Kanter for the review and lots of off-line discussion.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12011 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12011/)
          YARN-6625. yarn application -list returns a tracking URL for AM that (yufei: rev 9e0cde1469b8ffeb59619c64d6ece86b62424f04)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/amfilter/AmIpFilter.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/test/java/org/apache/hadoop/yarn/server/webproxy/amfilter/TestAmFilter.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12011 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12011/ ) YARN-6625 . yarn application -list returns a tracking URL for AM that (yufei: rev 9e0cde1469b8ffeb59619c64d6ece86b62424f04) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/amfilter/AmIpFilter.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/test/java/org/apache/hadoop/yarn/server/webproxy/amfilter/TestAmFilter.java
          Hide
          leftnoteasy Wangda Tan added a comment -

          Yufei Gu / Robert Kanter,

          Thanks for working on this patch, we recently saw this issue in our environment as well, is there any concerns to pull this fix to branch-2/branch-2.8?

          Show
          leftnoteasy Wangda Tan added a comment - Yufei Gu / Robert Kanter , Thanks for working on this patch, we recently saw this issue in our environment as well, is there any concerns to pull this fix to branch-2/branch-2.8?
          Hide
          yufeigu Yufei Gu added a comment -

          Tan, Wangda, not much as I know. May take time to rebase though.

          Show
          yufeigu Yufei Gu added a comment - Tan, Wangda , not much as I know. May take time to rebase though.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Yufei Gu!

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Yufei Gu !
          Hide
          leftnoteasy Wangda Tan added a comment -

          Attached ver.005 patch against branch-2. In the new patch I changed unit test to use mockito to stub isValidUrl. Major reason is I always facing issues to use org.mortbay.jetty implement same logics of org.eclipse.jetty. Yufei Gu, I'm not sure if you have any experiences of org.mortbay.jetty. Could you help to review this patch? Thanks.

          Show
          leftnoteasy Wangda Tan added a comment - Attached ver.005 patch against branch-2. In the new patch I changed unit test to use mockito to stub isValidUrl . Major reason is I always facing issues to use org.mortbay.jetty implement same logics of org.eclipse.jetty . Yufei Gu , I'm not sure if you have any experiences of org.mortbay.jetty . Could you help to review this patch? Thanks.
          Hide
          yufeigu Yufei Gu added a comment -

          Hi Tan, Wangda, I didn't see any change in the patch v5. Did you upload the right patch?

          Show
          yufeigu Yufei Gu added a comment - Hi Tan, Wangda , I didn't see any change in the patch v5. Did you upload the right patch?
          Hide
          leftnoteasy Wangda Tan added a comment -

          Yufei Gu, apologize for uploading a wrong patch, now this is the correct one, please let me know your thoughts.

          Show
          leftnoteasy Wangda Tan added a comment - Yufei Gu , apologize for uploading a wrong patch, now this is the correct one, please let me know your thoughts.
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks for the patch, Tan, Wangda. +1 for patch v5.

          Show
          yufeigu Yufei Gu added a comment - Thanks for the patch, Tan, Wangda . +1 for patch v5.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Yufei Gu for the review, I will commit the patch once Jenkins give +1.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Yufei Gu for the review, I will commit the patch once Jenkins give +1.
          Hide
          hadoopqa Hadoop QA 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.
                branch-2 Compile Tests
          +1 mvninstall 7m 25s branch-2 passed
          +1 compile 0m 14s branch-2 passed
          +1 checkstyle 0m 11s branch-2 passed
          +1 mvnsite 0m 16s branch-2 passed
          +1 findbugs 0m 26s branch-2 passed
          +1 javadoc 0m 11s branch-2 passed
                Patch Compile Tests
          +1 mvninstall 0m 12s the patch passed
          +1 compile 0m 13s the patch passed
          +1 javac 0m 13s the patch passed
          +1 checkstyle 0m 10s the patch passed
          +1 mvnsite 0m 17s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 34s the patch passed
          +1 javadoc 0m 10s the patch passed
                Other Tests
          +1 unit 0m 21s hadoop-yarn-server-web-proxy in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          12m 33s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:eaf5c66
          JIRA Issue YARN-6625
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12890179/YARN-6625.branch-2.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
          uname Linux 174db11a6e39 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 598329e
          Default Java 1.7.0_151
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/17751/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/17751/console
          Powered by Apache Yetus 0.6.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 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.       branch-2 Compile Tests +1 mvninstall 7m 25s branch-2 passed +1 compile 0m 14s branch-2 passed +1 checkstyle 0m 11s branch-2 passed +1 mvnsite 0m 16s branch-2 passed +1 findbugs 0m 26s branch-2 passed +1 javadoc 0m 11s branch-2 passed       Patch Compile Tests +1 mvninstall 0m 12s the patch passed +1 compile 0m 13s the patch passed +1 javac 0m 13s the patch passed +1 checkstyle 0m 10s the patch passed +1 mvnsite 0m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 34s the patch passed +1 javadoc 0m 10s the patch passed       Other Tests +1 unit 0m 21s hadoop-yarn-server-web-proxy in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 12m 33s Subsystem Report/Notes Docker Image:yetus/hadoop:eaf5c66 JIRA Issue YARN-6625 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12890179/YARN-6625.branch-2.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle uname Linux 174db11a6e39 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 598329e Default Java 1.7.0_151 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/17751/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy Console output https://builds.apache.org/job/PreCommit-YARN-Build/17751/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Pushed to branch-2 as well.

          Show
          leftnoteasy Wangda Tan added a comment - Pushed to branch-2 as well.

            People

            • Assignee:
              yufeigu Yufei Gu
              Reporter:
              yufeigu Yufei Gu
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development