----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51713/#review148193 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 101) <https://reviews.apache.org/r/51713/#comment215641> Can this possibly be null somehow can cause a NPE later on? ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java (line 117) <https://reviews.apache.org/r/51713/#comment215640> If the cluster wasn't found, then you didn't update anything, so this log message is misleading. Suggest returning an error if the cluster can't be found early on and removing the if-statement around the bulk of the logic. ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptorContainer.java (lines 763 - 765) <https://reviews.apache.org/r/51713/#comment215644> Can we simplify this method with Objects.hash(...) or a HashCodeBuilder() ? ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptorContainer.java (lines 786 - 789) <https://reviews.apache.org/r/51713/#comment215645> Can we simplify this method with an Objects.equals ? ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelper.java (line 537) <https://reviews.apache.org/r/51713/#comment215652> I don't think the else is needed here ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelper.java (lines 555 - 558) <https://reviews.apache.org/r/51713/#comment215660> I'm worried about this logic on downgrade. When upgrading from 2.3 to 2.5, the previous value is from 2.3 and the new value is from 2.5. However a downgrade also has the same values (from=2.3, to=2.5) but the direction changes to DOWNGRADE. Doesn't that mean that this needs to check direction as well? ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/kerberos.json (lines 107 - 108) <https://reviews.apache.org/r/51713/#comment215647> Does this change require work in the next upgrade catalog? - Jonathan Hurley On Sept. 8, 2016, 11:09 a.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51713/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2016, 11:09 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, > and Sumit Mohanty. > > > Bugs: AMBARI-18335 > https://issues.apache.org/jira/browse/AMBARI-18335 > > > Repository: ambari > > > Description > ------- > > Steps to repro: > - Install Ambari 2.2.2 > - Install HDP-2.4.x cluster with Atlas > - Stop Atlas > - Upgrade Ambari to 2.4 > - Delete Atlas service > - Upgrade the cluster to HDP-2.5.x cluster > - Add Atlas service. > > *Below config properties are missing from atlas-applicataion.properties file > for Atlas, Storm, Falcon, Hive services.* > ``` > atlas.jaas.KafkaClient.option.keyTab = > /etc/security/keytabs/atlas.service.keytab > atlas.jaas.KafkaClient.option.principal = atlas/_h...@example.com > ``` > > From HDP 2.4 to 2.5, the kerberos.json file for Atlas changed. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpgradeUserKerberosDescriptor.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptor.java > 84a9111 > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptorContainer.java > 39ebdaf > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosConfigurationDescriptor.java > 3cdd9908 > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptor.java > 484f65c > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelper.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosKeytabDescriptor.java > 7ce1c9f > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptor.java > 0156e4a > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosServiceDescriptor.java > 72dbcfe > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java > d472b79 > > ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/kerberos.json > d569447 > > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/nonrolling-upgrade-2.5.xml > ad9fc97 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml > 27f2010 > > ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/nonrolling-upgrade-2.5.xml > 4df5fef > ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml > b8c51f5 > > ambari-server/src/main/resources/stacks/HDP/2.5/services/HBASE/kerberos.json > 501bcd3 > > ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/nonrolling-upgrade-2.5.xml > 0b6f762 > ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml > 49e9d87 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterKerberosDescriptorResourceProviderTest.java > f551b42 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosComponentDescriptorTest.java > b74f417 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosConfigurationDescriptorTest.java > 6017fae > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorTest.java > 9463749 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelperTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosIdentityDescriptorTest.java > 874da31 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosKeytabDescriptorTest.java > c10d106 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java > 5c249e2 > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosServiceDescriptorTest.java > e1af515 > > Diff: https://reviews.apache.org/r/51713/diff/ > > > Testing > ------- > > unit tests, manually upgrades > > # Local test results: > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 1:22:13.782s > [INFO] Finished at: Wed Sep 07 22:51:30 EDT 2016 > [INFO] Final Memory: 60M/1835M > [INFO] > ------------------------------------------------------------------------ > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >
