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




ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java
 (line 1938)
<https://reviews.apache.org/r/51254/#comment212593>

    UpgradeCatalog240 is not the best option for trunk. It should be 
UpgradeCatalog for the next release number. Otherwise, upgrade from 240 to next 
release will miss these changes.
    
    You can open a JIRA to resolve it once next UpgradeCatalog is identified. 
For now, ok to leave the code here.



ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py (line 
939)
<https://reviews.apache.org/r/51254/#comment212594>

    nit: should be a space after comma


- Sumit Mohanty


On Aug. 19, 2016, 9: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, 9: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