You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2017/01/03 22:13:46 UTC
Review Request 55153: Previous Upgrade In Progress Pre-Req Check
Incorrectly Detects ABORTED Upgrade
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55153/
-----------------------------------------------------------
Review request for Ambari, Dmitro Lisnichenko and Nate Cole.
Bugs: AMBARI-19347
https://issues.apache.org/jira/browse/AMBARI-19347
Repository: ambari
Description
-------
Due to the fix made for
AMBARI-19053, the prereq check for previously created needs to be corrected for determining the downgrade that matched the failed upgrade.
The original logic for this check was overly complex (and kind of wrong). It ignored the state of the downgrade if one existed. In reality, we should care about the "last" downgrade and what its state is. Additionally, checking for `FinalizeUpgradeAction` as part of the commands in an upgrade is problematic for upgrades which may not have this command on different stacks.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/checks/PreviousUpgradeCompleted.java ff82750
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 67be152
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java 2d0a4d7
ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java 86ba1a1
ambari-server/src/test/java/org/apache/ambari/server/checks/PreviousUpgradeCompletedTest.java ab8da1b
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java 1861c0e
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java cc49cbd
Diff: https://reviews.apache.org/r/55153/diff/
Testing
-------
Manually tested:
- No prior upgrades
- Existing upgrade in progress
- Existing upgrade holding
- Existing upgrade suspended
- Aborted upgrade via API without a downgrade
- Existing downgrade
- Full upgrade and full downgrade
PENDING UNIT TESTS
Thanks,
Jonathan Hurley
Re: Review Request 55153: Previous Upgrade In Progress Pre-Req Check
Incorrectly Detects ABORTED Upgrade
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55153/#review160511
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Jan. 4, 2017, 12:19 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55153/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2017, 12:19 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko and Nate Cole.
>
>
> Bugs: AMBARI-19347
> https://issues.apache.org/jira/browse/AMBARI-19347
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Due to the fix made for
> AMBARI-19053, the prereq check for previously created needs to be corrected for determining the downgrade that matched the failed upgrade.
>
> The original logic for this check was overly complex (and kind of wrong). It ignored the state of the downgrade if one existed. In reality, we should care about the "last" downgrade and what its state is. Additionally, checking for `FinalizeUpgradeAction` as part of the commands in an upgrade is problematic for upgrades which may not have this command on different stacks.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/PreviousUpgradeCompleted.java ff82750
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 67be152
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java 2d0a4d7
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java 86ba1a1
> ambari-server/src/main/resources/stacks/HDP/2.6/upgrades/host-ordered-upgrade.xml PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/PreviousUpgradeCompletedTest.java ab8da1b
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java 1861c0e
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java cc49cbd
>
> Diff: https://reviews.apache.org/r/55153/diff/
>
>
> Testing
> -------
>
> Manually tested:
> - No prior upgrades
> - Existing upgrade in progress
> - Existing upgrade holding
> - Existing upgrade suspended
> - Aborted upgrade via API without a downgrade
> - Existing downgrade
> - Full upgrade and full downgrade
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 27:03 min
> [INFO] Finished at: 2017-01-03T18:23:59-05:00
> [INFO] Final Memory: 65M/736M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55153: Previous Upgrade In Progress Pre-Req Check
Incorrectly Detects ABORTED Upgrade
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55153/
-----------------------------------------------------------
(Updated Jan. 4, 2017, 12:19 p.m.)
Review request for Ambari, Dmitro Lisnichenko and Nate Cole.
Bugs: AMBARI-19347
https://issues.apache.org/jira/browse/AMBARI-19347
Repository: ambari
Description
-------
Due to the fix made for
AMBARI-19053, the prereq check for previously created needs to be corrected for determining the downgrade that matched the failed upgrade.
The original logic for this check was overly complex (and kind of wrong). It ignored the state of the downgrade if one existed. In reality, we should care about the "last" downgrade and what its state is. Additionally, checking for `FinalizeUpgradeAction` as part of the commands in an upgrade is problematic for upgrades which may not have this command on different stacks.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/checks/PreviousUpgradeCompleted.java ff82750
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 67be152
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java 2d0a4d7
ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java 86ba1a1
ambari-server/src/main/resources/stacks/HDP/2.6/upgrades/host-ordered-upgrade.xml PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/checks/PreviousUpgradeCompletedTest.java ab8da1b
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java 1861c0e
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java cc49cbd
Diff: https://reviews.apache.org/r/55153/diff/
Testing
-------
Manually tested:
- No prior upgrades
- Existing upgrade in progress
- Existing upgrade holding
- Existing upgrade suspended
- Aborted upgrade via API without a downgrade
- Existing downgrade
- Full upgrade and full downgrade
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 27:03 min
[INFO] Finished at: 2017-01-03T18:23:59-05:00
[INFO] Final Memory: 65M/736M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley
Re: Review Request 55153: Previous Upgrade In Progress Pre-Req Check
Incorrectly Detects ABORTED Upgrade
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55153/#review160504
-----------------------------------------------------------
Fix it, then Ship it!
ambari-server/src/main/java/org/apache/ambari/server/checks/PreviousUpgradeCompleted.java (line 65)
<https://reviews.apache.org/r/55153/#comment231643>
formatting
- Nate Cole
On Jan. 3, 2017, 8:04 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55153/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2017, 8:04 p.m.)
>
>
> Review request for Ambari, Dmitro Lisnichenko and Nate Cole.
>
>
> Bugs: AMBARI-19347
> https://issues.apache.org/jira/browse/AMBARI-19347
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Due to the fix made for
> AMBARI-19053, the prereq check for previously created needs to be corrected for determining the downgrade that matched the failed upgrade.
>
> The original logic for this check was overly complex (and kind of wrong). It ignored the state of the downgrade if one existed. In reality, we should care about the "last" downgrade and what its state is. Additionally, checking for `FinalizeUpgradeAction` as part of the commands in an upgrade is problematic for upgrades which may not have this command on different stacks.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/PreviousUpgradeCompleted.java ff82750
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 67be152
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java 2d0a4d7
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java 86ba1a1
> ambari-server/src/test/java/org/apache/ambari/server/checks/PreviousUpgradeCompletedTest.java ab8da1b
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java 1861c0e
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java cc49cbd
>
> Diff: https://reviews.apache.org/r/55153/diff/
>
>
> Testing
> -------
>
> Manually tested:
> - No prior upgrades
> - Existing upgrade in progress
> - Existing upgrade holding
> - Existing upgrade suspended
> - Aborted upgrade via API without a downgrade
> - Existing downgrade
> - Full upgrade and full downgrade
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 27:03 min
> [INFO] Finished at: 2017-01-03T18:23:59-05:00
> [INFO] Final Memory: 65M/736M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55153: Previous Upgrade In Progress Pre-Req Check
Incorrectly Detects ABORTED Upgrade
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55153/
-----------------------------------------------------------
(Updated Jan. 3, 2017, 8:04 p.m.)
Review request for Ambari, Dmitro Lisnichenko and Nate Cole.
Bugs: AMBARI-19347
https://issues.apache.org/jira/browse/AMBARI-19347
Repository: ambari
Description
-------
Due to the fix made for
AMBARI-19053, the prereq check for previously created needs to be corrected for determining the downgrade that matched the failed upgrade.
The original logic for this check was overly complex (and kind of wrong). It ignored the state of the downgrade if one existed. In reality, we should care about the "last" downgrade and what its state is. Additionally, checking for `FinalizeUpgradeAction` as part of the commands in an upgrade is problematic for upgrades which may not have this command on different stacks.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/checks/PreviousUpgradeCompleted.java ff82750
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 67be152
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java 2d0a4d7
ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java 86ba1a1
ambari-server/src/test/java/org/apache/ambari/server/checks/PreviousUpgradeCompletedTest.java ab8da1b
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java 1861c0e
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java cc49cbd
Diff: https://reviews.apache.org/r/55153/diff/
Testing (updated)
-------
Manually tested:
- No prior upgrades
- Existing upgrade in progress
- Existing upgrade holding
- Existing upgrade suspended
- Aborted upgrade via API without a downgrade
- Existing downgrade
- Full upgrade and full downgrade
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 27:03 min
[INFO] Finished at: 2017-01-03T18:23:59-05:00
[INFO] Final Memory: 65M/736M
[INFO] ------------------------------------------------------------------------
Thanks,
Jonathan Hurley