You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Robert Nettleton <rn...@hortonworks.com> on 2015/01/12 22:55:28 UTC

Review Request 29824: Re-introduce usage of excluded-configs API in Blueprint processor

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

Review request for Ambari, John Speidel, Robert Levas, and Tom Beerbower.


Repository: ambari


Description
-------

This patch implements a fix for AMBARI-9097.

Falcon deployments are not currently working with Blueprints.

The ClusterResourceProvider is not currently handling the case
  of a Falcon Blueprints deployment, with respect to configuration
  handling.

A portion of the changes from the fix for AMBARI-8009 originally
  addressed this problem by adding some code to filter out the
  "excluded" configuration references for a given stack service, since
  only included config references should be used during a cluster
  configuration update.

Some code refactorings in the patch for AMBARI-7175 moved the filtering
  code to a separate location, and this eventually resulted in a
  breakage, since some components seem to interpret "excluded"
  configuration references differently.

To resolve this issue, this current patch implements the following:

 - Re-introduces some of the changes from AMBARI-8009 that were removed by
   subsequent refactorings.  These changes modify the ClusterResourceProvider's
   setConfigurationsOnCluster() method, so that excluded configuration
   references are not included for a given service's config change
   request.

 - Implements new unit tests to verify this change.

 - Updates some existing unit tests to reflect the changes to the Stack
   class.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/StackServiceResponse.java 0eecb3f 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 2d6ad8f 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java 243e282 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java c620bc6 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java bc2fc02 

Diff: https://reviews.apache.org/r/29824/diff/


Testing
-------

1. Ran ambari-server unit tests, all Java tests passing.  Some python failures, but these are due to a separate problem currently being addressed in the codebase. 
2. Manually verified that applying this patch will fix Falcon Blueprints deployments in single-node and multi-node clusters. 


Thanks,

Robert Nettleton


Re: Review Request 29824: Re-introduce usage of excluded-configs API in Blueprint processor

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On Jan. 13, 2015, 3 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java, line 452
> > <https://reviews.apache.org/r/29824/diff/1/?file=818013#file818013line452>
> >
> >     Minor nit on the method name.  Is it actually parsing the excluded configurations or just mapping them to a service?

Thanks for the review.

The method doesn't parse any strings.  I chose this name to be uniform with the other methods called by the constructor (parseComponents, parseConfigurations, etc). Some of the parse methods listed don't actually parse strings, but I thought it made sense to try to make this fit in with the rest of the code.


- Robert


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


On Jan. 12, 2015, 9:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29824/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2015, 9:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Robert Levas, and Tom Beerbower.
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch implements a fix for AMBARI-9097.
> 
> Falcon deployments are not currently working with Blueprints.
> 
> The ClusterResourceProvider is not currently handling the case
>   of a Falcon Blueprints deployment, with respect to configuration
>   handling.
> 
> A portion of the changes from the fix for AMBARI-8009 originally
>   addressed this problem by adding some code to filter out the
>   "excluded" configuration references for a given stack service, since
>   only included config references should be used during a cluster
>   configuration update.
> 
> Some code refactorings in the patch for AMBARI-7175 moved the filtering
>   code to a separate location, and this eventually resulted in a
>   breakage, since some components seem to interpret "excluded"
>   configuration references differently.
> 
> To resolve this issue, this current patch implements the following:
> 
>  - Re-introduces some of the changes from AMBARI-8009 that were removed by
>    subsequent refactorings.  These changes modify the ClusterResourceProvider's
>    setConfigurationsOnCluster() method, so that excluded configuration
>    references are not included for a given service's config change
>    request.
> 
>  - Implements new unit tests to verify this change.
> 
>  - Updates some existing unit tests to reflect the changes to the Stack
>    class.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/StackServiceResponse.java 0eecb3f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 2d6ad8f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java 243e282 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java c620bc6 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java bc2fc02 
> 
> Diff: https://reviews.apache.org/r/29824/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran ambari-server unit tests, all Java tests passing.  Some python failures, but these are due to a separate problem currently being addressed in the codebase. 
> 2. Manually verified that applying this patch will fix Falcon Blueprints deployments in single-node and multi-node clusters. 
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 29824: Re-introduce usage of excluded-configs API in Blueprint processor

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29824/#review67851
-----------------------------------------------------------

Ship it!


Looks good to me.


ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
<https://reviews.apache.org/r/29824/#comment111943>

    Minor nit on the method name.  Is it actually parsing the excluded configurations or just mapping them to a service?


- Tom Beerbower


On Jan. 12, 2015, 9:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29824/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2015, 9:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Robert Levas, and Tom Beerbower.
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch implements a fix for AMBARI-9097.
> 
> Falcon deployments are not currently working with Blueprints.
> 
> The ClusterResourceProvider is not currently handling the case
>   of a Falcon Blueprints deployment, with respect to configuration
>   handling.
> 
> A portion of the changes from the fix for AMBARI-8009 originally
>   addressed this problem by adding some code to filter out the
>   "excluded" configuration references for a given stack service, since
>   only included config references should be used during a cluster
>   configuration update.
> 
> Some code refactorings in the patch for AMBARI-7175 moved the filtering
>   code to a separate location, and this eventually resulted in a
>   breakage, since some components seem to interpret "excluded"
>   configuration references differently.
> 
> To resolve this issue, this current patch implements the following:
> 
>  - Re-introduces some of the changes from AMBARI-8009 that were removed by
>    subsequent refactorings.  These changes modify the ClusterResourceProvider's
>    setConfigurationsOnCluster() method, so that excluded configuration
>    references are not included for a given service's config change
>    request.
> 
>  - Implements new unit tests to verify this change.
> 
>  - Updates some existing unit tests to reflect the changes to the Stack
>    class.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/StackServiceResponse.java 0eecb3f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 2d6ad8f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java 243e282 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java c620bc6 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java bc2fc02 
> 
> Diff: https://reviews.apache.org/r/29824/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran ambari-server unit tests, all Java tests passing.  Some python failures, but these are due to a separate problem currently being addressed in the codebase. 
> 2. Manually verified that applying this patch will fix Falcon Blueprints deployments in single-node and multi-node clusters. 
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 29824: Re-introduce usage of excluded-configs API in Blueprint processor

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29824/#review67853
-----------------------------------------------------------

Ship it!


Ship It!

- Robert Levas


On Jan. 12, 2015, 4:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29824/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2015, 4:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Robert Levas, and Tom Beerbower.
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch implements a fix for AMBARI-9097.
> 
> Falcon deployments are not currently working with Blueprints.
> 
> The ClusterResourceProvider is not currently handling the case
>   of a Falcon Blueprints deployment, with respect to configuration
>   handling.
> 
> A portion of the changes from the fix for AMBARI-8009 originally
>   addressed this problem by adding some code to filter out the
>   "excluded" configuration references for a given stack service, since
>   only included config references should be used during a cluster
>   configuration update.
> 
> Some code refactorings in the patch for AMBARI-7175 moved the filtering
>   code to a separate location, and this eventually resulted in a
>   breakage, since some components seem to interpret "excluded"
>   configuration references differently.
> 
> To resolve this issue, this current patch implements the following:
> 
>  - Re-introduces some of the changes from AMBARI-8009 that were removed by
>    subsequent refactorings.  These changes modify the ClusterResourceProvider's
>    setConfigurationsOnCluster() method, so that excluded configuration
>    references are not included for a given service's config change
>    request.
> 
>  - Implements new unit tests to verify this change.
> 
>  - Updates some existing unit tests to reflect the changes to the Stack
>    class.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/StackServiceResponse.java 0eecb3f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 2d6ad8f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java 243e282 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java c620bc6 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java bc2fc02 
> 
> Diff: https://reviews.apache.org/r/29824/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran ambari-server unit tests, all Java tests passing.  Some python failures, but these are due to a separate problem currently being addressed in the codebase. 
> 2. Manually verified that applying this patch will fix Falcon Blueprints deployments in single-node and multi-node clusters. 
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 29824: Re-introduce usage of excluded-configs API in Blueprint processor

Posted by John Speidel <js...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29824/#review67866
-----------------------------------------------------------

Ship it!


Ship It!

- John Speidel


On Jan. 12, 2015, 9:55 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29824/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2015, 9:55 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Robert Levas, and Tom Beerbower.
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch implements a fix for AMBARI-9097.
> 
> Falcon deployments are not currently working with Blueprints.
> 
> The ClusterResourceProvider is not currently handling the case
>   of a Falcon Blueprints deployment, with respect to configuration
>   handling.
> 
> A portion of the changes from the fix for AMBARI-8009 originally
>   addressed this problem by adding some code to filter out the
>   "excluded" configuration references for a given stack service, since
>   only included config references should be used during a cluster
>   configuration update.
> 
> Some code refactorings in the patch for AMBARI-7175 moved the filtering
>   code to a separate location, and this eventually resulted in a
>   breakage, since some components seem to interpret "excluded"
>   configuration references differently.
> 
> To resolve this issue, this current patch implements the following:
> 
>  - Re-introduces some of the changes from AMBARI-8009 that were removed by
>    subsequent refactorings.  These changes modify the ClusterResourceProvider's
>    setConfigurationsOnCluster() method, so that excluded configuration
>    references are not included for a given service's config change
>    request.
> 
>  - Implements new unit tests to verify this change.
> 
>  - Updates some existing unit tests to reflect the changes to the Stack
>    class.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/StackServiceResponse.java 0eecb3f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 2d6ad8f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java 243e282 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java c620bc6 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java bc2fc02 
> 
> Diff: https://reviews.apache.org/r/29824/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran ambari-server unit tests, all Java tests passing.  Some python failures, but these are due to a separate problem currently being addressed in the codebase. 
> 2. Manually verified that applying this patch will fix Falcon Blueprints deployments in single-node and multi-node clusters. 
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>