You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Tom Beerbower <tb...@hortonworks.com> on 2015/01/20 16:41:36 UTC
Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/
-----------------------------------------------------------
Review request for Ambari, Jonathan Hurley and Nate Cole.
Bugs: AMBARI-9214
https://issues.apache.org/jira/browse/AMBARI-9214
Repository: ambari
Description
-------
Add an Upgrade ResourceDefinition so that we can use these directives:
downgrade: boolean that makes the upgrade a downgrade
skip_service_checks: boolean that disables the upcoming service-check-all after core components
Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
Diff: https://reviews.apache.org/r/30077/diff/
Testing
-------
Unit tests added. All existing tests pass.
Manual tests in progress...
Thanks,
Tom Beerbower
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68732
-----------------------------------------------------------
Ship it!
Ship It!
- Jonathan Hurley
On Jan. 20, 2015, 10:41 a.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 10:41 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Nate Cole <nc...@hortonworks.com>.
> On Jan. 20, 2015, 11:45 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, lines 529-531
> > <https://reviews.apache.org/r/30077/diff/1/?file=826862#file826862line529>
> >
> > I'm not sure this is what should be happening. I think the directive may be to prevent the "full service check" from happening (3 times this gets scheduled). This code will stop all service checks from happening. Let's confirm with Yusaku since he was requesting it.
>
> Tom Beerbower wrote:
> Ok, I assumed that the "full" service check was just the collection of all the individual service checks. Where does the "full service check" stage get created? Is there already logic in place to skip it and I just need to hook up the directive?
>
> Thanks for the review!
>
> Nate Cole wrote:
> Confirmed, it's only for the "full upgrade" check. That happens currently in the UpgradeHelper class. If you want, I can take that one since I'm working on that code anyway?
>
> Tom Beerbower wrote:
> Ok, sure. How should I leave this? Remove the skip logic that I added from the RP, but leave the directive in place?
Yes, perfect. Thanks for doing this!
- Nate
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68730
-----------------------------------------------------------
On Jan. 20, 2015, 10:41 a.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 10:41 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Nate Cole <nc...@hortonworks.com>.
> On Jan. 20, 2015, 11:45 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, lines 529-531
> > <https://reviews.apache.org/r/30077/diff/1/?file=826862#file826862line529>
> >
> > I'm not sure this is what should be happening. I think the directive may be to prevent the "full service check" from happening (3 times this gets scheduled). This code will stop all service checks from happening. Let's confirm with Yusaku since he was requesting it.
>
> Tom Beerbower wrote:
> Ok, I assumed that the "full" service check was just the collection of all the individual service checks. Where does the "full service check" stage get created? Is there already logic in place to skip it and I just need to hook up the directive?
>
> Thanks for the review!
Confirmed, it's only for the "full upgrade" check. That happens currently in the UpgradeHelper class. If you want, I can take that one since I'm working on that code anyway?
- Nate
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68730
-----------------------------------------------------------
On Jan. 20, 2015, 10:41 a.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 10:41 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Jan. 20, 2015, 4:45 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, lines 529-531
> > <https://reviews.apache.org/r/30077/diff/1/?file=826862#file826862line529>
> >
> > I'm not sure this is what should be happening. I think the directive may be to prevent the "full service check" from happening (3 times this gets scheduled). This code will stop all service checks from happening. Let's confirm with Yusaku since he was requesting it.
Ok, I assumed that the "full" service check was just the collection of all the individual service checks. Where does the "full service check" stage get created? Is there already logic in place to skip it and I just need to hook up the directive?
Thanks for the review!
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68730
-----------------------------------------------------------
On Jan. 20, 2015, 3:41 p.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 3:41 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Jan. 20, 2015, 4:45 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, lines 529-531
> > <https://reviews.apache.org/r/30077/diff/1/?file=826862#file826862line529>
> >
> > I'm not sure this is what should be happening. I think the directive may be to prevent the "full service check" from happening (3 times this gets scheduled). This code will stop all service checks from happening. Let's confirm with Yusaku since he was requesting it.
>
> Tom Beerbower wrote:
> Ok, I assumed that the "full" service check was just the collection of all the individual service checks. Where does the "full service check" stage get created? Is there already logic in place to skip it and I just need to hook up the directive?
>
> Thanks for the review!
>
> Nate Cole wrote:
> Confirmed, it's only for the "full upgrade" check. That happens currently in the UpgradeHelper class. If you want, I can take that one since I'm working on that code anyway?
Ok, sure. How should I leave this? Remove the skip logic that I added from the RP, but leave the directive in place?
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68730
-----------------------------------------------------------
On Jan. 20, 2015, 3:41 p.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 3:41 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68730
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
<https://reviews.apache.org/r/30077/#comment113112>
I'm not sure this is what should be happening. I think the directive may be to prevent the "full service check" from happening (3 times this gets scheduled). This code will stop all service checks from happening. Let's confirm with Yusaku since he was requesting it.
- Nate Cole
On Jan. 20, 2015, 10:41 a.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 10:41 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Jan. 20, 2015, 4:06 p.m., Jonathan Hurley wrote:
> >
Thanks for reviewing!
> On Jan. 20, 2015, 4:06 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java, line 57
> > <https://reviews.apache.org/r/30077/diff/1/?file=826857#file826857line57>
> >
> > Should this be protected in order to let subresources modify it?
Sub resources can still access the collection through getCreateDirectives() and can still add/remove directives to that collection. I think that this is basically the same as the existing code, just that now the base level collection of create directives can be populated through a constructor.
> On Jan. 20, 2015, 4:06 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java, lines 61-67
> > <https://reviews.apache.org/r/30077/diff/1/?file=826858#file826858line61>
> >
> > You removed `validate_topology` but it never gets added to `createDirectives`; how does this directive get used now?
This actually had nothing to do with the Jira. I just noticed this bad cut and paste job so I cleaned it up. Client configs doesn't use the directive. It just carried over from when the Blueprint resource definition was copied to create the client config definition.
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68727
-----------------------------------------------------------
On Jan. 20, 2015, 3:41 p.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 3:41 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68727
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java
<https://reviews.apache.org/r/30077/#comment113107>
Should this be protected in order to let subresources modify it?
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java
<https://reviews.apache.org/r/30077/#comment113108>
You removed `validate_topology` but it never gets added to `createDirectives`; how does this directive get used now?
- Jonathan Hurley
On Jan. 20, 2015, 10:41 a.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 10:41 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68753
-----------------------------------------------------------
Ship it!
Ship It!
- Alejandro Fernandez
On Jan. 20, 2015, 6:18 p.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 6:18 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Jan. 20, 2015, 6:26 p.m., Alejandro Fernandez wrote:
> >
Thanks for the review!
> On Jan. 20, 2015, 6:26 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java, line 39
> > <https://reviews.apache.org/r/30077/diff/2/?file=826962#file826962line39>
> >
> > Shouldn't this still pass the "validate_topology" directive?
I doesn't look like the validate_topology directive is actually used for client configs. I think that it was included in the resource definition by accident when it was cut and pasted from the blueprint resource definition. Do you think or know for sure that the client configs uses this directive? I only see it used for blueprints.
> On Jan. 20, 2015, 6:26 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java, line 38
> > <https://reviews.apache.org/r/30077/diff/2/?file=826965#file826965line38>
> >
> > I assume you bit the bullet and used this approach so as to not assume that an upgrade/downgrade will always require service checks. I think this is more flexible :-)
I think it makes it a little easier to constrct a resource definition. In fact, the only reason that I even added this class was because I thought it was the logical place to define the directive constants. Otherwise, I would have just constructed a SimpleResourceDefinition inline in the factory.
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68747
-----------------------------------------------------------
On Jan. 20, 2015, 6:18 p.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 6:18 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68747
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java
<https://reviews.apache.org/r/30077/#comment113158>
Shouldn't this still pass the "validate_topology" directive?
ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java
<https://reviews.apache.org/r/30077/#comment113154>
I assume you bit the bullet and used this approach so as to not assume that an upgrade/downgrade will always require service checks. I think this is more flexible :-)
- Alejandro Fernandez
On Jan. 20, 2015, 6:18 p.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 6:18 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/#review68749
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Jan. 20, 2015, 1:18 p.m., Tom Beerbower wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30077/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2015, 1:18 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley and Nate Cole.
>
>
> Bugs: AMBARI-9214
> https://issues.apache.org/jira/browse/AMBARI-9214
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Add an Upgrade ResourceDefinition so that we can use these directives:
>
> downgrade: boolean that makes the upgrade a downgrade
> skip_service_checks: boolean that disables the upcoming service-check-all after core components
>
>
> Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
>
> Diff: https://reviews.apache.org/r/30077/diff/
>
>
> Testing
> -------
>
> Unit tests added. All existing tests pass.
>
> Manual tests in progress...
>
>
> Thanks,
>
> Tom Beerbower
>
>
Re: Review Request 30077: Upgrades: Make a ResourceDefinition for
UpgradeResourceProvider for downgrade directive
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30077/
-----------------------------------------------------------
(Updated Jan. 20, 2015, 6:18 p.m.)
Review request for Ambari, Jonathan Hurley and Nate Cole.
Changes
-------
Removed skip check logic from upgrade resource provider.
Bugs: AMBARI-9214
https://issues.apache.org/jira/browse/AMBARI-9214
Repository: ambari
Description
-------
Add an Upgrade ResourceDefinition so that we can use these directives:
downgrade: boolean that makes the upgrade a downgrade
skip_service_checks: boolean that disables the upcoming service-check-all after core components
Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java 1cd7e17
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java 4ff37ac
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java fc28c13
ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java 92ecd1e
ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java d9c9aec
ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java 73aa828
ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java d264511
ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java cd5f23e
Diff: https://reviews.apache.org/r/30077/diff/
Testing
-------
Unit tests added. All existing tests pass.
Manual tests in progress...
Thanks,
Tom Beerbower