You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Oliver Szabo <os...@hortonworks.com> on 2015/10/20 18:37:29 UTC

Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

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

Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}

I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 8f3869f 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.

> On Oct. 23, 2015, 4:56 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java, line 85
> > <https://reviews.apache.org/r/39481/diff/1/?file=1101930#file1101930line85>
> >
> >     How will the new dynamic model, introduced in 2.1.0, be supported here?  
> >     
> >     Since a Cluster Creation template can specify a predicate to use for matching host groups to servers as they come online, there needs to be a way to obtain recommendations for hosts as they are registered.  
> >     
> >     This question may be answered later in the code, so I'll remove this issue if that is the case.

good point. host names are not resolved here. so I relocated adviseConfiguration for this. thanks


- Oliver


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


On Oct. 26, 2015, 3:01 p.m., Oliver Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39481/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2015, 3:01 p.m.)
> 
> 
> Review request for Ambari, Robert Nettleton and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-13487
>     https://issues.apache.org/jira/browse/AMBARI-13487
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
> The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
> {
>   "blueprint" : "multi-node-conf-yarn",
>   "default_password" : "secret",
>   "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
>     "host_groups": [
>         {
>           "hosts": [
>               { "fqdn": "c6402.ambari.apache.org" }
>           ],
>           "name": "master",
>           "configurations" : [ ]
>         },
>         {
>           "hosts": [
>               { "fqdn": "c6403.ambari.apache.org" }
>           ],
>           "name": "slave_1",
>           "configurations" : [ ]
>         }
>     ],
>   "Clusters" : {"cluster_name":"cluster_name"}
> }
> Also tested with host count and predicate:
> ...
> "host_count" : ..,
> "host_predicate" : "...",
> ...
> I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
> As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)
> 
> question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 
> 
> Diff: https://reviews.apache.org/r/39481/diff/
> 
> 
> Testing
> -------
> 
> Unit tests passed:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 01:30 h
> [INFO] Finished at: 2015-10-20T16:23:54+00:00
> [INFO] Final Memory: 48M/687M
> [INFO] -----------------------------------------------------------------------
> 
> Functional tests: Manually
> (added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)
> 
> 
> Thanks,
> 
> Oliver Szabo
> 
>


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.

> On Oct. 23, 2015, 4:56 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, line 478
> > <https://reviews.apache.org/r/39481/diff/1/?file=1101932#file1101932line478>
> >
> >     Minor issue:
> >     
> >     Please reverse the ordering of the conditional here.
> >     
> >     Example:  if (valueAttrEntry.getValue().getDelete().equalsIgnoreCase("true"))

I often choose this ordering, because "true".equals(..getObject()) cannot throw nullpointer but ..getObject().equals("true") can


> On Oct. 23, 2015, 4:56 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, line 402
> > <https://reviews.apache.org/r/39481/diff/1/?file=1101932#file1101932line402>
> >
> >     Minor issue:
> >     
> >     I'd recommend using a left-to-right pattern for the conditionals.
> >     
> >     Example:
> >     
> >     if (configRecommendationStrategy.equals(ConfigRecommendationStrategy.ONLY_STACK_DEFAULTS_APPLY))

In this way how I did its nullsafe (but yes, it looks strange)


- Oliver


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


On Oct. 20, 2015, 4:37 p.m., Oliver Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39481/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2015, 4:37 p.m.)
> 
> 
> Review request for Ambari, Robert Nettleton and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-13487
>     https://issues.apache.org/jira/browse/AMBARI-13487
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
> The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
> {
>   "blueprint" : "multi-node-conf-yarn",
>   "default_password" : "secret",
>   "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
>     "host_groups": [
>         {
>           "hosts": [
>               { "fqdn": "c6402.ambari.apache.org" }
>           ],
>           "name": "master",
>           "configurations" : [ ]
>         },
>         {
>           "hosts": [
>               { "fqdn": "c6403.ambari.apache.org" }
>           ],
>           "name": "slave_1",
>           "configurations" : [ ]
>         }
>     ],
>   "Clusters" : {"cluster_name":"cluster_name"}
> }
> 
> I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
> As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)
> 
> question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 8f3869f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 
> 
> Diff: https://reviews.apache.org/r/39481/diff/
> 
> 
> Testing
> -------
> 
> Unit tests passed:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 01:30 h
> [INFO] Finished at: 2015-10-20T16:23:54+00:00
> [INFO] Final Memory: 48M/687M
> [INFO] -----------------------------------------------------------------------
> 
> Functional tests: Manually
> (added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)
> 
> 
> Thanks,
> 
> Oliver Szabo
> 
>


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/#review103776
-----------------------------------------------------------


Overall, I think the changes here look great.

There are a few minor issues that need to be resolved.

As I see it, there is only one major issue to be resolved.  I think that the approach towards filtering out the stack defaults should be reconsidered.  My comments mention that the Stack class provides a way to obtain the Configuration object associated with the Stack.  If this Configuration instance contains all stack defaults for the full Stack hierarchy (from the current version back), then I think this would be a more straightforward way to filter out the stack defaults vs. user-supplied configuration.  

I'm going to hold off on a +1 until this issue is re-visited, but I think the patch is in great shape.  I just think that it may be simpler and easier to maintain if we can skip calling the Configuration.getParent() method at multiple levels, since the underlying implementation may change someday.  

Thanks for the great work!


ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 50)
<https://reviews.apache.org/r/39481/#comment161856>

    Minor issue: 
    
    It appears that these strings are meant for the error-handling exceptions below, but they don't appear to be used.
    
    If my understanding is correct, then I'd recommend the following minor changes:
    
    1. Make these two static strings "final" as well
    2. Update the exceptions being created in adviseConfiguration() to use these constants.



ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 85)
<https://reviews.apache.org/r/39481/#comment161858>

    How will the new dynamic model, introduced in 2.1.0, be supported here?  
    
    Since a Cluster Creation template can specify a predicate to use for matching host groups to servers as they come online, there needs to be a way to obtain recommendations for hosts as they are registered.  
    
    This question may be answered later in the code, so I'll remove this issue if that is the case.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java (line 402)
<https://reviews.apache.org/r/39481/#comment161860>

    Minor issue:
    
    I'd recommend using a left-to-right pattern for the conditionals.
    
    Example:
    
    if (configRecommendationStrategy.equals(ConfigRecommendationStrategy.ONLY_STACK_DEFAULTS_APPLY))



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java (line 410)
<https://reviews.apache.org/r/39481/#comment161863>

    Minor issue: 
    
    It might be helpful to add the name of the config type being updated to this log statement.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java (line 420)
<https://reviews.apache.org/r/39481/#comment161866>

    Minor javadoc issue:
    
    Please change
    
    "what is not found..." 
    
    to
    
    "that is not found..."



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java (line 430)
<https://reviews.apache.org/r/39481/#comment161865>

    I think there might be a more straightforward way to do this, rather than rely on the Configuration hierarchy.  
    
    There might be cases in which using the parent 2 levels up might not work (host-group level configuration, for example).
    
    The following class for Stack processing:
    
    org.apache.ambari.server.controller.internal.Stack
    
    includes a "getConfiguration()" method, that appears to return a Configuration instance that only includes the Stack defaults for that Stack version (plus any defaults in the Stack hierarchy.  
    
    I think this method should be modified to use the Stack.getConfiguration() method, and then compare the cluster config with the stack config, in order to make the distinction between properties supplied by the Blueprint/Cluster Creation Template and stack defaults.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java (line 478)
<https://reviews.apache.org/r/39481/#comment161867>

    Minor issue:
    
    Please reverse the ordering of the conditional here.
    
    Example:  if (valueAttrEntry.getValue().getDelete().equalsIgnoreCase("true"))



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java (line 108)
<https://reviews.apache.org/r/39481/#comment161868>

    Minor issue:
    
    It might be good to declare this strategy as a "final" variable, since this strategy should be an invariant of the cluster deployment.



ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java (line 34)
<https://reviews.apache.org/r/39481/#comment161876>

    Minor javadoc change:
    
    Please change:
    
    "but no for..."
    
    to 
    
    "but not for..."



ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java (line 116)
<https://reviews.apache.org/r/39481/#comment161877>

    Minor issue:
    
    As in my comments above, I'd recommend reversing the ordering of this conditional, for the sake of readability.  
    
    Thanks.



ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java (line 5266)
<https://reviews.apache.org/r/39481/#comment161888>

    These tests look great.  
    
    I'd make a small request here:
    
    Maybe add a unit test to specifically test out the "NEVER_APPLY" case as well.
    
    While I realize that most other unit tests will verify this, I think it helps to explicitly test this mode as well. That will help with maintaining this code in the future, especially if new strategies are added, or if we decide to change the default strategy in the future.


- Robert Nettleton


On Oct. 20, 2015, 4:37 p.m., Oliver Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39481/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2015, 4:37 p.m.)
> 
> 
> Review request for Ambari, Robert Nettleton and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-13487
>     https://issues.apache.org/jira/browse/AMBARI-13487
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
> The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
> {
>   "blueprint" : "multi-node-conf-yarn",
>   "default_password" : "secret",
>   "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
>     "host_groups": [
>         {
>           "hosts": [
>               { "fqdn": "c6402.ambari.apache.org" }
>           ],
>           "name": "master",
>           "configurations" : [ ]
>         },
>         {
>           "hosts": [
>               { "fqdn": "c6403.ambari.apache.org" }
>           ],
>           "name": "slave_1",
>           "configurations" : [ ]
>         }
>     ],
>   "Clusters" : {"cluster_name":"cluster_name"}
> }
> 
> I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
> As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)
> 
> question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 8f3869f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 
> 
> Diff: https://reviews.apache.org/r/39481/diff/
> 
> 
> Testing
> -------
> 
> Unit tests passed:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 01:30 h
> [INFO] Finished at: 2015-10-20T16:23:54+00:00
> [INFO] Final Memory: 48M/687M
> [INFO] -----------------------------------------------------------------------
> 
> Functional tests: Manually
> (added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)
> 
> 
> Thanks,
> 
> Oliver Szabo
> 
>


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/#review104297
-----------------------------------------------------------

Ship it!


Looks great overall. 

Just a minor javadoc/commenting issue that should be fixed before we push this patch. 

Thanks!


ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java (line 114)
<https://reviews.apache.org/r/39481/#comment162557>

    Minor issue:
    
    It looks like a comment was overwritten by this patch.
    
    I'd recommend re-introducing this comment in the relevant location. 
    
    Thanks.


- Robert Nettleton


On Oct. 27, 2015, 3:17 p.m., Oliver Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39481/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2015, 3:17 p.m.)
> 
> 
> Review request for Ambari, Robert Nettleton and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-13487
>     https://issues.apache.org/jira/browse/AMBARI-13487
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
> The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
> {
>   "blueprint" : "multi-node-conf-yarn",
>   "default_password" : "secret",
>   "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
>     "host_groups": [
>         {
>           "hosts": [
>               { "fqdn": "c6402.ambari.apache.org" }
>           ],
>           "name": "master",
>           "configurations" : [ ]
>         },
>         {
>           "hosts": [
>               { "fqdn": "c6403.ambari.apache.org" }
>           ],
>           "name": "slave_1",
>           "configurations" : [ ]
>         }
>     ],
>   "Clusters" : {"cluster_name":"cluster_name"}
> }
> Also tested with host count and predicate:
> ...
> "host_count" : ..,
> "host_predicate" : "...",
> ...
> I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
> As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)
> 
> question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 
> 
> Diff: https://reviews.apache.org/r/39481/diff/
> 
> 
> Testing
> -------
> 
> Unit tests passed:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 01:30 h
> [INFO] Finished at: 2015-10-20T16:23:54+00:00
> [INFO] Final Memory: 48M/687M
> [INFO] -----------------------------------------------------------------------
> 
> Functional tests: Manually
> (added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)
> 
> 
> Thanks,
> 
> Oliver Szabo
> 
>


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

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

Ship it!


Ship It!

- Robert Levas


On Oct. 29, 2015, 1:10 p.m., Oliver Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39481/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2015, 1:10 p.m.)
> 
> 
> Review request for Ambari, Robert Nettleton and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-13487
>     https://issues.apache.org/jira/browse/AMBARI-13487
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
> The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
> {
>   "blueprint" : "multi-node-conf-yarn",
>   "default_password" : "secret",
>   "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
>     "host_groups": [
>         {
>           "hosts": [
>               { "fqdn": "c6402.ambari.apache.org" }
>           ],
>           "name": "master",
>           "configurations" : [ ]
>         },
>         {
>           "hosts": [
>               { "fqdn": "c6403.ambari.apache.org" }
>           ],
>           "name": "slave_1",
>           "configurations" : [ ]
>         }
>     ],
>   "Clusters" : {"cluster_name":"cluster_name"}
> }
> Also tested with host count and predicate:
> ...
> "host_count" : ..,
> "host_predicate" : "...",
> ...
> I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
> As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)
> 
> question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 9044e37 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java d3d95bd 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java cd28aac 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 9716abe 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 26d8c3e 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java bd4f13d 
> 
> Diff: https://reviews.apache.org/r/39481/diff/
> 
> 
> Testing
> -------
> 
> Unit tests passed:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 01:30 h
> [INFO] Finished at: 2015-10-20T16:23:54+00:00
> [INFO] Final Memory: 48M/687M
> [INFO] -----------------------------------------------------------------------
> 
> Functional tests: Manually
> (added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)
> 
> 
> Thanks,
> 
> Oliver Szabo
> 
>


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 29, 2015, 5:10 p.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Changes
-------

fixed conflicts for trunk


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}
Also tested with host count and predicate:
...
"host_count" : ..,
"host_predicate" : "...",
...
I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 9044e37 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java d3d95bd 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java cd28aac 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 9716abe 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 26d8c3e 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java bd4f13d 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 28, 2015, 5:47 p.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Changes
-------

- fixed comment
- added configurations input for stack advisor (and other ones too, despite of they are all needed or not for configuration recommendation)


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}
Also tested with host count and predicate:
...
"host_count" : ..,
"host_predicate" : "...",
...
I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 27, 2015, 3:17 p.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Changes
-------

fixed licenses and imports


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}
Also tested with host count and predicate:
...
"host_count" : ..,
"host_predicate" : "...",
...
I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 27, 2015, 11:18 a.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Changes
-------

- set configRecommendationStrategy to final in ProvisionClusterRequest


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}
Also tested with host count and predicate:
...
"host_count" : ..,
"host_predicate" : "...",
...
I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 26, 2015, 3:20 p.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Changes
-------

removed import


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}
Also tested with host count and predicate:
...
"host_count" : ..,
"host_predicate" : "...",
...
I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 26, 2015, 3:01 p.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Changes
-------

added missing files


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}
Also tested with host count and predicate:
...
"host_count" : ..,
"host_predicate" : "...",
...
I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 26, 2015, 2:32 p.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description (updated)
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}
Also tested with host count and predicate:
...
"host_count" : ..,
"host_predicate" : "...",
...
I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo


Re: Review Request 39481: Basic StackAdvisor support for blueprint (configurations)

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39481/
-----------------------------------------------------------

(Updated Oct. 26, 2015, 1:08 p.m.)


Review request for Ambari, Robert Nettleton and Sumit Mohanty.


Changes
-------

- obtaining stack defaults from Stack object
- relocate adviseConfiguration method call -> for working with host predicate + host count
- added test for NEVER_APPLY strategy
- fixed some messages
- host group support ??? -> I thinking of that: call stackAdvisor multiple times with different hosts input (per host group) but im not sure it would be the best solution. maybe the response of the stack advisor should change for this.


Bugs: AMBARI-13487
    https://issues.apache.org/jira/browse/AMBARI-13487


Repository: ambari


Description
-------

During update configuration for cluster creation, configurations are upgraded based on response of the StackAdvisor.
The advised configurations will be added to cluster topology, and used during BlueprintConfigurationProcessor#doUpdateForClusterCreate(). The advised configurations filtered out based on ConfigRecommendationStrategy, which can be set in the Cluster Creation Template, e.g.:
{
  "blueprint" : "multi-node-conf-yarn",
  "default_password" : "secret",
  "config_recommendation_strategy" : "ONLY_STACK_DEFAULTS_APPLY",
    "host_groups": [
        {
          "hosts": [
              { "fqdn": "c6402.ambari.apache.org" }
          ],
          "name": "master",
          "configurations" : [ ]
        },
        {
          "hosts": [
              { "fqdn": "c6403.ambari.apache.org" }
          ],
          "name": "slave_1",
          "configurations" : [ ]
        }
    ],
  "Clusters" : {"cluster_name":"cluster_name"}
}

I added the configuration modifications BEFORE the basic updates happen in BlueprintConfigurationProcessor.
As I see stackAdvisor not supports host group scoped configurations, so as my changes. I made a new AdvisedConfiguration which has a Map field for properties, this field can be a Configuration object in the future (if property attributes will be supported..because they are not supported too)

question: is that the correct way to geather stack defaults in BlueprintConfigurationProcessor#doFilterStackDefaults() ?


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 5bc0962 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 18fca8a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java e2f132e 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 4a906b1 
  ambari-server/src/main/java/org/apache/ambari/server/topology/AdvisedConfiguration.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java 1ebde17 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 284a913 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 5b716ae 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupConfigType.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 64be609 
  ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 34b72b6 
  ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 8eeb54c 

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


Testing
-------

Unit tests passed:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 h
[INFO] Finished at: 2015-10-20T16:23:54+00:00
[INFO] Final Memory: 48M/687M
[INFO] -----------------------------------------------------------------------

Functional tests: Manually
(added "config_recommendation_strategy" field to the cluster creation template: tried 4 variations: field is missing, and set the field to NEVER_APPLY, ALWAYS_APPLY and ONLY_STACK_DEFAULTS_APPLY)


Thanks,

Oliver Szabo