> On Aug. 19, 2015, 1:25 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java,
> >  lines 1001-1013
> > <https://reviews.apache.org/r/37461/diff/4/?file=1043807#file1043807line1001>
> >
> >     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.

Not all of the StageWrapper types are a function, i.e., SERVER_SIDE_ACTION and 
RU_TASKS
Since I'm explicitly checking for START, STOP, and RESTART, I will set function 
= wrapper.getType().name();


> On Aug. 19, 2015, 1:25 p.m., Nate Cole wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml,
> >  lines 154-156
> > <https://reviews.apache.org/r/37461/diff/4/?file=1043834#file1043834line154>
> >
> >     Not really a TODO - if the component isn't present, it better not get 
> > scheduled.

These are my own comments so I remember to check those cases.


> On Aug. 19, 2015, 1:25 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeType.java,
> >  lines 28-29
> > <https://reviews.apache.org/r/37461/diff/4/?file=1043831#file1043831line28>
> >
> >     Formatting.

What formatting change did you want me to make?


> On Aug. 19, 2015, 1:25 p.m., Nate Cole wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml,
> >  lines 169-172
> > <https://reviews.apache.org/r/37461/diff/4/?file=1043834#file1043834line169>
> >
> >     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.

I haven't quite gotten to this point yet, but I image it will be something like 
that. When performing an UPGRADE, it we ever cross this group, then a Downgrade 
will have actual steps to do. If we haven't crossed it, then the Downgrade only 
needs to revert configs (since we've only taken backups and stopped services).


> On Aug. 19, 2015, 1:25 p.m., Nate Cole wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java,
> >  line 7187
> > <https://reviews.apache.org/r/37461/diff/4/?file=1043838#file1043838line7187>
> >
> >     I prefer the previous assertion.  It keeps the test honest.

Will revert.


> On Aug. 19, 2015, 1:25 p.m., Nate Cole wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, 
> > lines 337-342
> > <https://reviews.apache.org/r/37461/diff/4/?file=1043836#file1043836line337>
> >
> >     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.

Will revert to processing components again.


- Alejandro


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


On Aug. 18, 2015, 8: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, 8: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
> 
>

Reply via email to