You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by John Speidel <js...@hortonworks.com> on 2015/03/02 20:06:20 UTC

Review Request 31631: Testing of KDC connection doesn't work for UDP

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31631/
-----------------------------------------------------------

Review request for Ambari, Robert Levas and Robert Nettleton.


Bugs: AMBARI-9870
    https://issues.apache.org/jira/browse/AMBARI-9870


Repository: ambari


Description
-------

The KDC connection test functionality that is used by the UI doesn't work when the KDC server is configured to use UDP. 
This is a significant issue on Ubuntu where the MIT KDC uses UDP as the default. 
The current functionality only attempts to connect via TCP.


Diffs
-----

  ambari-project/pom.xml 0577bee 
  ambari-server/pom.xml c57a2d0 
  ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java 8bfbc5f 
  ambari-server/src/test/java/org/apache/ambari/server/api/rest/KdcServerConnectionVerificationTest.java f8ec650 

Diff: https://reviews.apache.org/r/31631/diff/


Testing
-------

Manual Functional Tests:
 - both positive and negative tests for TCP and UDP

Unit Tests:
 - added new unit tests for UDP validation
 - all unit tests pass
 
Results :
Tests run: 2751, Failures: 0, Errors: 0, Skipped: 15
...
Total run:608
Total errors:0
Total failures:0


Thanks,

John Speidel


Re: Review Request 31631: Testing of KDC connection doesn't work for UDP

Posted by John Speidel <js...@hortonworks.com>.

> On March 2, 2015, 7:59 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java, line 69
> > <https://reviews.apache.org/r/31631/diff/1/?file=882274#file882274line69>
> >
> >     This looks fine as is, but maybe this should be configurable in the future? 
> >     
> >     I'm not an expert in UDP, but perhaps we should consider making this a configurable setting, just to make things flexible in larger clusters.  
> >     
> >     Ten seconds is probably fine, it just might be nice to have the ability to customize this value if we hit a network that needs a larger timeout.
> >     
> >     If there isn't really a need to customize this timeout property, then I'd recommend making this final.

I don't really see a need to make the default value configurable and setUdpTimeout() allows the default to be overriden.


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31631/#review74804
-----------------------------------------------------------


On March 2, 2015, 7:06 p.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31631/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 7:06 p.m.)
> 
> 
> Review request for Ambari, Robert Levas and Robert Nettleton.
> 
> 
> Bugs: AMBARI-9870
>     https://issues.apache.org/jira/browse/AMBARI-9870
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The KDC connection test functionality that is used by the UI doesn't work when the KDC server is configured to use UDP. 
> This is a significant issue on Ubuntu where the MIT KDC uses UDP as the default. 
> The current functionality only attempts to connect via TCP.
> 
> 
> Diffs
> -----
> 
>   ambari-project/pom.xml 0577bee 
>   ambari-server/pom.xml c57a2d0 
>   ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java 8bfbc5f 
>   ambari-server/src/test/java/org/apache/ambari/server/api/rest/KdcServerConnectionVerificationTest.java f8ec650 
> 
> Diff: https://reviews.apache.org/r/31631/diff/
> 
> 
> Testing
> -------
> 
> Manual Functional Tests:
>  - both positive and negative tests for TCP and UDP
> 
> Unit Tests:
>  - added new unit tests for UDP validation
>  - all unit tests pass
>  
> Results :
> Tests run: 2751, Failures: 0, Errors: 0, Skipped: 15
> ...
> Total run:608
> Total errors:0
> Total failures:0
> 
> 
> Thanks,
> 
> John Speidel
> 
>


Re: Review Request 31631: Testing of KDC connection doesn't work for UDP

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On March 2, 2015, 7:59 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java, line 69
> > <https://reviews.apache.org/r/31631/diff/1/?file=882274#file882274line69>
> >
> >     This looks fine as is, but maybe this should be configurable in the future? 
> >     
> >     I'm not an expert in UDP, but perhaps we should consider making this a configurable setting, just to make things flexible in larger clusters.  
> >     
> >     Ten seconds is probably fine, it just might be nice to have the ability to customize this value if we hit a network that needs a larger timeout.
> >     
> >     If there isn't really a need to customize this timeout property, then I'd recommend making this final.
> 
> John Speidel wrote:
>     I don't really see a need to make the default value configurable and setUdpTimeout() allows the default to be overriden.

Sure, that's fine.  My point though was that if the timeout is too small for some network scenario that we haven't thought of yet, then a code change would be required to fix it, rather than a configuration flag.  I wouldn't hold up a merge for this, but it may be worth investigating in the future.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31631/#review74804
-----------------------------------------------------------


On March 2, 2015, 7:06 p.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31631/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 7:06 p.m.)
> 
> 
> Review request for Ambari, Robert Levas and Robert Nettleton.
> 
> 
> Bugs: AMBARI-9870
>     https://issues.apache.org/jira/browse/AMBARI-9870
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The KDC connection test functionality that is used by the UI doesn't work when the KDC server is configured to use UDP. 
> This is a significant issue on Ubuntu where the MIT KDC uses UDP as the default. 
> The current functionality only attempts to connect via TCP.
> 
> 
> Diffs
> -----
> 
>   ambari-project/pom.xml 0577bee 
>   ambari-server/pom.xml c57a2d0 
>   ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java 8bfbc5f 
>   ambari-server/src/test/java/org/apache/ambari/server/api/rest/KdcServerConnectionVerificationTest.java f8ec650 
> 
> Diff: https://reviews.apache.org/r/31631/diff/
> 
> 
> Testing
> -------
> 
> Manual Functional Tests:
>  - both positive and negative tests for TCP and UDP
> 
> Unit Tests:
>  - added new unit tests for UDP validation
>  - all unit tests pass
>  
> Results :
> Tests run: 2751, Failures: 0, Errors: 0, Skipped: 15
> ...
> Total run:608
> Total errors:0
> Total failures:0
> 
> 
> Thanks,
> 
> John Speidel
> 
>


Re: Review Request 31631: Testing of KDC connection doesn't work for UDP

Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31631/#review74804
-----------------------------------------------------------

Ship it!


Looks fine to me, just added some minor issues below.


ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java
<https://reviews.apache.org/r/31631/#comment121566>

    The code below treats this variable as if it were specified in seconds, so I'd recommend updating the javadoc here to reflect that.



ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java
<https://reviews.apache.org/r/31631/#comment121572>

    This looks fine as is, but maybe this should be configurable in the future? 
    
    I'm not an expert in UDP, but perhaps we should consider making this a configurable setting, just to make things flexible in larger clusters.  
    
    Ten seconds is probably fine, it just might be nice to have the ability to customize this value if we hit a network that needs a larger timeout.
    
    If there isn't really a need to customize this timeout property, then I'd recommend making this final.


- Robert Nettleton


On March 2, 2015, 7:06 p.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31631/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 7:06 p.m.)
> 
> 
> Review request for Ambari, Robert Levas and Robert Nettleton.
> 
> 
> Bugs: AMBARI-9870
>     https://issues.apache.org/jira/browse/AMBARI-9870
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The KDC connection test functionality that is used by the UI doesn't work when the KDC server is configured to use UDP. 
> This is a significant issue on Ubuntu where the MIT KDC uses UDP as the default. 
> The current functionality only attempts to connect via TCP.
> 
> 
> Diffs
> -----
> 
>   ambari-project/pom.xml 0577bee 
>   ambari-server/pom.xml c57a2d0 
>   ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java 8bfbc5f 
>   ambari-server/src/test/java/org/apache/ambari/server/api/rest/KdcServerConnectionVerificationTest.java f8ec650 
> 
> Diff: https://reviews.apache.org/r/31631/diff/
> 
> 
> Testing
> -------
> 
> Manual Functional Tests:
>  - both positive and negative tests for TCP and UDP
> 
> Unit Tests:
>  - added new unit tests for UDP validation
>  - all unit tests pass
>  
> Results :
> Tests run: 2751, Failures: 0, Errors: 0, Skipped: 15
> ...
> Total run:608
> Total errors:0
> Total failures:0
> 
> 
> Thanks,
> 
> John Speidel
> 
>


Re: Review Request 31631: Testing of KDC connection doesn't work for UDP

Posted by John Speidel <js...@hortonworks.com>.

> On March 2, 2015, 7:34 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java, line 160
> > <https://reviews.apache.org/r/31631/diff/1/?file=882274#file882274line160>
> >
> >     Doesn't this do the timeout _work_ rather than needed to create a thread?

You would think so, but in some UDP cases this library doesn't honor the timeout and attempts to read data forever.
I was also surprised by this.


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31631/#review74798
-----------------------------------------------------------


On March 2, 2015, 7:06 p.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31631/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 7:06 p.m.)
> 
> 
> Review request for Ambari, Robert Levas and Robert Nettleton.
> 
> 
> Bugs: AMBARI-9870
>     https://issues.apache.org/jira/browse/AMBARI-9870
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The KDC connection test functionality that is used by the UI doesn't work when the KDC server is configured to use UDP. 
> This is a significant issue on Ubuntu where the MIT KDC uses UDP as the default. 
> The current functionality only attempts to connect via TCP.
> 
> 
> Diffs
> -----
> 
>   ambari-project/pom.xml 0577bee 
>   ambari-server/pom.xml c57a2d0 
>   ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java 8bfbc5f 
>   ambari-server/src/test/java/org/apache/ambari/server/api/rest/KdcServerConnectionVerificationTest.java f8ec650 
> 
> Diff: https://reviews.apache.org/r/31631/diff/
> 
> 
> Testing
> -------
> 
> Manual Functional Tests:
>  - both positive and negative tests for TCP and UDP
> 
> Unit Tests:
>  - added new unit tests for UDP validation
>  - all unit tests pass
>  
> Results :
> Tests run: 2751, Failures: 0, Errors: 0, Skipped: 15
> ...
> Total run:608
> Total errors:0
> Total failures:0
> 
> 
> Thanks,
> 
> John Speidel
> 
>


Re: Review Request 31631: Testing of KDC connection doesn't work for UDP

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31631/#review74798
-----------------------------------------------------------

Ship it!


Ship It!


ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java
<https://reviews.apache.org/r/31631/#comment121558>

    Doesn't this do the timeout _work_ rather than needed to create a thread?


- Robert Levas


On March 2, 2015, 2:06 p.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31631/
> -----------------------------------------------------------
> 
> (Updated March 2, 2015, 2:06 p.m.)
> 
> 
> Review request for Ambari, Robert Levas and Robert Nettleton.
> 
> 
> Bugs: AMBARI-9870
>     https://issues.apache.org/jira/browse/AMBARI-9870
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The KDC connection test functionality that is used by the UI doesn't work when the KDC server is configured to use UDP. 
> This is a significant issue on Ubuntu where the MIT KDC uses UDP as the default. 
> The current functionality only attempts to connect via TCP.
> 
> 
> Diffs
> -----
> 
>   ambari-project/pom.xml 0577bee 
>   ambari-server/pom.xml c57a2d0 
>   ambari-server/src/main/java/org/apache/ambari/server/KdcServerConnectionVerification.java 8bfbc5f 
>   ambari-server/src/test/java/org/apache/ambari/server/api/rest/KdcServerConnectionVerificationTest.java f8ec650 
> 
> Diff: https://reviews.apache.org/r/31631/diff/
> 
> 
> Testing
> -------
> 
> Manual Functional Tests:
>  - both positive and negative tests for TCP and UDP
> 
> Unit Tests:
>  - added new unit tests for UDP validation
>  - all unit tests pass
>  
> Results :
> Tests run: 2751, Failures: 0, Errors: 0, Skipped: 15
> ...
> Total run:608
> Total errors:0
> Total failures:0
> 
> 
> Thanks,
> 
> John Speidel
> 
>