Re: Review Request 64397: AMBARI-22602. Add 'clusterSettings' and 'stackSettings' parameters in Execution Command.

2017-12-11 Thread Swapan Shridhar 

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

(Updated Dec. 12, 2017, 4:07 a.m.)


Review request for Ambari, Attila Doroszlai, Jayush Luniya, and Madhuvanthi 
Radhakrishnan.


Changes
---

Code updated as per review comments.


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


Repository: ambari


Description
---

AMBARI-22198 added "stack settings", and AMBARI-22196 introduced "cluster 
settings" in Ambari.

This review adds 2 new parameters to Execution Command : **(1).** 
clusterSettings and **(2).** stackSettings, and adds their respective set of 
key-values pairs.

- This enables these parameters to be passed in to the ambari-agent's 
**command*.json**.


Test cases are tracked in : AMBARI-22603


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
 5ee4bf6 
  
ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java
 fd43edf 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
 e7dea06 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 59e6622 
  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
 9043297 
  
ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
 8bd24f2 
  ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
c8768dd 
  
ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
 0b38b36 


Diff: https://reviews.apache.org/r/64397/diff/2/

Changes: https://reviews.apache.org/r/64397/diff/1-2/


Testing
---

Tested on Live cluster doing following : 
(1). Start Service 
(2). Restart Service 
(3). Stop Service 
(4). Refreshing client configs
(5). Adding Client.


**Snippet from command*.json as part of testing:**
{
...
...
"clusterSettings": {
"security_enabled": "false",
"hide_yarn_memory_widget": "false",
"enable_external_ranger": "false",
"override_uid": "true",
"kerberos_domain": "EXAMPLE.COM",
"one_dir_per_partition": "false",
"repo_ubuntu_template": "{{package_type}} {{base_url}} {{components}}",
"ignore_groupsusers_create": "false",
"alerts_repeat_tolerance": "1",
"namenode_rolling_restart_timeout": "4200",
"fetch_nonlocal_groups": "true",
"manage_dirs_on_root": "true",
"recovery_lifetime_max_count": "1024",
"agent_mounts_ignore_list": "",
"ignore_bad_mounts": "false",
"recovery_window_in_minutes": "60",
"sysprep_skip_copy_tarballs_hdfs": "false",
"recovery_type": "AUTO_START",
"user_group": "hadoop",
"namenode_rolling_restart_safemode_exit_timeout": "3600",
"recovery_retry_interval": "5",
"sysprep_skip_copy_oozie_share_lib_to_hdfs": "false",
"sysprep_skip_setup_jce": "false",
"manage_hive_fsroot": "true",
"service_check_type": "full",
"recovery_enabled": "true",
"recovery_max_count": "6",
"sysprep_skip_create_users_and_groups": "false",
"smokeuser_keytab": "/etc/security/keytabs/smokeuser.headless.keytab",
"managed_hdfs_resource_property_names": "false",
"smokeuser": "ambari-qa",
"sysprep_skip_copy_fast_jar_hdfs": "false"
},
...
...
...
...
...
...
...
"stackSettings": {
"stack_features": "{\n  \"HDP\": {\n\"stack_features\": [\n  
{\n\"name\": \"snappy\",\n\"description\": \"Snappy 
compressor/decompressor support\",\n\"min_version\": \"2.0.0.0\",\n 
   \"max_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
\"lzo\",\n\"description\": \"LZO libraries support\",\n
\"min_version\": \"2.2.1.0\"\n  },\n  {\n\"name\": 
\"express_upgrade\",\n\"description\": \"Express upgrade support\",\n   
 \"min_version\": \"2.1.0.0\"\n  },\n  {\n\"name\": 
\"rolling_upgrade\",\n\"description\": \"Rolling upgrade support\",\n   
 \"min_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
\"kafka_acl_migration_support\",\n\"description\": \"ACL migration 
support\",\n\"min_version\": \"2.3.4.0\"\n  },\n  {\n
\"name\": \"secure_zookeeper\",\n\"description\": \"Protect ZNodes with 
SASL acl in secure clusters\"
 ,\n\"min_version\": \"2.6.0.0\"\n  },\n  {\n\"name\": 
\"config_versioning\",\n\"description\": \"Configurable versions 
support\",\n\"min_version\": \"2.3.0.0\"\n  },\n  {\n
\"name\": \"datanode_non_root\",\n\"description\":

Re: Review Request 64397: AMBARI-22602. Add 'clusterSettings' and 'stackSettings' parameters in Execution Command.

2017-12-11 Thread Swapan Shridhar 


> On Dec. 11, 2017, 5:44 p.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
> > Lines 291 (patched)
> > 
> >
> > settings or clusterSettings instead of params?

Fixed.


> On Dec. 11, 2017, 5:44 p.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
> > Lines 300 (patched)
> > 
> >
> > settings or stackSettings instead of params?

Fixed.


- Swapan


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


On Dec. 7, 2017, 8:11 a.m., Swapan Shridhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64397/
> ---
> 
> (Updated Dec. 7, 2017, 8:11 a.m.)
> 
> 
> Review request for Ambari, Jayush Luniya and Madhuvanthi Radhakrishnan.
> 
> 
> Bugs: AMBARI-22602
> https://issues.apache.org/jira/browse/AMBARI-22602
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> AMBARI-22198 added "stack settings", and AMBARI-22196 introduced "cluster 
> settings" in Ambari.
> 
> This review adds 2 new parameters to Execution Command : **(1).** 
> clusterSettings and **(2).** stackSettings, and adds their respective set of 
> key-values pairs.
> 
> - This enables these parameters to be passed in to the ambari-agent's 
> **command*.json**.
> 
> 
> Test cases are tracked in : AMBARI-22603
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
>  5ee4bf6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  e7dea06 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  59e6622 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  9043297 
>   
> ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
>  8bd24f2 
> 
> 
> Diff: https://reviews.apache.org/r/64397/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Live cluster doing following : 
> (1). Start Service 
> (2). Restart Service 
> (3). Stop Service 
> (4). Refreshing client configs
> (5). Adding Client.
> 
> 
> **Snippet from command*.json as part of testing:**
> {
> ...
> ...
> "clusterSettings": {
> "security_enabled": "false",
> "hide_yarn_memory_widget": "false",
> "enable_external_ranger": "false",
> "override_uid": "true",
> "kerberos_domain": "EXAMPLE.COM",
> "one_dir_per_partition": "false",
> "repo_ubuntu_template": "{{package_type}} {{base_url}} 
> {{components}}",
> "ignore_groupsusers_create": "false",
> "alerts_repeat_tolerance": "1",
> "namenode_rolling_restart_timeout": "4200",
> "fetch_nonlocal_groups": "true",
> "manage_dirs_on_root": "true",
> "recovery_lifetime_max_count": "1024",
> "agent_mounts_ignore_list": "",
> "ignore_bad_mounts": "false",
> "recovery_window_in_minutes": "60",
> "sysprep_skip_copy_tarballs_hdfs": "false",
> "recovery_type": "AUTO_START",
> "user_group": "hadoop",
> "namenode_rolling_restart_safemode_exit_timeout": "3600",
> "recovery_retry_interval": "5",
> "sysprep_skip_copy_oozie_share_lib_to_hdfs": "false",
> "sysprep_skip_setup_jce": "false",
> "manage_hive_fsroot": "true",
> "service_check_type": "full",
> "recovery_enabled": "true",
> "recovery_max_count": "6",
> "sysprep_skip_create_users_and_groups": "false",
> "smokeuser_keytab": "/etc/security/keytabs/smokeuser.headless.keytab",
> "managed_hdfs_resource_property_names": "false",
> "smokeuser": "ambari-qa",
> "sysprep_skip_copy_fast_jar_hdfs": "false"
> },
> ...
> ...
> ...
> ...
> ...
> ...
> ...
> "stackSettings": {
> "stack_features": "{\n  \"HDP\": {\n\"stack_features\": [\n  
> {\n\"name\": \"snappy\",\n\"description\": \"Snappy 
> compressor/decompressor support\",\n\"min_version\": \"2.0.0.0\",\n   
>  \"max_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
> \"lzo\",\n\"description\": \"LZO libraries support\",\n
> \"min_version\": \"2.2.1.0\"\n  },\n  {\n\"name\": 
> \"express_upgrade\",\n\"description\": \"Express upgrade support\",\n 
>\"min_version\": \"2.1.0.0\"\n  },\n

Re: Review Request 64397: AMBARI-22602. Add 'clusterSettings' and 'stackSettings' parameters in Execution Command.

2017-12-11 Thread Swapan Shridhar 


> On Dec. 8, 2017, 8:01 p.m., Attila Doroszlai wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
> > Lines 436 (patched)
> > 
> >
> > * no need for explicit type arg in Maps.newHashMap()
> > * empty map should be created lazily in `else` below

Fixed.


> On Dec. 8, 2017, 8:01 p.m., Attila Doroszlai wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
> > Lines 438 (patched)
> > 
> >
> > According to IDEA there is no need to include the 
> > `clusterSettings.values() != null` check, as it is always true if 
> > `clusterSettings != null` is true.

Removed.


> On Dec. 8, 2017, 8:01 p.m., Attila Doroszlai wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
> > Lines 449-451 (patched)
> > 
> >
> > Please use `collect(toMap())` (as above) instead of `forEach` and `put`

Fixed.


> On Dec. 8, 2017, 8:01 p.m., Attila Doroszlai wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
> > Lines 2676-2695 (patched)
> > 
> >
> > Can you please extract the logic to 2 utility methods to avoid code 
> > duplication among `AmbariCustomCommandExecutionHelper`, 
> > `AmbariManagementControllerImpl` and `ClientConfigResourceProvider`?

Refactored.


- Swapan


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


On Dec. 7, 2017, 8:11 a.m., Swapan Shridhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64397/
> ---
> 
> (Updated Dec. 7, 2017, 8:11 a.m.)
> 
> 
> Review request for Ambari, Jayush Luniya and Madhuvanthi Radhakrishnan.
> 
> 
> Bugs: AMBARI-22602
> https://issues.apache.org/jira/browse/AMBARI-22602
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> AMBARI-22198 added "stack settings", and AMBARI-22196 introduced "cluster 
> settings" in Ambari.
> 
> This review adds 2 new parameters to Execution Command : **(1).** 
> clusterSettings and **(2).** stackSettings, and adds their respective set of 
> key-values pairs.
> 
> - This enables these parameters to be passed in to the ambari-agent's 
> **command*.json**.
> 
> 
> Test cases are tracked in : AMBARI-22603
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
>  5ee4bf6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  e7dea06 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  59e6622 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  9043297 
>   
> ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
>  8bd24f2 
> 
> 
> Diff: https://reviews.apache.org/r/64397/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Live cluster doing following : 
> (1). Start Service 
> (2). Restart Service 
> (3). Stop Service 
> (4). Refreshing client configs
> (5). Adding Client.
> 
> 
> **Snippet from command*.json as part of testing:**
> {
> ...
> ...
> "clusterSettings": {
> "security_enabled": "false",
> "hide_yarn_memory_widget": "false",
> "enable_external_ranger": "false",
> "override_uid": "true",
> "kerberos_domain": "EXAMPLE.COM",
> "one_dir_per_partition": "false",
> "repo_ubuntu_template": "{{package_type}} {{base_url}} 
> {{components}}",
> "ignore_groupsusers_create": "false",
> "alerts_repeat_tolerance": "1",
> "namenode_rolling_restart_timeout": "4200",
> "fetch_nonlocal_groups": "true",
> "manage_dirs_on_root": "true",
> "recovery_lifetime_max_count": "1024",
> "agent_mounts_ignore_list": "",
> "ignore_bad_mounts": "false",
> "recovery_window_in_minutes": "60",
> "sysprep_skip_copy_tarballs_hdfs": "false",
> "recovery_type": "AUTO_START",
> "user_group": "hadoop",
> "namenode_rolling_restart_safemode_exit_timeout": "3600",
> "recovery_retry_interval": "5",
> "sysprep_skip_copy_oozie_share_lib_to_hdfs": "false",
> "sysprep_skip_setup_jce": "false",
>

Re: Review Request 64397: AMBARI-22602. Add 'clusterSettings' and 'stackSettings' parameters in Execution Command.

2017-12-11 Thread Swapan Shridhar 


> On Dec. 8, 2017, 11:41 p.m., Madhuvanthi Radhakrishnan wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
> > Line 57 (original)
> > 
> >
> > Are we using this map anywhere?

Nope. Its part of the cleanup.


- Swapan


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


On Dec. 7, 2017, 8:11 a.m., Swapan Shridhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64397/
> ---
> 
> (Updated Dec. 7, 2017, 8:11 a.m.)
> 
> 
> Review request for Ambari, Jayush Luniya and Madhuvanthi Radhakrishnan.
> 
> 
> Bugs: AMBARI-22602
> https://issues.apache.org/jira/browse/AMBARI-22602
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> AMBARI-22198 added "stack settings", and AMBARI-22196 introduced "cluster 
> settings" in Ambari.
> 
> This review adds 2 new parameters to Execution Command : **(1).** 
> clusterSettings and **(2).** stackSettings, and adds their respective set of 
> key-values pairs.
> 
> - This enables these parameters to be passed in to the ambari-agent's 
> **command*.json**.
> 
> 
> Test cases are tracked in : AMBARI-22603
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
>  5ee4bf6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  e7dea06 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  59e6622 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  9043297 
>   
> ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
>  8bd24f2 
> 
> 
> Diff: https://reviews.apache.org/r/64397/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Live cluster doing following : 
> (1). Start Service 
> (2). Restart Service 
> (3). Stop Service 
> (4). Refreshing client configs
> (5). Adding Client.
> 
> 
> **Snippet from command*.json as part of testing:**
> {
> ...
> ...
> "clusterSettings": {
> "security_enabled": "false",
> "hide_yarn_memory_widget": "false",
> "enable_external_ranger": "false",
> "override_uid": "true",
> "kerberos_domain": "EXAMPLE.COM",
> "one_dir_per_partition": "false",
> "repo_ubuntu_template": "{{package_type}} {{base_url}} 
> {{components}}",
> "ignore_groupsusers_create": "false",
> "alerts_repeat_tolerance": "1",
> "namenode_rolling_restart_timeout": "4200",
> "fetch_nonlocal_groups": "true",
> "manage_dirs_on_root": "true",
> "recovery_lifetime_max_count": "1024",
> "agent_mounts_ignore_list": "",
> "ignore_bad_mounts": "false",
> "recovery_window_in_minutes": "60",
> "sysprep_skip_copy_tarballs_hdfs": "false",
> "recovery_type": "AUTO_START",
> "user_group": "hadoop",
> "namenode_rolling_restart_safemode_exit_timeout": "3600",
> "recovery_retry_interval": "5",
> "sysprep_skip_copy_oozie_share_lib_to_hdfs": "false",
> "sysprep_skip_setup_jce": "false",
> "manage_hive_fsroot": "true",
> "service_check_type": "full",
> "recovery_enabled": "true",
> "recovery_max_count": "6",
> "sysprep_skip_create_users_and_groups": "false",
> "smokeuser_keytab": "/etc/security/keytabs/smokeuser.headless.keytab",
> "managed_hdfs_resource_property_names": "false",
> "smokeuser": "ambari-qa",
> "sysprep_skip_copy_fast_jar_hdfs": "false"
> },
> ...
> ...
> ...
> ...
> ...
> ...
> ...
> "stackSettings": {
> "stack_features": "{\n  \"HDP\": {\n\"stack_features\": [\n  
> {\n\"name\": \"snappy\",\n\"description\": \"Snappy 
> compressor/decompressor support\",\n\"min_version\": \"2.0.0.0\",\n   
>  \"max_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
> \"lzo\",\n\"description\": \"LZO libraries support\",\n
> \"min_version\": \"2.2.1.0\"\n  },\n  {\n\"name\": 
> \"express_upgrade\",\n\"description\": \"Express upgrade support\",\n 
>\"min_version\": \"2.1.0.0\"\n  },\n  {\n\"name\": 
> \"rolling_upgrade\",\n\"description\": \"Rolling upgrade support\",\n 
>\"min_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
> \"kafka_acl_migration_support\",\n\"description\": \"ACL migration 
> support\",\n

Re: Review Request 64397: AMBARI-22602. Add 'clusterSettings' and 'stackSettings' parameters in Execution Command.

2017-12-11 Thread Jayush Luniya 

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




ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
Lines 291 (patched)


settings or clusterSettings instead of params?



ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
Lines 300 (patched)


settings or stackSettings instead of params?


- Jayush Luniya


On Dec. 7, 2017, 8:11 a.m., Swapan Shridhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64397/
> ---
> 
> (Updated Dec. 7, 2017, 8:11 a.m.)
> 
> 
> Review request for Ambari, Jayush Luniya and Madhuvanthi Radhakrishnan.
> 
> 
> Bugs: AMBARI-22602
> https://issues.apache.org/jira/browse/AMBARI-22602
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> AMBARI-22198 added "stack settings", and AMBARI-22196 introduced "cluster 
> settings" in Ambari.
> 
> This review adds 2 new parameters to Execution Command : **(1).** 
> clusterSettings and **(2).** stackSettings, and adds their respective set of 
> key-values pairs.
> 
> - This enables these parameters to be passed in to the ambari-agent's 
> **command*.json**.
> 
> 
> Test cases are tracked in : AMBARI-22603
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
>  5ee4bf6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  e7dea06 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  59e6622 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  9043297 
>   
> ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
>  8bd24f2 
> 
> 
> Diff: https://reviews.apache.org/r/64397/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Live cluster doing following : 
> (1). Start Service 
> (2). Restart Service 
> (3). Stop Service 
> (4). Refreshing client configs
> (5). Adding Client.
> 
> 
> **Snippet from command*.json as part of testing:**
> {
> ...
> ...
> "clusterSettings": {
> "security_enabled": "false",
> "hide_yarn_memory_widget": "false",
> "enable_external_ranger": "false",
> "override_uid": "true",
> "kerberos_domain": "EXAMPLE.COM",
> "one_dir_per_partition": "false",
> "repo_ubuntu_template": "{{package_type}} {{base_url}} 
> {{components}}",
> "ignore_groupsusers_create": "false",
> "alerts_repeat_tolerance": "1",
> "namenode_rolling_restart_timeout": "4200",
> "fetch_nonlocal_groups": "true",
> "manage_dirs_on_root": "true",
> "recovery_lifetime_max_count": "1024",
> "agent_mounts_ignore_list": "",
> "ignore_bad_mounts": "false",
> "recovery_window_in_minutes": "60",
> "sysprep_skip_copy_tarballs_hdfs": "false",
> "recovery_type": "AUTO_START",
> "user_group": "hadoop",
> "namenode_rolling_restart_safemode_exit_timeout": "3600",
> "recovery_retry_interval": "5",
> "sysprep_skip_copy_oozie_share_lib_to_hdfs": "false",
> "sysprep_skip_setup_jce": "false",
> "manage_hive_fsroot": "true",
> "service_check_type": "full",
> "recovery_enabled": "true",
> "recovery_max_count": "6",
> "sysprep_skip_create_users_and_groups": "false",
> "smokeuser_keytab": "/etc/security/keytabs/smokeuser.headless.keytab",
> "managed_hdfs_resource_property_names": "false",
> "smokeuser": "ambari-qa",
> "sysprep_skip_copy_fast_jar_hdfs": "false"
> },
> ...
> ...
> ...
> ...
> ...
> ...
> ...
> "stackSettings": {
> "stack_features": "{\n  \"HDP\": {\n\"stack_features\": [\n  
> {\n\"name\": \"snappy\",\n\"description\": \"Snappy 
> compressor/decompressor support\",\n\"min_version\": \"2.0.0.0\",\n   
>  \"max_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
> \"lzo\",\n\"description\": \"LZO libraries support\",\n
> \"min_version\": \"2.2.1.0\"\n  },\n  {\n\"name\": 
> \"express_upgrade\",\n\"description\": \"Express upgrade support\",\n 
>\"min_version\": \"2.1.0.0\"\n  },\n  {\n\"name\": 
> \"rolling_upgrade\",\n\"description\": \"Rolling upgrade support\",\n 
>\"min_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
>

Re: Review Request 64397: AMBARI-22602. Add 'clusterSettings' and 'stackSettings' parameters in Execution Command.

2017-12-08 Thread Madhuvanthi Radhakrishnan 

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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
Line 57 (original)


Are we using this map anywhere?


- Madhuvanthi Radhakrishnan


On Dec. 7, 2017, 8:11 a.m., Swapan Shridhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64397/
> ---
> 
> (Updated Dec. 7, 2017, 8:11 a.m.)
> 
> 
> Review request for Ambari, Jayush Luniya and Madhuvanthi Radhakrishnan.
> 
> 
> Bugs: AMBARI-22602
> https://issues.apache.org/jira/browse/AMBARI-22602
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> AMBARI-22198 added "stack settings", and AMBARI-22196 introduced "cluster 
> settings" in Ambari.
> 
> This review adds 2 new parameters to Execution Command : **(1).** 
> clusterSettings and **(2).** stackSettings, and adds their respective set of 
> key-values pairs.
> 
> - This enables these parameters to be passed in to the ambari-agent's 
> **command*.json**.
> 
> 
> Test cases are tracked in : AMBARI-22603
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
>  5ee4bf6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  e7dea06 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  59e6622 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  9043297 
>   
> ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
>  8bd24f2 
> 
> 
> Diff: https://reviews.apache.org/r/64397/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Live cluster doing following : 
> (1). Start Service 
> (2). Restart Service 
> (3). Stop Service 
> (4). Refreshing client configs
> (5). Adding Client.
> 
> 
> **Snippet from command*.json as part of testing:**
> {
> ...
> ...
> "clusterSettings": {
> "security_enabled": "false",
> "hide_yarn_memory_widget": "false",
> "enable_external_ranger": "false",
> "override_uid": "true",
> "kerberos_domain": "EXAMPLE.COM",
> "one_dir_per_partition": "false",
> "repo_ubuntu_template": "{{package_type}} {{base_url}} 
> {{components}}",
> "ignore_groupsusers_create": "false",
> "alerts_repeat_tolerance": "1",
> "namenode_rolling_restart_timeout": "4200",
> "fetch_nonlocal_groups": "true",
> "manage_dirs_on_root": "true",
> "recovery_lifetime_max_count": "1024",
> "agent_mounts_ignore_list": "",
> "ignore_bad_mounts": "false",
> "recovery_window_in_minutes": "60",
> "sysprep_skip_copy_tarballs_hdfs": "false",
> "recovery_type": "AUTO_START",
> "user_group": "hadoop",
> "namenode_rolling_restart_safemode_exit_timeout": "3600",
> "recovery_retry_interval": "5",
> "sysprep_skip_copy_oozie_share_lib_to_hdfs": "false",
> "sysprep_skip_setup_jce": "false",
> "manage_hive_fsroot": "true",
> "service_check_type": "full",
> "recovery_enabled": "true",
> "recovery_max_count": "6",
> "sysprep_skip_create_users_and_groups": "false",
> "smokeuser_keytab": "/etc/security/keytabs/smokeuser.headless.keytab",
> "managed_hdfs_resource_property_names": "false",
> "smokeuser": "ambari-qa",
> "sysprep_skip_copy_fast_jar_hdfs": "false"
> },
> ...
> ...
> ...
> ...
> ...
> ...
> ...
> "stackSettings": {
> "stack_features": "{\n  \"HDP\": {\n\"stack_features\": [\n  
> {\n\"name\": \"snappy\",\n\"description\": \"Snappy 
> compressor/decompressor support\",\n\"min_version\": \"2.0.0.0\",\n   
>  \"max_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
> \"lzo\",\n\"description\": \"LZO libraries support\",\n
> \"min_version\": \"2.2.1.0\"\n  },\n  {\n\"name\": 
> \"express_upgrade\",\n\"description\": \"Express upgrade support\",\n 
>\"min_version\": \"2.1.0.0\"\n  },\n  {\n\"name\": 
> \"rolling_upgrade\",\n\"description\": \"Rolling upgrade support\",\n 
>\"min_version\": \"2.2.0.0\"\n  },\n  {\n\"name\": 
> \"kafka_acl_migration_support\",\n\"description\": \"ACL migration 
> support\",\n\"min_version\": \"2.3.4.0\"\n  },\n  {\n
> \"name\":

Re: Review Request 64397: AMBARI-22602. Add 'clusterSettings' and 'stackSettings' parameters in Execution Command.

2017-12-08 Thread Attila Doroszlai 

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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
Lines 436 (patched)


* no need for explicit type arg in Maps.newHashMap()
* empty map should be created lazily in `else` below



ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
Lines 438 (patched)


According to IDEA there is no need to include the `clusterSettings.values() 
!= null` check, as it is always true if `clusterSettings != null` is true.



ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
Lines 449-451 (patched)


Please use `collect(toMap())` (as above) instead of `forEach` and `put`



ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
Lines 2676-2695 (patched)


Can you please extract the logic to 2 utility methods to avoid code 
duplication among `AmbariCustomCommandExecutionHelper`, 
`AmbariManagementControllerImpl` and `ClientConfigResourceProvider`?


- Attila Doroszlai


On Dec. 7, 2017, 9:11 a.m., Swapan Shridhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64397/
> ---
> 
> (Updated Dec. 7, 2017, 9:11 a.m.)
> 
> 
> Review request for Ambari, Jayush Luniya and Madhuvanthi Radhakrishnan.
> 
> 
> Bugs: AMBARI-22602
> https://issues.apache.org/jira/browse/AMBARI-22602
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> AMBARI-22198 added "stack settings", and AMBARI-22196 introduced "cluster 
> settings" in Ambari.
> 
> This review adds 2 new parameters to Execution Command : **(1).** 
> clusterSettings and **(2).** stackSettings, and adds their respective set of 
> key-values pairs.
> 
> - This enables these parameters to be passed in to the ambari-agent's 
> **command*.json**.
> 
> 
> Test cases are tracked in : AMBARI-22603
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/ExecutionCommand.java
>  5ee4bf6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  e7dea06 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  59e6622 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
>  9043297 
>   
> ambari-server/src/main/java/org/apache/ambari/server/resources/RootLevelSettingsManager.java
>  8bd24f2 
> 
> 
> Diff: https://reviews.apache.org/r/64397/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Live cluster doing following : 
> (1). Start Service 
> (2). Restart Service 
> (3). Stop Service 
> (4). Refreshing client configs
> (5). Adding Client.
> 
> 
> **Snippet from command*.json as part of testing:**
> {
> ...
> ...
> "clusterSettings": {
> "security_enabled": "false",
> "hide_yarn_memory_widget": "false",
> "enable_external_ranger": "false",
> "override_uid": "true",
> "kerberos_domain": "EXAMPLE.COM",
> "one_dir_per_partition": "false",
> "repo_ubuntu_template": "{{package_type}} {{base_url}} 
> {{components}}",
> "ignore_groupsusers_create": "false",
> "alerts_repeat_tolerance": "1",
> "namenode_rolling_restart_timeout": "4200",
> "fetch_nonlocal_groups": "true",
> "manage_dirs_on_root": "true",
> "recovery_lifetime_max_count": "1024",
> "agent_mounts_ignore_list": "",
> "ignore_bad_mounts": "false",
> "recovery_window_in_minutes": "60",
> "sysprep_skip_copy_tarballs_hdfs": "false",
> "recovery_type": "AUTO_START",
> "user_group": "hadoop",
> "namenode_rolling_restart_safemode_exit_timeout": "3600",
> "recovery_retry_interval": "5",
> "sysprep_skip_copy_oozie_share_lib_to_hdfs": "false",
> "sysprep_skip_setup_jce": "false",
> "manage_hive_fsroot": "true",
> "service_check_type": "full",
> "recovery_enabled": "true",
> "recovery_max_count": "6",
> "sysprep_skip_create_users_and_groups": "false",
> "smokeuser_keytab": "/etc/security/keytabs/smokeuser.headless.keytab",
> "managed_hdfs_resource_property_names": "false",
> "smokeuser": "ambari-qa",
> "sysprep_skip_copy_fast_jar_hdfs": "false"
>