----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37461/#review95857 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java (lines 1001 - 1013) <https://reviews.apache.org/r/37461/#comment151006> Aren't you just using the enum name() here? If there is a difference, put the function string value as a getter on the enum values. The default enum for wrapper.getType() should be UNKNOWN. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java (line 1055) <https://reviews.apache.org/r/37461/#comment151007> function string can be UKNOWN, so if something ever made it the agent, this will likely fail (but I think that's ok) ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeType.java (line 25) <https://reviews.apache.org/r/37461/#comment151009> This is good - I envision one day adding a PATCH type :) ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeType.java (lines 28 - 29) <https://reviews.apache.org/r/37461/#comment151008> Formatting. ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml (lines 154 - 156) <https://reviews.apache.org/r/37461/#comment151002> Not really a TODO - if the component isn't present, it better not get scheduled. ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml (lines 169 - 172) <https://reviews.apache.org/r/37461/#comment151003> This thing is really strange. What is it's purpose? If it's a marker for downgrade, why does it have a direction of UPGRADE? Are you trying to say that in the direction of UPGRADE, this is their chance to downgrade? Weird. ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml <https://reviews.apache.org/r/37461/#comment151004> See other comment. I think these should stay. ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml <https://reviews.apache.org/r/37461/#comment151001> Your reasoning here is conflicting with a previous review about the rolling/non-rolling element. Either you like to be specific or you prefer to use implicit meanings. We started off last Sept making it implicit and Mahadev indicated we should have the element so people know exactly what's going to happen, because the element was there. I can absolutely see the case where we can make an upgrade pack that has some rolling (using "restart") and other stop the world. In fact, I think Oozie would prefer a stop-all in the regular rolling upgrade. We shouldn't be changing original decisions like this because we think the file is big. ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java (line 7187) <https://reviews.apache.org/r/37461/#comment151005> I prefer the previous assertion. It keeps the test honest. - Nate Cole On Aug. 18, 2015, 4:59 p.m., Alejandro Fernandez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37461/ > ----------------------------------------------------------- > > (Updated Aug. 18, 2015, 4:59 p.m.) > > > Review request for Ambari, Dmitro Lisnichenko, Jonathan Hurley, and Nate Cole. > > > Bugs: AMBARI-12755 > https://issues.apache.org/jira/browse/AMBARI-12755 > > > Repository: ambari > > > Description > ------- > > Subtask of Epic, AMBARI-12698 (Update the stack by stopping and starting > services in an orchestrated fashion) > > Initial commit to introduce a "nonrolling" type of Upgrade Pack, so that > UpgradeHelper can orchestrate it correctly during an upgrade. > For starters, this can create a nonrolling upgrade pack for HDP 2.2.x -> 2.2.y > > This is commit #1 out of many, so there are a lot of #TODO comments. > > > Diffs > ----- > > > ambari-common/src/main/python/resource_management/libraries/functions/hdp_select.py > d0ee9ad > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java > 43bdbfe > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > 770cc04 > > ambari-server/src/main/java/org/apache/ambari/server/metadata/ActionMetadata.java > e821827 > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeContext.java > 86dbccd > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java > 5e63744 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java > 9691292 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java > ad84210 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java > 8a9e2e5 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ExecuteTask.java > a0afdfb > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java > a1e1fcd > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ManualTask.java > 2b1ba56 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RestartGrouping.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RestartTask.java > 1b69b5b > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServerActionTask.java > 7a42c3b > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServerSideActionTask.java > 97981ae > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java > 4fe5e98 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckTask.java > 5893edf > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapper.java > eac5ce5 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java > f7b37ab > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StartGrouping.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StartTask.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StopGrouping.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StopTask.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Task.java > 6416b57 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeFunction.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeType.java > PRE-CREATION > > ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/scripts/hbase_upgrade.py > 610f527 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/namenode.py > 1415367 > > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml > PRE-CREATION > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml > 72032c3 > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml > f6823c8 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml > 7471025 > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > 599a1f7 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java > 7d2c117 > > ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java > 6267f53 > > ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java > 7a1d522 > ambari-server/src/test/python/stacks/2.0.6/HBASE/test_hbase_master.py > c4ff3dc > > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_bucket_test.xml > 92e8c6a > > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_direction.xml > 89a9e4f > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml > b7a62f5 > > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml > b4b6663 > > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_nonrolling.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_to_new_stack.xml > 02b0ebf > ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test.xml > 5271ae6 > > ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_checks.xml > 892b9b4 > > Diff: https://reviews.apache.org/r/37461/diff/ > > > Testing > ------- > > Deployed a cluster, and copied my changes over, then verified that > registering a new version still allowed performing a Rolling Upgrade (this > defaulted to using the rolling upgrade pack). > Manually edited the DB to use my nonrolling upgrade pack, and was able to > perform a Stop-and-Start Upgrade where the orchestration was correct. > > I ran unit tests for the following classes, and they all passed. > UpgradeActionTest > UpgradeResourceProviderTest > UpgradeResourceProviderHDP22Test > UpgradeResourceDefinitionTest > UpgradePackTest > UpgradeCheckStackVersionTest > UpgradeCheckOrderTest > UpgradeItemServiceTest > UpgradeHelperTest > > Waiting for full unit test results. > > > Thanks, > > Alejandro Fernandez > >
