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


Ship it!




Looks good once the prior comments are resolved.

- Jonathan Hurley


On Aug. 19, 2016, 5:55 p.m., Anita Jebaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51254/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2016, 5:55 p.m.)
> 
> 
> Review request for Ambari, Di Li, Jonathan Hurley, Nate Cole, Robert Levas, 
> Sumit Mohanty, Sriharsha Chintalapani, and Vitalyi Brodetskyi.
> 
> 
> Bugs: AMBARI-17694
>     https://issues.apache.org/jira/browse/AMBARI-17694
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This is a continuation of the review request 
> https://reviews.apache.org/r/50047/ . Opening a new request for clarity.
> 
> When kerberos is enabled, the protocol for listeners in 
> /etc/kafka/conf/server.properties is updated from PLAINTEXT to PLAINTEXTSASL, 
> even though the Ambari UI shows otherwise
> 
> 
> Updated the patch based on comments from Sumit, A follow up JIRA Ambari-17929 
> was reverted earlier, I have included the code for upgrade from the patch 
> with some minor changes. Also included the code for validation in 
> stack_advisor based on which a warning will be thrown in the UI, when the 
> listeners and security.inter.broker.protocol values are not in sync.
> 
> "Stack advisor code to recommend changes to revert to PLAINTEXT if not 
> kerberized" --> All the values get reverted back to default when kerberos is 
> disabled, so I didn't make any changes to this.
> 
> 
> Including Sumit's comment here for reference.
> 
> sorry, had to revert it. After deployment and some user operations the 
> configurations went out of sync
> 
> ...
> listeners=PLAINTEXT://nat-r7-kxqs-xaagents-re-3.openstacklocal:6667,SSL://nat-r7-kxqs-xaagents-re-3.openstacklocal:6666
> security.inter.broker.protocol=PLAINTEXTSASL
> ...
> 
> The over all approach is sound - works for fresh deployments blueprint and 
> UI. Looked through the patch and here are some additional changes (by the 
> way, I am not very familiar with Kafka):
> 
>     Existing deployments (that will go through Ambari upgrade to 2.4.0) will 
> either need 
>     1) code to replace PLAINTEXT to PLAINTEXTSASL in kafka.py or, 
>     2) UpgradeCatalog code to fix the configs stored in the DB. The later is 
> a better approach.
>     
>     Stack advisor code to ensure "listeners" and 
> "security.inter.broker.protocol" values are in sync. E.g. error if one is 
> PLAINTEXTSASL and one isn't
>     Stack advisor code to recommend changes to revert to PLAINTEXT if not 
> kerberized. I did not try but I was not sure if config will revert back 
> properly when unkerberized.
> 
> Sorry, could not get to it during code review.
> 
> Can we move this JIRA to 2.5.0, next release. It appears that some more test 
> scenarios need to be covered. Its too close for the 2.4.0 release to get all 
> paths tested.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
>  66be3bf 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java
>  12553a5 
>   ambari-server/src/main/resources/common-services/KAFKA/0.10.0/kerberos.json 
> e1e6461 
>   
> ambari-server/src/main/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
>  9066ab5 
>   ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
> 2b1c01b 
>   ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py 
> 64e8e03 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelperTest.java
>  ee2a671 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java
>  854ce7d 
> 
> Diff: https://reviews.apache.org/r/51254/diff/
> 
> 
> Testing
> -------
> 
> Added 2 new test case,
>  Ran mvn test
>  Tested in Ambari UI, by enabling kerberos, listeners protocol is updated and 
> kafka started successfully
>  
>  Deployed Ambari 2.2.2 enabled kerberos and then did an update, the kafka 
> listeners value got updated
>  
>  Changed the value in the UI, a warning is thrown on save.
> 
>  Disabled kerberos, value got reset to default PLAINTEXT://localhost:6667
>  
>  
>  
>  Sumit, please let me know if any other scenarios need to be tested.
> 
> 
> File Attachments
> ----------------
> 
> validation.jpg
>   
> https://reviews.apache.org/media/uploaded/files/2016/08/19/286c8651-2954-41ba-8d53-b734129ff1a8__validation.jpg
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Reply via email to