You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Madhuvanthi Radhakrishnan <mr...@hortonworks.com> on 2017/02/08 00:52:37 UTC
Review Request 56418: Export Blueprints does not contain the settings
object and hence the credential store values
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/
-----------------------------------------------------------
Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
Bugs: AMBARI-19909
https://issues.apache.org/jira/browse/AMBARI-19909
Repository: ambari
Description
-------
Export Blueprints does not contain the settings object and hence the credential store values
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
Diff: https://reviews.apache.org/r/56418/diff/
Testing
-------
Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
Resultant blueprint has the following object added:
"settings" : [
{
"recovery_settings" : [
{
"recovery_enabled" : "true"
}
]
},
{
"service_settings" : [
{
"name" : "OOZIE",
"credential_store_enabled" : "true"
},
{
"recovery_enabled" : "true",
"name" : "HIVE",
"credential_store_enabled" : "true"
},
{
"recovery_enabled" : "true",
"name" : "HDFS"
},
{
"recovery_enabled" : "true",
"name" : "AMBARI_METRICS"
}
]
},
{
"component_settings" : [
{
"recovery_enabled" : "true",
"name" : "METRICS_COLLECTOR"
},
{
"recovery_enabled" : "true",
"name" : "SECONDARY_NAMENODE"
},
{
"recovery_enabled" : "true",
"name" : "WEBHCAT_SERVER"
}
]
}
]
Thanks,
Madhuvanthi Radhakrishnan
Re: Review Request 56418: Export Blueprints does not contain the
settings object and hence the credential store values
Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/#review164728
-----------------------------------------------------------
Fix it, then Ship it!
The code changes in this patch look fine to me, but I'd like to request two things:
1. Unit tests for this change be added to the patch
2. Some additional manual testing, mentioned below, to verify that the exported Blueprint will deploy properly.
Thanks for providing this patch!
ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java (line 75)
<https://reviews.apache.org/r/56418/#comment236481>
Overall, the changes look fine to me, but could you please add some unit tests for this change?
If you could add some tests to the following:
org.apache.ambari.server.api.query.render.ClusterBlueprintRendererTest
that would be great.
Thanks.
ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java (line 76)
<https://reviews.apache.org/r/56418/#comment236482>
Could you also please try out some manual tests to use the exported Blueprint to create a new cluster?
When changes to the export renderer are made, its always a good thing to try to make sure that the exported Blueprint is valid, and can be used for a successful cluster creation.
Thanks.
- Robert Nettleton
On Feb. 8, 2017, 12:52 a.m., Madhuvanthi Radhakrishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2017, 12:52 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
>
>
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Export Blueprints does not contain the settings object and hence the credential store values
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
>
> Diff: https://reviews.apache.org/r/56418/diff/
>
>
> Testing
> -------
>
> Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
> "recovery_settings" : [
> {
> "recovery_enabled" : "true"
> }
> ]
> },
> {
> "service_settings" : [
> {
> "name" : "OOZIE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HIVE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HDFS"
> },
> {
> "recovery_enabled" : "true",
> "name" : "AMBARI_METRICS"
> }
> ]
> },
> {
> "component_settings" : [
> {
> "recovery_enabled" : "true",
> "name" : "METRICS_COLLECTOR"
> },
> {
> "recovery_enabled" : "true",
> "name" : "SECONDARY_NAMENODE"
> },
> {
> "recovery_enabled" : "true",
> "name" : "WEBHCAT_SERVER"
> }
> ]
> }
> ]
>
>
> Thanks,
>
> Madhuvanthi Radhakrishnan
>
>
Re: Review Request 56418: Export Blueprints does not contain the
settings object and hence the credential store values
Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/#review165585
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Nettleton
On Feb. 9, 2017, 11:14 p.m., Madhuvanthi Radhakrishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2017, 11:14 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
>
>
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Export Blueprints does not contain the settings object and hence the credential store values
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 1fe48df
>
> Diff: https://reviews.apache.org/r/56418/diff/
>
>
> Testing
> -------
>
> Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
> "recovery_settings" : [
> {
> "recovery_enabled" : "true"
> }
> ]
> },
> {
> "service_settings" : [
> {
> "name" : "OOZIE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HIVE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HDFS"
> },
> {
> "recovery_enabled" : "true",
> "name" : "AMBARI_METRICS"
> }
> ]
> },
> {
> "component_settings" : [
> {
> "recovery_enabled" : "true",
> "name" : "METRICS_COLLECTOR"
> },
> {
> "recovery_enabled" : "true",
> "name" : "SECONDARY_NAMENODE"
> },
> {
> "recovery_enabled" : "true",
> "name" : "WEBHCAT_SERVER"
> }
> ]
> }
> ]
>
>
> Thanks,
>
> Madhuvanthi Radhakrishnan
>
>
Re: Review Request 56418: Export Blueprints does not contain the
settings object and hence the credential store values
Posted by Sumit Mohanty <sm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/#review165057
-----------------------------------------------------------
Ship it!
Ship It!
- Sumit Mohanty
On Feb. 9, 2017, 11:14 p.m., Madhuvanthi Radhakrishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2017, 11:14 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
>
>
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Export Blueprints does not contain the settings object and hence the credential store values
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 1fe48df
>
> Diff: https://reviews.apache.org/r/56418/diff/
>
>
> Testing
> -------
>
> Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
> "recovery_settings" : [
> {
> "recovery_enabled" : "true"
> }
> ]
> },
> {
> "service_settings" : [
> {
> "name" : "OOZIE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HIVE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HDFS"
> },
> {
> "recovery_enabled" : "true",
> "name" : "AMBARI_METRICS"
> }
> ]
> },
> {
> "component_settings" : [
> {
> "recovery_enabled" : "true",
> "name" : "METRICS_COLLECTOR"
> },
> {
> "recovery_enabled" : "true",
> "name" : "SECONDARY_NAMENODE"
> },
> {
> "recovery_enabled" : "true",
> "name" : "WEBHCAT_SERVER"
> }
> ]
> }
> ]
>
>
> Thanks,
>
> Madhuvanthi Radhakrishnan
>
>
Re: Review Request 56418: Export Blueprints does not contain the
settings object and hence the credential store values
Posted by Jayush Luniya <jl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/#review165066
-----------------------------------------------------------
Ship it!
Ship It!
- Jayush Luniya
On Feb. 9, 2017, 11:14 p.m., Madhuvanthi Radhakrishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2017, 11:14 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
>
>
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Export Blueprints does not contain the settings object and hence the credential store values
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 1fe48df
>
> Diff: https://reviews.apache.org/r/56418/diff/
>
>
> Testing
> -------
>
> Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
> "recovery_settings" : [
> {
> "recovery_enabled" : "true"
> }
> ]
> },
> {
> "service_settings" : [
> {
> "name" : "OOZIE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HIVE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HDFS"
> },
> {
> "recovery_enabled" : "true",
> "name" : "AMBARI_METRICS"
> }
> ]
> },
> {
> "component_settings" : [
> {
> "recovery_enabled" : "true",
> "name" : "METRICS_COLLECTOR"
> },
> {
> "recovery_enabled" : "true",
> "name" : "SECONDARY_NAMENODE"
> },
> {
> "recovery_enabled" : "true",
> "name" : "WEBHCAT_SERVER"
> }
> ]
> }
> ]
>
>
> Thanks,
>
> Madhuvanthi Radhakrishnan
>
>
Re: Review Request 56418: Export Blueprints does not contain the
settings object and hence the credential store values
Posted by Madhuvanthi Radhakrishnan <mr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/
-----------------------------------------------------------
(Updated Feb. 9, 2017, 11:14 p.m.)
Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
Changes
-------
Incorporating Review Comments - performance improvement, unit tests
Bugs: AMBARI-19909
https://issues.apache.org/jira/browse/AMBARI-19909
Repository: ambari
Description
-------
Export Blueprints does not contain the settings object and hence the credential store values
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 1fe48df
Diff: https://reviews.apache.org/r/56418/diff/
Testing
-------
Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
Resultant blueprint has the following object added:
"settings" : [
{
"recovery_settings" : [
{
"recovery_enabled" : "true"
}
]
},
{
"service_settings" : [
{
"name" : "OOZIE",
"credential_store_enabled" : "true"
},
{
"recovery_enabled" : "true",
"name" : "HIVE",
"credential_store_enabled" : "true"
},
{
"recovery_enabled" : "true",
"name" : "HDFS"
},
{
"recovery_enabled" : "true",
"name" : "AMBARI_METRICS"
}
]
},
{
"component_settings" : [
{
"recovery_enabled" : "true",
"name" : "METRICS_COLLECTOR"
},
{
"recovery_enabled" : "true",
"name" : "SECONDARY_NAMENODE"
},
{
"recovery_enabled" : "true",
"name" : "WEBHCAT_SERVER"
}
]
}
]
Thanks,
Madhuvanthi Radhakrishnan
Re: Review Request 56418: Export Blueprints does not contain the
settings object and hence the credential store values
Posted by Jayush Luniya <jl...@hortonworks.com>.
> On Feb. 9, 2017, 3:29 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java, line 136
> > <https://reviews.apache.org/r/56418/diff/1/?file=1627089#file1627089line136>
> >
> > Madhu,
> > As we discussed, it looks like getting the entire ServiceComponentInfo is expensive. If I query /api/v1/clusters/<cluster-name>/services/<service-name>/components/<component-name> it takes a long time. When exporting blueprint since we will be loading all service components we will have significant perf impact. Lets add filters to only get required fields for ServiceComponent
> >
> > serviceComponentNode.getObject().add("ServiceComponentInfo/cluster_name");
> > serviceComponentNode.getObject().add("ServiceComponentInfo/service_name");
> > serviceComponentNode.getObject().add("ServiceComponentInfo/component_name");
> > serviceComponentNode.getObject().add("ServiceComponentInfo/recovery_enabled");
My suspicion is that the metrics node is taking a long time to load. Will follow up on that with Sid.
- Jayush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/#review164829
-----------------------------------------------------------
On Feb. 8, 2017, 12:52 a.m., Madhuvanthi Radhakrishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2017, 12:52 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
>
>
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Export Blueprints does not contain the settings object and hence the credential store values
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
>
> Diff: https://reviews.apache.org/r/56418/diff/
>
>
> Testing
> -------
>
> Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
> "recovery_settings" : [
> {
> "recovery_enabled" : "true"
> }
> ]
> },
> {
> "service_settings" : [
> {
> "name" : "OOZIE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HIVE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HDFS"
> },
> {
> "recovery_enabled" : "true",
> "name" : "AMBARI_METRICS"
> }
> ]
> },
> {
> "component_settings" : [
> {
> "recovery_enabled" : "true",
> "name" : "METRICS_COLLECTOR"
> },
> {
> "recovery_enabled" : "true",
> "name" : "SECONDARY_NAMENODE"
> },
> {
> "recovery_enabled" : "true",
> "name" : "WEBHCAT_SERVER"
> }
> ]
> }
> ]
>
>
> Thanks,
>
> Madhuvanthi Radhakrishnan
>
>
Re: Review Request 56418: Export Blueprints does not contain the
settings object and hence the credential store values
Posted by Jayush Luniya <jl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56418/#review164829
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java (line 136)
<https://reviews.apache.org/r/56418/#comment236621>
Madhu,
As we discussed, it looks like getting the entire ServiceComponentInfo is expensive. If I query /api/v1/clusters/<cluster-name>/services/<service-name>/components/<component-name> it takes a long time. When exporting blueprint since we will be loading all service components we will have significant perf impact. Lets add filters to only get required fields for ServiceComponent
serviceComponentNode.getObject().add("ServiceComponentInfo/cluster_name");
serviceComponentNode.getObject().add("ServiceComponentInfo/service_name");
serviceComponentNode.getObject().add("ServiceComponentInfo/component_name");
serviceComponentNode.getObject().add("ServiceComponentInfo/recovery_enabled");
- Jayush Luniya
On Feb. 8, 2017, 12:52 a.m., Madhuvanthi Radhakrishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56418/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2017, 12:52 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert Nettleton, and Sumit Mohanty.
>
>
> Bugs: AMBARI-19909
> https://issues.apache.org/jira/browse/AMBARI-19909
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Export Blueprints does not contain the settings object and hence the credential store values
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 1a9ea91
>
> Diff: https://reviews.apache.org/r/56418/diff/
>
>
> Testing
> -------
>
> Issued http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint command to ensure that the settings object is present.
> Resultant blueprint has the following object added:
> "settings" : [
> {
> "recovery_settings" : [
> {
> "recovery_enabled" : "true"
> }
> ]
> },
> {
> "service_settings" : [
> {
> "name" : "OOZIE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HIVE",
> "credential_store_enabled" : "true"
> },
> {
> "recovery_enabled" : "true",
> "name" : "HDFS"
> },
> {
> "recovery_enabled" : "true",
> "name" : "AMBARI_METRICS"
> }
> ]
> },
> {
> "component_settings" : [
> {
> "recovery_enabled" : "true",
> "name" : "METRICS_COLLECTOR"
> },
> {
> "recovery_enabled" : "true",
> "name" : "SECONDARY_NAMENODE"
> },
> {
> "recovery_enabled" : "true",
> "name" : "WEBHCAT_SERVER"
> }
> ]
> }
> ]
>
>
> Thanks,
>
> Madhuvanthi Radhakrishnan
>
>