Re: Review Request 58489: VersionAdvertised should be set to False by default in stack_tool.py

2017-04-18 Thread Madhuvanthi Radhakrishnan 


> On April 18, 2017, 12:36 a.m., Alejandro Fernandez wrote:
> > ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
> > Line 52 (original), 52 (patched)
> > 
> >
> > Why would service_name be the string "null"?
> 
> Madhuvanthi Radhakrishnan wrote:
> During install_packages roleCommand, the service name is set to "null"
> /var/lib/ambari-agent/data/command-5.json:"serviceName": "null",
> 
> Nate Cole wrote:
> If serviceName is the string "null" then that's pretty lame. :)  That 
> should be fixed.
> 
> Jayush Luniya wrote:
> I agree we should not have serviceName = "null" :) However we ensure 
> there are no other side effects to it.

Hi,
On debugging it looks like during json.dumps (CustomServiceOrchestrator.py) it 
converts everything to string and hence null is converted to "null".
I see that this is the case for not just serviceName but several other values.
For eg:
[root@jay-hdf-1 data]# grep \"null\" command-302.json
"capacity-scheduler": "null",
"_storm.min.ruid": "null",
"topology.acker.executors": "null",
"topology.max.task.parallelism": "null",
"transactional.zookeeper.port": "null",
"topology.worker.childopts": "null",
"transactional.zookeeper.servers": "null",
"topology.tick.tuple.freq.secs": "null",
"ui.filter": "null",

One way to fix this is in the AmbariActionExecutionHelper.java, where I can 
assign serviceName = "" instead of null and then remove the "null" check in 
stack_tools.py.
This opens up a host of unit test failures where the expectation is null (java 
side) and test command.json files use "serviceName" : "null" (python side)
Since the fix in this patch is a blocker for system tests, I can address the 
null issue in a follow up jira.


- Madhuvanthi


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


On April 18, 2017, 11:30 p.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58489/
> ---
> 
> (Updated April 18, 2017, 11:30 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-20775
> https://issues.apache.org/jira/browse/AMBARI-20775
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> All the commands.json files should always contain the versionAdvertised 
> field. If versionAdvertised is not set in command.json then we should assume 
> that versionAdvertised=False when calling stack_tools.py
> 
> 
> Diffs
> -
> 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/get_stack_version.py
>  463d61f53d 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
>  93ec0b74f5 
>   
> ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
>  3579fcbd2a 
>   ambari-server/src/test/python/stacks/2.0.6/YARN/test_nodemanager.py 
> ab5e2cdaf9 
>   ambari-server/src/test/python/stacks/2.0.6/configs/client-upgrade.json 
> 0b5ba6a0ba 
>   ambari-server/src/test/python/stacks/2.0.6/configs/default.json 94425e0a30 
>   ambari-server/src/test/python/stacks/2.0.6/configs/flume_22.json 6ec9ec929c 
>   ambari-server/src/test/python/stacks/2.0.6/configs/flume_only.json 
> 1550715d72 
>   ambari-server/src/test/python/stacks/2.1/configs/client-upgrade.json 
> 013bb8b8d2 
>   ambari-server/src/test/python/stacks/2.1/configs/default.json e04e1eb9f4 
>   ambari-server/src/test/python/stacks/2.2/configs/default.json bcb021b74f 
>   ambari-server/src/test/python/stacks/2.2/configs/falcon-upgrade.json 
> 7ded3a01e1 
>   ambari-server/src/test/python/stacks/2.2/configs/hive-upgrade.json 
> 7a29ea0c0d 
>   ambari-server/src/test/python/stacks/2.2/configs/knox_upgrade.json 
> a9db11c645 
>   ambari-server/src/test/python/stacks/2.2/configs/oozie-downgrade.json 
> fb561f74f3 
>   ambari-server/src/test/python/stacks/2.2/configs/oozie-upgrade.json 
> 86ca03a4a3 
>   ambari-server/src/test/python/stacks/2.2/configs/ranger-admin-upgrade.json 
> cbe8ebeb15 
>   
> ambari-server/src/test/python/stacks/2.2/configs/ranger-usersync-upgrade.json 
> dc4c43b9e0 
>   ambari-server/src/test/python/stacks/2.3/configs/hbase_default.json 
> 0da58ce2a5 
> 
> 
> Diff: https://reviews.apache.org/r/58489/diff/2/
> 
> 
> Testing
> ---
> 
> Tested for service actions:
> Start
> Stop
> Restart
> Reconfigure
> Service Check
> Enable HA
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 58489: VersionAdvertised should be set to False by default in stack_tool.py

2017-04-18 Thread Madhuvanthi Radhakrishnan 

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

(Updated April 18, 2017, 11:30 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush Luniya, 
Nate Cole, and Sumit Mohanty.


Changes
---

Fixed unit tests. Incorporated review comments


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


Repository: ambari


Description
---

All the commands.json files should always contain the versionAdvertised field. 
If versionAdvertised is not set in command.json then we should assume that 
versionAdvertised=False when calling stack_tools.py


Diffs (updated)
-

  
ambari-common/src/main/python/resource_management/libraries/functions/get_stack_version.py
 463d61f53d 
  
ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
 93ec0b74f5 
  
ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
 3579fcbd2a 
  ambari-server/src/test/python/stacks/2.0.6/YARN/test_nodemanager.py 
ab5e2cdaf9 
  ambari-server/src/test/python/stacks/2.0.6/configs/client-upgrade.json 
0b5ba6a0ba 
  ambari-server/src/test/python/stacks/2.0.6/configs/default.json 94425e0a30 
  ambari-server/src/test/python/stacks/2.0.6/configs/flume_22.json 6ec9ec929c 
  ambari-server/src/test/python/stacks/2.0.6/configs/flume_only.json 1550715d72 
  ambari-server/src/test/python/stacks/2.1/configs/client-upgrade.json 
013bb8b8d2 
  ambari-server/src/test/python/stacks/2.1/configs/default.json e04e1eb9f4 
  ambari-server/src/test/python/stacks/2.2/configs/default.json bcb021b74f 
  ambari-server/src/test/python/stacks/2.2/configs/falcon-upgrade.json 
7ded3a01e1 
  ambari-server/src/test/python/stacks/2.2/configs/hive-upgrade.json 7a29ea0c0d 
  ambari-server/src/test/python/stacks/2.2/configs/knox_upgrade.json a9db11c645 
  ambari-server/src/test/python/stacks/2.2/configs/oozie-downgrade.json 
fb561f74f3 
  ambari-server/src/test/python/stacks/2.2/configs/oozie-upgrade.json 
86ca03a4a3 
  ambari-server/src/test/python/stacks/2.2/configs/ranger-admin-upgrade.json 
cbe8ebeb15 
  ambari-server/src/test/python/stacks/2.2/configs/ranger-usersync-upgrade.json 
dc4c43b9e0 
  ambari-server/src/test/python/stacks/2.3/configs/hbase_default.json 
0da58ce2a5 


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

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


Testing
---

Tested for service actions:
Start
Stop
Restart
Reconfigure
Service Check
Enable HA


Thanks,

Madhuvanthi Radhakrishnan

Re: Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

2017-04-18 Thread Amruta Borkar 

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

(Updated April 18, 2017, 9:13 p.m.)


Review request for Ambari, Di Li, Robert Levas, Robert Nettleton, and Sandor 
Magyari.


Changes
---

Updated patch for latest trunk code


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


Repository: ambari


Description
---

If blueprint is exported from a kerberos enabled cluster Kerberos rules export 
principal names which contain cluster name and Realm, this exports existing 
cluster name and realm name as tokens and replaces those tokens with new values 
of cluster name and realm during successive cluster deployments.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
 bb771a54a6 
  
ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java
 13db5f8b56 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
 5c1836a87d 


Diff: https://reviews.apache.org/r/57610/diff/4/

Changes: https://reviews.apache.org/r/57610/diff/3-4/


Testing
---

Tested manually.
Modified test cases.


Thanks,

Amruta Borkar

Re: Review Request 58490: AMBARI-20777 : AMS changes to use instanceId for cluster based segregation of data

2017-04-18 Thread Aravindan Vijayan 

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

(Updated April 18, 2017, 8:04 p.m.)


Review request for Ambari, Dmytro Sen, Sumit Mohanty, and Sid Wagle.


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


Repository: ambari


Description
---

This issue tracks changes in the following work items to facilitate the use of 
the "instanceId" field in AMS to capture per cluster metric data.

Collector API
Schema
Metadata


Diffs (updated)
-

  
ambari-metrics/ambari-metrics-hadoop-sink/src/main/java/org/apache/hadoop/metrics2/sink/timeline/HadoopTimelineMetricsSink.java
 1f0adc0 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java
 72ae4ac 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java
 a4539c4 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStore.java
 d049e33 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/aggregators/TimelineMetricClusterAggregatorSecond.java
 6683c0d 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TimelineMetricMetadataManager.java
 312abfc 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TimelineMetricMetadataSync.java
 25b525a 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/PhoenixTransactSQL.java
 0c8e5a7 
  
ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java
 304a8e0 
  
ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestTimelineMetricStore.java
 b2e8cac 
  
ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TestMetadataManager.java
 0c7009c 
  
ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TestMetadataSync.java
 5eab903 


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

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


Testing
---

Manually tested. 
Added unit tests.


Thanks,

Aravindan Vijayan

Re: Review Request 58490: AMBARI-20777 : AMS changes to use instanceId for cluster based segregation of data

2017-04-18 Thread Aravindan Vijayan 


> On April 18, 2017, 7:55 p.m., Sid Wagle wrote:
> > ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java
> > Lines 778 (patched)
> > 
> >
> > Null check for instanceid?

Done in method.

  public void putIfModifiedHostedInstanceMetadata(String instanceId, String 
hostname) {
if (StringUtils.isEmpty(instanceId)) {
  return;
}


- Aravindan


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


On April 18, 2017, 12:43 a.m., Aravindan Vijayan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58490/
> ---
> 
> (Updated April 18, 2017, 12:43 a.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20777
> https://issues.apache.org/jira/browse/AMBARI-20777
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This issue tracks changes in the following work items to facilitate the use 
> of the "instanceId" field in AMS to capture per cluster metric data.
> 
> Collector API
> Schema
> Metadata
> 
> 
> Diffs
> -
> 
>   
> ambari-metrics/ambari-metrics-hadoop-sink/src/main/java/org/apache/hadoop/metrics2/sink/timeline/HadoopTimelineMetricsSink.java
>  1f0adc0 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java
>  72ae4ac 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java
>  a4539c4 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStore.java
>  d049e33 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/aggregators/TimelineMetricClusterAggregatorSecond.java
>  6683c0d 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TimelineMetricMetadataManager.java
>  312abfc 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TimelineMetricMetadataSync.java
>  25b525a 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/PhoenixTransactSQL.java
>  0c8e5a7 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java
>  304a8e0 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestTimelineMetricStore.java
>  b2e8cac 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TestMetadataManager.java
>  0c7009c 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TestMetadataSync.java
>  5eab903 
> 
> 
> Diff: https://reviews.apache.org/r/58490/diff/1/
> 
> 
> Testing
> ---
> 
> Manually tested. 
> Added unit tests.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>

Re: Review Request 58490: AMBARI-20777 : AMS changes to use instanceId for cluster based segregation of data

2017-04-18 Thread Sid Wagle 

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


Fix it, then Ship it!





ambari-metrics/ambari-metrics-hadoop-sink/src/main/java/org/apache/hadoop/metrics2/sink/timeline/HadoopTimelineMetricsSink.java
Lines 54 (patched)


Should be null initialized similar to the if condition?



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java
Lines 778 (patched)


Null check for instanceid?


- Sid Wagle


On April 18, 2017, 12:43 a.m., Aravindan Vijayan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58490/
> ---
> 
> (Updated April 18, 2017, 12:43 a.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-20777
> https://issues.apache.org/jira/browse/AMBARI-20777
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This issue tracks changes in the following work items to facilitate the use 
> of the "instanceId" field in AMS to capture per cluster metric data.
> 
> Collector API
> Schema
> Metadata
> 
> 
> Diffs
> -
> 
>   
> ambari-metrics/ambari-metrics-hadoop-sink/src/main/java/org/apache/hadoop/metrics2/sink/timeline/HadoopTimelineMetricsSink.java
>  1f0adc0 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java
>  72ae4ac 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java
>  a4539c4 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStore.java
>  d049e33 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/aggregators/TimelineMetricClusterAggregatorSecond.java
>  6683c0d 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TimelineMetricMetadataManager.java
>  312abfc 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TimelineMetricMetadataSync.java
>  25b525a 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/PhoenixTransactSQL.java
>  0c8e5a7 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java
>  304a8e0 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestTimelineMetricStore.java
>  b2e8cac 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TestMetadataManager.java
>  0c7009c 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TestMetadataSync.java
>  5eab903 
> 
> 
> Diff: https://reviews.apache.org/r/58490/diff/1/
> 
> 
> Testing
> ---
> 
> Manually tested. 
> Added unit tests.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>

Re: Review Request 58489: VersionAdvertised should be set to False by default in stack_tool.py

2017-04-18 Thread Jayush Luniya 


> On April 18, 2017, 12:36 a.m., Alejandro Fernandez wrote:
> > ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
> > Line 52 (original), 52 (patched)
> > 
> >
> > Why would service_name be the string "null"?
> 
> Madhuvanthi Radhakrishnan wrote:
> During install_packages roleCommand, the service name is set to "null"
> /var/lib/ambari-agent/data/command-5.json:"serviceName": "null",
> 
> Nate Cole wrote:
> If serviceName is the string "null" then that's pretty lame. :)  That 
> should be fixed.

I agree we should not have serviceName = "null" :) However we ensure there are 
no other side effects to it.


- Jayush


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


On April 18, 2017, 12:31 a.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58489/
> ---
> 
> (Updated April 18, 2017, 12:31 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-20775
> https://issues.apache.org/jira/browse/AMBARI-20775
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> All the commands.json files should always contain the versionAdvertised 
> field. If versionAdvertised is not set in command.json then we should assume 
> that versionAdvertised=False when calling stack_tools.py
> 
> 
> Diffs
> -
> 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
>  93ec0b74f5 
>   
> ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
>  03f53d490b 
> 
> 
> Diff: https://reviews.apache.org/r/58489/diff/1/
> 
> 
> Testing
> ---
> 
> Tested for service actions:
> Start
> Stop
> Restart
> Reconfigure
> Service Check
> Enable HA
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 58483: Service Upgrade VDF Creates Host Version Entries For All Hosts With INSTALLING

2017-04-18 Thread Jonathan Hurley 

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

(Updated April 18, 2017, 2:14 p.m.)


Review request for Ambari, Dmytro Grinenko and Nate Cole.


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


Repository: ambari


Description
---

When a VDF for a {{SERVICE}} upgrade targets only a subset of hosts in the 
cluster, there is a host version entry created for all hosts with the state of 
{{INSTALLING}}. This causes the web client to think that the installation is 
not complete and it prevents the upgrade button from displaying.

STR:
- Install a cluster with ZK and Storm
-- ZK should be on all 3 hosts, Storm only on 2
- Upload a VDF for Storm only and distribute it

With the introduction of PATCH/SERVICE repositories, we need to properly set 
the host_version based on a variety of factors:
- NOT_REQUIRED if the host has no versionable components or has no components 
targetted by the repository
- OUT_OF_SYNC _only_ if the host is supposed to have the repository installed 
but is in MM
- INSTALLING for all others (INSTALLED when forced)


Diffs
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java
 9c54a9d 
  ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
c961995 
  
ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
 b7cc4cd 
  
ambari-server/src/test/java/org/apache/ambari/server/checks/ServicesNamenodeTruncateCheckTest.java
 2954f0d 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
 5ef31b5 
  
ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
 345c463 


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


Testing (updated)
---

[INFO] 
[INFO] BUILD SUCCESS
[INFO] 


Thanks,

Jonathan Hurley

Re: Review Request 58483: Service Upgrade VDF Creates Host Version Entries For All Hosts With INSTALLING

2017-04-18 Thread Jonathan Hurley 

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

(Updated April 18, 2017, 2:05 p.m.)


Review request for Ambari, Dmytro Grinenko and Nate Cole.


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


Repository: ambari


Description
---

When a VDF for a {{SERVICE}} upgrade targets only a subset of hosts in the 
cluster, there is a host version entry created for all hosts with the state of 
{{INSTALLING}}. This causes the web client to think that the installation is 
not complete and it prevents the upgrade button from displaying.

STR:
- Install a cluster with ZK and Storm
-- ZK should be on all 3 hosts, Storm only on 2
- Upload a VDF for Storm only and distribute it

With the introduction of PATCH/SERVICE repositories, we need to properly set 
the host_version based on a variety of factors:
- NOT_REQUIRED if the host has no versionable components or has no components 
targetted by the repository
- OUT_OF_SYNC _only_ if the host is supposed to have the repository installed 
but is in MM
- INSTALLING for all others (INSTALLED when forced)


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java
 9c54a9d 
  ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
c961995 
  
ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
 b7cc4cd 
  
ambari-server/src/test/java/org/apache/ambari/server/checks/ServicesNamenodeTruncateCheckTest.java
 2954f0d 
  
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
 5ef31b5 
  
ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
 345c463 


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

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


Testing
---

PENDING...


Thanks,

Jonathan Hurley

Re: Review Request 58489: VersionAdvertised should be set to False by default in stack_tool.py

2017-04-18 Thread Jayush Luniya 

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




ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
Line 72 (original)


Also fix the get_stack_version.py to add null check for stack_selector_path

diff --git 
a/ambari-common/src/main/python/resource_management/libraries/functions/get_stack_version.py
 
b/ambari-common/src/main/python/resource_management/libraries/functions/get_stack_version.py
index 7274a598ed..6926be3c39 100644
--- 
a/ambari-common/src/main/python/resource_management/libraries/functions/get_stack_version.py
+++ 
b/ambari-common/src/main/python/resource_management/libraries/functions/get_stack_version.py
@@ -67,7 +67,7 @@ def get_stack_version(package_name):

   stack_selector_path = 
stack_tools.get_stack_tool_path(stack_tools.STACK_SELECTOR_NAME)

-  if not os.path.exists(stack_selector_path):
+  if not stack_selector_path or not os.path.exists(stack_selector_path):
 Logger.info('Skipping get_stack_version since " + stack_selector_tool 
+ " is not yet available')
 return None # lazy fail


- Jayush Luniya


On April 18, 2017, 12:31 a.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58489/
> ---
> 
> (Updated April 18, 2017, 12:31 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-20775
> https://issues.apache.org/jira/browse/AMBARI-20775
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> All the commands.json files should always contain the versionAdvertised 
> field. If versionAdvertised is not set in command.json then we should assume 
> that versionAdvertised=False when calling stack_tools.py
> 
> 
> Diffs
> -
> 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
>  93ec0b74f5 
>   
> ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
>  03f53d490b 
> 
> 
> Diff: https://reviews.apache.org/r/58489/diff/1/
> 
> 
> Testing
> ---
> 
> Tested for service actions:
> Start
> Stop
> Restart
> Reconfigure
> Service Check
> Enable HA
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 58489: VersionAdvertised should be set to False by default in stack_tool.py

2017-04-18 Thread Jayush Luniya 

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




ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
Line 49 (original), 49 (patched)


I believe you will need to fix the unit test command.json files to set the 
versionAdvertised correctly otherwise it will cause the unit tests to fail.


- Jayush Luniya


On April 18, 2017, 12:31 a.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58489/
> ---
> 
> (Updated April 18, 2017, 12:31 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-20775
> https://issues.apache.org/jira/browse/AMBARI-20775
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> All the commands.json files should always contain the versionAdvertised 
> field. If versionAdvertised is not set in command.json then we should assume 
> that versionAdvertised=False when calling stack_tools.py
> 
> 
> Diffs
> -
> 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
>  93ec0b74f5 
>   
> ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
>  03f53d490b 
> 
> 
> Diff: https://reviews.apache.org/r/58489/diff/1/
> 
> 
> Testing
> ---
> 
> Tested for service actions:
> Start
> Stop
> Restart
> Reconfigure
> Service Check
> Enable HA
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

2017-04-18 Thread Oliver Szabo 

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




ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java
Lines 131 (patched)


Do not use inline comments, maybe its better to use private methods with 
names that describe the behaviour.



ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java
Lines 132 (patched)


Do not use AMBARI.ADMINISTRATOR as a constant, better to be a configuration 
property (external auth admin role or something like that).
Also if you change the behaviour you will need to change the description of 
the parameter in the logsearch stack code (in ambari-server common-services)

Anyway if the "AMBARI.ADMINISTRATOR" role is not in the roles allowed list 
I would not expect the user to log into Log Search UI. (that not violates 
Ambari Administrator privilege as that is a different service). On Ambari UI 
side, Ambari uses its own rest API to get logSearch data, and for that 
(internally) it uses the default logsearch admin user (+ password), but to 
access to that rest endpoint you need the proper privileges, that should work 
as you expected on the Ambari UI. Also that means you will need to test your 
changes with the default admin/password as well (which you can set during 
logsearch install through Ambari)

I would say as your first point seems to be an issue, maybe the second one 
is not.


- Oliver Szabo


On April 18, 2017, 5:33 a.m., Keta Patel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> ---
> 
> (Updated April 18, 2017, 5:33 a.m.)
> 
> 
> Review request for Ambari.
> 
> 
> Bugs: AMBARI-20768
> https://issues.apache.org/jira/browse/AMBARI-20768
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> A local Ambari user with no cluster roles assigned to it can successfully log 
> into the Logsearch UI.
> 
> Logsearch service exercises restriction on who can access its UI using a 
> property "logsearch.roles.allowed". This property is a comma-separated list 
> of roles to be allowed access to Logsearch UI. This defect deals with the 
> following 2 issues:
> 1. If Logsearch service requires that only certain roles be allowed to access 
> its UI, then a local Ambari user with no roles must not be allowed to access 
> the UI.
> 2. If some user with privilege to edit the config properties, updates 
> "logsearch.roles.allowed" by removing the "AMBARI.ADMINISTRATOR" role from 
> its list, then the Ambari Admins will not be able to access the Logsearch UI. 
> This violates the Ambari Administrator privilege which must be able to access 
> all frames of Ambari UI as well as perform all UI operations.
> 
> 
> DESIRED BEHAVIOR:
> =
> 1. A local user with no role assigned to it, must not be able to access 
> Logsearch UI.
> 2. Ambari Administrators must be always be allowed to access the Logsearch 
> UI. No user is allowed to revoke this access right of Ambari Administrator 
> for the Logsearch UI.
> 
> 
> Diffs
> -
> 
>   
> ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java
>  e23f0a2 
> 
> 
> Diff: https://reviews.apache.org/r/58493/diff/1/
> 
> 
> Testing
> ---
> 
> The patch *AMBARI-20768.patch* contains the fix for this issue. The fix 
> involves correction in 2 places in the 
> LogsearchExternalServerAuthenticationProvider class.
> 1. In order to prevent a local user with no cluster roles assigned to it from 
> logging into Logsearch UI, we return *false*.
> 2. We implicitly check whether the user is an Ambari Administrator or not, 
> thus removing the requirement of having "AMBARI.ADMINISTRATOR" role in the 
> "logsearch.roles.allowed" property on the UI. Now, even if some user removes 
> the "AMBARI.ADMINISTRATOR" property from the UI, it will not affect the 
> Ambari admin's accessibility to the Logsearch UI. Ambari Admins will always 
> be allowed to login.
> 
> The results of the logsearch tests after applying the patch are shown in the 
> screenshot "all_tests_successful.png" on the Jira.
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Re: Review Request 58489: VersionAdvertised should be set to False by default in stack_tool.py

2017-04-18 Thread Nate Cole 


> On April 17, 2017, 8:36 p.m., Alejandro Fernandez wrote:
> > ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
> > Line 52 (original), 52 (patched)
> > 
> >
> > Why would service_name be the string "null"?
> 
> Madhuvanthi Radhakrishnan wrote:
> During install_packages roleCommand, the service name is set to "null"
> /var/lib/ambari-agent/data/command-5.json:"serviceName": "null",

If serviceName is the string "null" then that's pretty lame. :)  That should be 
fixed.


- Nate


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


On April 17, 2017, 8:31 p.m., Madhuvanthi Radhakrishnan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58489/
> ---
> 
> (Updated April 17, 2017, 8:31 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-20775
> https://issues.apache.org/jira/browse/AMBARI-20775
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> All the commands.json files should always contain the versionAdvertised 
> field. If versionAdvertised is not set in command.json then we should assume 
> that versionAdvertised=False when calling stack_tools.py
> 
> 
> Diffs
> -
> 
>   
> ambari-common/src/main/python/resource_management/libraries/functions/stack_tools.py
>  93ec0b74f5 
>   
> ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/package/scripts/params_linux.py
>  03f53d490b 
> 
> 
> Diff: https://reviews.apache.org/r/58489/diff/1/
> 
> 
> Testing
> ---
> 
> Tested for service actions:
> Start
> Stop
> Restart
> Reconfigure
> Service Check
> Enable HA
> 
> 
> Thanks,
> 
> Madhuvanthi Radhakrishnan
> 
>

Re: Review Request 58483: Service Upgrade VDF Creates Host Version Entries For All Hosts With INSTALLING

2017-04-18 Thread Nate Cole 

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


Ship it!




Ship It!

- Nate Cole


On April 17, 2017, 5:22 p.m., Jonathan Hurley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58483/
> ---
> 
> (Updated April 17, 2017, 5:22 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko and Nate Cole.
> 
> 
> Bugs: AMBARI-20774
> https://issues.apache.org/jira/browse/AMBARI-20774
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> When a VDF for a {{SERVICE}} upgrade targets only a subset of hosts in the 
> cluster, there is a host version entry created for all hosts with the state 
> of {{INSTALLING}}. This causes the web client to think that the installation 
> is not complete and it prevents the upgrade button from displaying.
> 
> STR:
> - Install a cluster with ZK and Storm
> -- ZK should be on all 3 hosts, Storm only on 2
> - Upload a VDF for Storm only and distribute it
> 
> With the introduction of PATCH/SERVICE repositories, we need to properly set 
> the host_version based on a variety of factors:
> - NOT_REQUIRED if the host has no versionable components or has no components 
> targetted by the repository
> - OUT_OF_SYNC _only_ if the host is supposed to have the repository installed 
> but is in MM
> - INSTALLING for all others (INSTALLED when forced)
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java
>  6e08e7b 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
> c961995 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
>  b7cc4cd 
>   
> ambari-server/src/test/java/org/apache/ambari/server/checks/ServicesNamenodeTruncateCheckTest.java
>  df1aa37 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProviderTest.java
>  5ef31b5 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
>  345c463 
> 
> 
> Diff: https://reviews.apache.org/r/58483/diff/1/
> 
> 
> Testing
> ---
> 
> PENDING...
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Review Request 58497: AMBARI-20779 Create Ranger KMS HDFS audit folder as part of install

2017-04-18 Thread Mugdha Varadkar 

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

Review request for Ambari, Alejandro Fernandez, Gautam Borad, Sumit Mohanty, 
and Velmurugan Periasamy.


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


Repository: ambari


Description
---

Create audit directory /ranger/audit/kms in hdfs, if 
xasecure.audit.destination.hdfs (Audit To HDFS) property is set to true.


Diffs
-

  
ambari-server/src/main/resources/common-services/RANGER/0.5.0/role_command_order.json
 df62dfd 
  
ambari-server/src/main/resources/common-services/RANGER_KMS/0.5.0.2.3/metainfo.xml
 b20201c 
  
ambari-server/src/main/resources/common-services/RANGER_KMS/0.5.0.2.3/package/scripts/kms.py
 423cdec 
  
ambari-server/src/main/resources/common-services/RANGER_KMS/0.5.0.2.3/package/scripts/params.py
 9fe0a61 
  
ambari-server/src/main/resources/common-services/RANGER_KMS/0.5.0.2.3/role_command_order.json
 006d177 
  ambari-server/src/main/resources/stacks/HDP/2.3/role_command_order.json 
4f279c6 
  ambari-server/src/test/python/stacks/2.5/RANGER_KMS/test_kms_server.py 
7082a33 
  ambari-server/src/test/python/stacks/2.5/configs/ranger-kms-secured.json 
7054e8f 


Diff: https://reviews.apache.org/r/58497/diff/1/


Testing
---

Unit Test Result:

Running tests for stack:2.5 service:RANGER_KMS
test_configure_default (test_kms_server.TestRangerKMS) ... 2017-04-18 
13:13:22,030 - Stack Feature Version Info: stack_version=2.5, 
version=2.5.0.0-777, current_cluster_version=2.5.0.0-777 -> 2.5.0.0-777
2017-04-18 13:13:22,046 - Using hadoop conf dir: 
/usr/hdp/current/hadoop-client/conf
ok
test_configure_secured (test_kms_server.TestRangerKMS) ... 2017-04-18 
13:13:22,062 - Stack Feature Version Info: stack_version=2.5, 
version=2.5.0.0-801, current_cluster_version=2.5.0.0-801 -> 2.5.0.0-801
2017-04-18 13:13:22,080 - Using hadoop conf dir: 
/usr/hdp/current/hadoop-client/conf
ok
test_start_default (test_kms_server.TestRangerKMS) ... 2017-04-18 13:13:22,095 
- Stack Feature Version Info: stack_version=2.5, version=2.5.0.0-777, 
current_cluster_version=2.5.0.0-777 -> 2.5.0.0-777
2017-04-18 13:13:22,111 - Using hadoop conf dir: 
/usr/hdp/current/hadoop-client/conf
2017-04-18 13:13:22,116 - Rangeradmin: Skip ranger admin if it's down !
ok
test_start_secured (test_kms_server.TestRangerKMS) ... 2017-04-18 13:13:22,129 
- Stack Feature Version Info: stack_version=2.5, version=2.5.0.0-801, 
current_cluster_version=2.5.0.0-801 -> 2.5.0.0-801
2017-04-18 13:13:22,145 - Using hadoop conf dir: 
/usr/hdp/current/hadoop-client/conf
2017-04-18 13:13:22,150 - RangeradminV2: Skip ranger admin if it's down !
2017-04-18 13:13:22,151 - KMS repository c1_kms exist
ok
test_stop_default (test_kms_server.TestRangerKMS) ... 2017-04-18 13:13:22,163 - 
Stack Feature Version Info: stack_version=2.5, version=2.5.0.0-777, 
current_cluster_version=2.5.0.0-777 -> 2.5.0.0-777
2017-04-18 13:13:22,179 - Using hadoop conf dir: 
/usr/hdp/current/hadoop-client/conf
ok

--
Ran 5 tests in 0.164s

OK


Thanks,

Mugdha Varadkar