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 2016/01/19 16:06:15 UTC
Review Request 42501: Blueprints: Stack advisor should consider user
provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/
-----------------------------------------------------------
Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
Bugs: AMBARI-14722
https://issues.apache.org/jira/browse/AMBARI-14722
Repository: ambari
Description
-------
If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
(ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
Diff: https://reviews.apache.org/r/42501/diff/
Testing
-------
FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
Unit testing is in progress...
Thanks,
Oliver Szabo
Re: Review Request 42501: Blueprints: Stack advisor should consider
user provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
Posted by Oliver Szabo <os...@hortonworks.com>.
> On Jan. 19, 2016, 4:39 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java, line 185
> > <https://reviews.apache.org/r/42501/diff/2/?file=1201709#file1201709line185>
> >
> > This should work fine, but I'd recommend looking into using the hierarchy defined by the Configuration object in Blueprints.
> >
> > I believe that using:
> >
> > blueprint.getConfiguration().getProperties()
> >
> > will return just the config entries defined in the Blueprint, as the stack defaults would be included in the parent hierarchy, which would be returned by getConfiguration().getFullProperties(), which returns all configuration (stack defaults + blueprint overrides).
> >
> > If the hierarchy is still setup as I understand it, just getting the first level of configuration from the Configuration object associated with the Blueprint should be enough to make this work.
True, but it does not contain the configurations from the cluster creation template. In that case Cluster creation template related configurations located in the lowest level of the hierarchy. Im not sure it can change in the future, but for me its a more clear solution just gather properties without the unmodified stack defaults.
- Oliver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/#review115179
-----------------------------------------------------------
On Jan. 19, 2016, 4:20 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42501/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 4:20 p.m.)
>
>
> Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-14722
> https://issues.apache.org/jira/browse/AMBARI-14722
>
>
> Repository: ambari
>
>
> Description
> -------
>
> If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
> In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
> (ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
>
> Diff: https://reviews.apache.org/r/42501/diff/
>
>
> Testing
> -------
>
> FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
> Unit testing done.
>
> Total run:831
> Total errors:0
> Total failures:0
> OK
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:00:46.271s
> [INFO] Finished at: Tue Jan 19 16:55:58 CET 2016
> [INFO] Final Memory: 39M/1277M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 42501: Blueprints: Stack advisor should consider
user provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
Posted by Robert Nettleton <rn...@hortonworks.com>.
> On Jan. 19, 2016, 4:39 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java, line 185
> > <https://reviews.apache.org/r/42501/diff/2/?file=1201709#file1201709line185>
> >
> > This should work fine, but I'd recommend looking into using the hierarchy defined by the Configuration object in Blueprints.
> >
> > I believe that using:
> >
> > blueprint.getConfiguration().getProperties()
> >
> > will return just the config entries defined in the Blueprint, as the stack defaults would be included in the parent hierarchy, which would be returned by getConfiguration().getFullProperties(), which returns all configuration (stack defaults + blueprint overrides).
> >
> > If the hierarchy is still setup as I understand it, just getting the first level of configuration from the Configuration object associated with the Blueprint should be enough to make this work.
>
> Oliver Szabo wrote:
> True, but it does not contain the configurations from the cluster creation template. In that case Cluster creation template related configurations located in the lowest level of the hierarchy. Im not sure it can change in the future, but for me its a more clear solution just gather properties without the unmodified stack defaults.
Upon further review of this area of the code, I agree with the patch submitter's last comment. The Configuration hierachy could potentially be used for things like this in the future, but currently the APIs do not provide a way just to obtain the config in the Blueprint and Cluster Creation template. The hierarchy can change, based on things like host group-level configuration.
While I do think the Configuration objects in Blueprints should be refactored to make this simpler, that should probably occur in a separate patch, so I'll drop my issue and approve this patch.
Thanks.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/#review115179
-----------------------------------------------------------
On Jan. 19, 2016, 4:20 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42501/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 4:20 p.m.)
>
>
> Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-14722
> https://issues.apache.org/jira/browse/AMBARI-14722
>
>
> Repository: ambari
>
>
> Description
> -------
>
> If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
> In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
> (ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
>
> Diff: https://reviews.apache.org/r/42501/diff/
>
>
> Testing
> -------
>
> FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
> Unit testing done.
>
> Total run:831
> Total errors:0
> Total failures:0
> OK
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:00:46.271s
> [INFO] Finished at: Tue Jan 19 16:55:58 CET 2016
> [INFO] Final Memory: 39M/1277M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 42501: Blueprints: Stack advisor should consider
user provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/#review115179
-----------------------------------------------------------
While the patch should work as-is, I'd recommend considering the use of the Configuration hierarchy to achieve the same result. I believe this will result in a simpler change that is easier to maintain over time.
Thanks.
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 185)
<https://reviews.apache.org/r/42501/#comment176076>
This should work fine, but I'd recommend looking into using the hierarchy defined by the Configuration object in Blueprints.
I believe that using:
blueprint.getConfiguration().getProperties()
will return just the config entries defined in the Blueprint, as the stack defaults would be included in the parent hierarchy, which would be returned by getConfiguration().getFullProperties(), which returns all configuration (stack defaults + blueprint overrides).
If the hierarchy is still setup as I understand it, just getting the first level of configuration from the Configuration object associated with the Blueprint should be enough to make this work.
- Robert Nettleton
On Jan. 19, 2016, 4:20 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42501/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 4:20 p.m.)
>
>
> Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-14722
> https://issues.apache.org/jira/browse/AMBARI-14722
>
>
> Repository: ambari
>
>
> Description
> -------
>
> If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
> In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
> (ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
>
> Diff: https://reviews.apache.org/r/42501/diff/
>
>
> Testing
> -------
>
> FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
> Unit testing done.
>
> Total run:831
> Total errors:0
> Total failures:0
> OK
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:00:46.271s
> [INFO] Finished at: Tue Jan 19 16:55:58 CET 2016
> [INFO] Final Memory: 39M/1277M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 42501: Blueprints: Stack advisor should consider
user provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/#review115199
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Nettleton
On Jan. 19, 2016, 4:20 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42501/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 4:20 p.m.)
>
>
> Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-14722
> https://issues.apache.org/jira/browse/AMBARI-14722
>
>
> Repository: ambari
>
>
> Description
> -------
>
> If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
> In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
> (ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
>
> Diff: https://reviews.apache.org/r/42501/diff/
>
>
> Testing
> -------
>
> FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
> Unit testing done.
>
> Total run:831
> Total errors:0
> Total failures:0
> OK
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:00:46.271s
> [INFO] Finished at: Tue Jan 19 16:55:58 CET 2016
> [INFO] Final Memory: 39M/1277M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 42501: Blueprints: Stack advisor should consider
user provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/
-----------------------------------------------------------
(Updated Jan. 19, 2016, 4:20 p.m.)
Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
Bugs: AMBARI-14722
https://issues.apache.org/jira/browse/AMBARI-14722
Repository: ambari
Description
-------
If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
(ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
Diff: https://reviews.apache.org/r/42501/diff/
Testing (updated)
-------
FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
Unit testing done.
Total run:831
Total errors:0
Total failures:0
OK
INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
INFO: Return code from stack upgrade command, retcode = 0
StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
Returning DefaultStackAdvisor implementation
StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
Returning XYZ101StackAdvisor implementation
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1:00:46.271s
[INFO] Finished at: Tue Jan 19 16:55:58 CET 2016
[INFO] Final Memory: 39M/1277M
[INFO] ------------------------------------------------------------------------
Thanks,
Oliver Szabo
Re: Review Request 42501: Blueprints: Stack advisor should consider
user provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/#review115174
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Levas
On Jan. 19, 2016, 10:06 a.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42501/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 10:06 a.m.)
>
>
> Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-14722
> https://issues.apache.org/jira/browse/AMBARI-14722
>
>
> Repository: ambari
>
>
> Description
> -------
>
> If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
> In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
> (ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
>
> Diff: https://reviews.apache.org/r/42501/diff/
>
>
> Testing
> -------
>
> FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
> Unit testing is in progress...
>
>
> Thanks,
>
> Oliver Szabo
>
>
Re: Review Request 42501: Blueprints: Stack advisor should consider
user provided inputs on ''ONLY_STACK_DEFAULTS_APPLY" strategy
Posted by Sebastian Toader <st...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42501/#review115171
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 182)
<https://reviews.apache.org/r/42501/#comment176063>
Add comment what this method does.
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 183)
<https://reviews.apache.org/r/42501/#comment176062>
Why do we need this to be a ConcurentMap ?
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 195)
<https://reviews.apache.org/r/42501/#comment176064>
Maybe it's better to name it defaultValue or configPropDefaultValue.
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 200)
<https://reviews.apache.org/r/42501/#comment176065>
This "else" seems wrong. It adds the same configProp value as the "if" branch above.
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 211)
<https://reviews.apache.org/r/42501/#comment176067>
Add comment to the method header.
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java (line 214)
<https://reviews.apache.org/r/42501/#comment176066>
Use rather "==" as that's null safe.
- Sebastian Toader
On Jan. 19, 2016, 4:06 p.m., Oliver Szabo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42501/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 4:06 p.m.)
>
>
> Review request for Ambari, Robert Levas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-14722
> https://issues.apache.org/jira/browse/AMBARI-14722
>
>
> Repository: ambari
>
>
> Description
> -------
>
> If the config_recommendation_strategy is 'ONLY_STACK_DEFAULTS_APPLY' in the cluster creation template, it should apply configurations only on stack defaults.
> In case of a user overrides a stack default property, stack advisor output should not be considered, and force to use the user provided property.
> (ALWAYS_APPLY strategy still won't use user provided properties in the output of stack advisor, only as inputs)
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java 337ad06
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 60a8dde
>
> Diff: https://reviews.apache.org/r/42501/diff/
>
>
> Testing
> -------
>
> FT: provision with multiple blueprints (with user provided properties in the cluster-creation tempalte and in the blueprint too)
> Unit testing is in progress...
>
>
> Thanks,
>
> Oliver Szabo
>
>