----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61500/#review182462 -----------------------------------------------------------
Ship it! I just had a quick question about the first check encountered being a service check. I see you added a test case for this, so I guess I'm just not seeing how it's covered here. ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java Lines 501-503 (original), 509-527 (patched) <https://reviews.apache.org/r/61500/#comment258384> So, how does this guard against the first X services being skipped and the first group encountered is a ServiceCheck. Wouldn't `previousGroupHolder` be null in this case, so it would always add it? - Jonathan Hurley On Aug. 8, 2017, 12:03 p.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61500/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2017, 12:03 p.m.) > > > Review request for Ambari, Dmitro Lisnichenko and Jonathan Hurley. > > > Bugs: AMBARI-21679 > https://issues.apache.org/jira/browse/AMBARI-21679 > > > Repository: ambari > > > Description > ------- > > There are two issues with Service Checks and patches: > > 1. A service check grouping can be orchestrated one right after the other if > the grouping in between them has no processing. > 2. A service check grouping can be orchestrated before any components have > been scheduled. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java > 07012961af > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java > 3deb7c866e > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java > fed5b77942 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java > 6a085f331f > > ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java > 24a3fa2ce2 > > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml > f82b0258de > > > Diff: https://reviews.apache.org/r/61500/diff/1/ > > > Testing > ------- > > Manual. Automated: > > Tests run: 4481, Failures: 0, Errors: 0, Skipped: 31 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 21:58.183s > [INFO] Finished at: Tue Aug 08 11:36:30 EDT 2017 > [INFO] Final Memory: 57M/637M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Nate Cole > >
