> On Aug. 19, 2016, 10:46 p.m., Sumit Mohanty wrote: > > ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py, > > line 940 > > <https://reviews.apache.org/r/51254/diff/1/?file=1479690#file1479690line940> > > > > nit: should be a space after comma
Please refer to the validation.jpg for the message, I have included the space in the previous line of the code. > On Aug. 19, 2016, 10:46 p.m., Sumit Mohanty wrote: > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java, > > line 1938 > > <https://reviews.apache.org/r/51254/diff/1/?file=1479686#file1479686line1938> > > > > 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. sure Sumit, I will create a jira to update the code in UpgradeCatalog once it is identified. Is it okay to leave the code in UpgradeCatalog240 for now and move it later or should I remove the UpgrageCatalog240 code. - Anita ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51254/#review146225 ----------------------------------------------------------- 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 > >
