----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37396/#review95095 -----------------------------------------------------------
Ship it! Looks fine to me. Minor issue listed below. ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java (line 127) <https://reviews.apache.org/r/37396/#comment149910> Minor issue: I would recommend using an enum for the connection type here. This would make the code a little more straightforward when people look at the calls to isKdcReachable() in the future. It may also simplify some of the logging logic you have in place, since you could just convert the enum to a string, rather than have the test for useUDP in the logging code. This isn't something I'd hold up a merge for, but might be useful for the future. - Robert Nettleton On Aug. 12, 2015, 3:27 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37396/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2015, 3:27 p.m.) > > > Review request for Ambari, Jonathan Hurley, John Speidel, and Robert > Nettleton. > > > Bugs: AMBARI-12744 > https://issues.apache.org/jira/browse/AMBARI-12744 > > > Repository: ambari > > > Description > ------- > > When enabling Kerberos, click Test KDC Connection, things succeed but there > is an error printed to ambari-server log. This is happening when the KDC > listens only on UDP and the TCP check fails. > > ``` > 06 Aug 2015 21:20:16,179 ERROR [qtp-client-29] > KdcServerConnectionVerification:133 - Unable to connect to Kerberos Server > 06 Aug 2015 21:20:26,111 ERROR [qtp-client-25] > KdcServerConnectionVerification:133 - Unable to connect to Kerberos Server > ``` > > #Solution > Fix logging to be more explicit to avoid confusion > > Also, consolidate code to use the same mechanism for testing UDP and TCP > connections to the KDC. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java > e464800 > > ambari-server/src/test/java/org/apache/ambari/server/api/rest/KdcServerConnectionVerificationTest.java > da47eb2 > > Diff: https://reviews.apache.org/r/37396/diff/ > > > Testing > ------- > > Manually tested against MIT KDC and Active Directory to see updated logging > messages and ensure changes worked as expected. > > #Local test results: > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 50:15.061s > [INFO] Finished at: Wed Aug 12 11:09:34 EDT 2015 > [INFO] Final Memory: 67M/1559M > [INFO] > ------------------------------------------------------------------------ > > > #Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >
