[GitHub] metron pull request #799: METRON-1249 Improve Metron MPack service checks

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/799


---

[GitHub] metron pull request #799: METRON-1249 Improve Metron MPack service checks

[nickwallen]

Github user nickwallen commented on a diff in the pull request:

https://github.com/apache/metron/pull/799#discussion_r144955205
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py
 ---
@@ -201,15 +200,123 @@ def init_kafka_topics(params, topics):
 user=params.kafka_user, tries=3, try_sleep=5, logoutput=True)
   Logger.info("Done creating Kafka topics")
 
-def init_kafka_acls(params, topics, groups):
-  Logger.info('Creating Kafka ACLs')
 
+def check_kafka_topics(params, topics):
+
+  if params.security_enabled:
+kinit(params.kinit_path_local,
+  params.metron_keytab_path,
+  params.metron_principal_name,
+  execute_user=params.metron_user)
+
+  cmd = """{0}/kafka-topics.sh \
+--zookeeper {1} \
+--list | \
+awk 'BEGIN {{cnt=0;}} /{2}/ {{cnt++}} END {{if (cnt > 0) {{exit 0}} 
else {{exit 1'"""
+  for topic in topics:
+Logger.info("Checking existence of Kafka topic '{0}'".format(topic))
+try:
+  Execute(
--- End diff --

I refactored this a bit and added some pydocs.


---

[GitHub] metron pull request #800: METRON-1251: Typo and formatting fixes for metron-...

[JonZeolla]

GitHub user JonZeolla opened a pull request:

https://github.com/apache/metron/pull/800

METRON-1251: Typo and formatting fixes for metron-rest README

## Contributor Comments
This fix properly format the github md in addition to the site-book docs 
(previously the github formatting was broken).

## Pull Request Checklist

Thank you for submitting a contribution to Apache Metron.  
Please refer to our [Development 
Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235)
 for the complete guide to follow for contributions.  
Please refer also to our [Build Verification 
Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview)
 for complete smoke testing guides.  


In order to streamline the review of the contribution we ask you follow 
these guidelines and ask you to double check the following:

### For all changes:
- [X] Is there a JIRA ticket associated with this PR? If not one needs to 
be created at [Metron 
Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
 
- [X] Does your PR title start with METRON- where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
- [X] Has your PR been rebased against the latest commit within the target 
branch (typically master)?

### For documentation related changes:
- [X] Have you ensured that format looks appropriate for the output in 
which it is rendered by building and verifying the site-book? If not then run 
the following commands and the verify changes via 
`site-book/target/site/index.html`:

  ```
  cd site-book
  mvn site
  ```

 Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and submit an update to your PR as soon as possible.
It is also recommended that [travis-ci](https://travis-ci.org) is set up 
for your personal repository such that your branches are built there before 
submitting a pull request.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/JonZeolla/metron METRON-1251

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/metron/pull/800.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #800


commit 6dc567456ef740420fb79c1f707649b3c295b01e
Author: Jon Zeolla 
Date:   2017-10-16T19:04:28Z

METRON-1251 - Typo and formatting fixes for metron-rest README




---

[GitHub] metron issue #682: METRON-1081: Fix Alerts and Ops UI Notices file

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/metron/pull/682
  
Sorry otto, I didn't see that comment, I'll admit.  I apologize.  Let me 
take a look at them and if there's further notices work to do, I'll make a 
follow-on PR.  Again, sorry, ignoring your discrepancy wasn't my intent.


---

[GitHub] metron issue #579: METRON-941 fix PaloAltoParser

[james-sirota]

Github user james-sirota commented on the issue:

https://github.com/apache/metron/pull/579
  
Hi we would like to get this into the next release.  @ctramnitz we'll be 
happy to help you fix it


---

[GitHub] metron issue #682: METRON-1081: Fix Alerts and Ops UI Notices file

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/682
  
I assume we are doing this for apache compliance.  Our acceptance of this 
variance is compliant then.  That was not my understanding.  But I'm sure the 
chair knows. 
Thanks


---

[GitHub] metron pull request #792: METRON-1237 address javadoc warnings in metron-maa...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/792


---

[GitHub] metron pull request #794: METRON-1240 address javadoc warnings in metron-pla...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/794


---

[GitHub] metron issue #682: METRON-1081: Fix Alerts and Ops UI Notices file

[simonellistonball]

Github user simonellistonball commented on the issue:

https://github.com/apache/metron/pull/682
  
@ottobackwards I just re-ran this. We're not fixing versions in the UIs at 
the moment, so I would expect this kind of minor variance. This is mainly to 
ensure licenses are at least covered. The solution to the version fixing is the 
yarn stuff we're discussing elsewhere.


---

[GitHub] metron issue #794: METRON-1240 address javadoc warnings in metron-platform a...

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/metron/pull/794
  
+1 by inspection.  Sorry this slipped through the cracks!


---

[GitHub] metron pull request #799: METRON-1249 Improve Metron MPack service checks

[cestella]

Github user cestella commented on a diff in the pull request:

https://github.com/apache/metron/pull/799#discussion_r144896412
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/enrichment_commands.py
 ---
@@ -232,3 +241,48 @@ def set_hbase_acls(self):
 
 Logger.info("Done setting HBase ACLs")
 self.set_hbase_acl_configured()
+
+def service_check(self, env):
+"""
+Performs a service check for Enrichment.
+:param env: Environment
+"""
+Logger.info("Checking for Geo database")
+metron_service.check_hdfs_file_exists(self.__params, 
self.__params.geoip_hdfs_dir + "/GeoLite2-City.mmdb.gz")
--- End diff --

Yeah, we just have the HDFS directory, which is what you're using.  For 
this PR, I think it's good.  I'd hate to make the perfect the enemy of the good.

Going forward, what would be cool is if we could execute a stellar script 
via the REPL and if it fails, fail the service check.  Since the REPL can 
interact with global config parameters and has the capability to validate HDFS, 
that would be a clean way to do this.


---

[GitHub] metron pull request #793: METRON-1226 Searching Can Errantly Query the Wrong...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/793


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[nickwallen]

Github user nickwallen commented on the issue:

https://github.com/apache/metron/pull/799
  
@ottobackwards Thanks.  Your points definitely warrant a discussion.  Just 
wanted to make sure we're green lighted on this PR.


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/799
  
@nickwallen no, the problem exists before already


---

[GitHub] metron issue #682: METRON-1081: Fix Alerts and Ops UI Notices file

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/682
  
So, no comment on the discrepancy I found then?



---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[nickwallen]

Github user nickwallen commented on the issue:

https://github.com/apache/metron/pull/799
  
@ottobackwards Do you have any concerns that need addressed in this PR?


---

[GitHub] metron pull request #682: METRON-1081: Fix Alerts and Ops UI Notices file

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/metron/pull/682


---

[GitHub] metron issue #682: METRON-1081: Fix Alerts and Ops UI Notices file

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/metron/pull/682
  
+1 by inspection


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/metron/pull/799
  
Really, what I'd like to see is a status on the parsers in the management 
UI that indicates that those parsers should be running all the time and a REST 
call to validate that they are running or not.  I'd then expect ambari to call 
the REST call and fail the service check if any of the installed sensors that 
are marked as running aren't running.  

What we have now is that list in Ambari and ambari managing to start them 
and stop them.  I'd prefer to delegate teh starting and stopping of sensors to 
the management UI and JUST have ambari interact with the REST API to indicate 
whether the current state is nominal.  That, however, is really more of a topic 
for a discuss thread, though.


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/799
  
But I am taking what you are saying about managing parsers to mean 
'managing configured sensors is not done in ambari, but managing the services 
is'


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/799
  
untangling ambari and zookeeper would be required.  If we want ( as I think 
we need to ) to be able to manage the parsers from the ui / rest, but also have 
the ambari service management still work ( restart all effected services ).  
I'm not sure if is more than having ambari read and write to zookeeper for the 
all_parsers list or not


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/metron/pull/799
  
Yes, that's right.  In my opinion, though, we should probably stop managing 
parsers in ambari and just focus on the management UI to have one place to 
manage all parsers.


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[ottobackwards]

Github user ottobackwards commented on the issue:

https://github.com/apache/metron/pull/799
  
If my understanding is correct, this will work for all parser topologies 
listed to automatically start through the ambari configuration, but not all 
parser topologies that may in fact be running, as we still have the disconnect 
between the management ui not working with ambari.  That is to say, topologies 
started through the management ui will **not** be tracked by this.



---

[GitHub] metron issue #793: METRON-1226 Searching Can Errantly Query the Wrong Indice...

[mmiklavc]

Github user mmiklavc commented on the issue:

https://github.com/apache/metron/pull/793
  
+1 by inspection


---

[GitHub] metron issue #799: METRON-1249 Improve Metron MPack service checks

[cestella]

Github user cestella commented on the issue:

https://github.com/apache/metron/pull/799
  
I really like this.  Modulo a couple of very minor nits, I'm +1 by 
inspection.


---

[GitHub] metron pull request #799: METRON-1249 Improve Metron MPack service checks

[cestella]

Github user cestella commented on a diff in the pull request:

https://github.com/apache/metron/pull/799#discussion_r144868387
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py
 ---
@@ -201,15 +200,123 @@ def init_kafka_topics(params, topics):
 user=params.kafka_user, tries=3, try_sleep=5, logoutput=True)
   Logger.info("Done creating Kafka topics")
 
-def init_kafka_acls(params, topics, groups):
-  Logger.info('Creating Kafka ACLs')
 
+def check_kafka_topics(params, topics):
+
+  if params.security_enabled:
+kinit(params.kinit_path_local,
+  params.metron_keytab_path,
+  params.metron_principal_name,
+  execute_user=params.metron_user)
+
+  cmd = """{0}/kafka-topics.sh \
+--zookeeper {1} \
+--list | \
+awk 'BEGIN {{cnt=0;}} /{2}/ {{cnt++}} END {{if (cnt > 0) {{exit 0}} 
else {{exit 1'"""
+  for topic in topics:
+Logger.info("Checking existence of Kafka topic '{0}'".format(topic))
+try:
+  Execute(
--- End diff --

Any chance we could make a function rather than cutting and pasting?  
Something like:
```
def exec( cmd, user_as, fail_msg ) :
try:
  Execute(cmd,
tries=3,
try_sleep=5,
logoutput=True,
path='/usr/sbin:/sbin:/usr/local/bin:/bin:/usr/bin',
user=user_as)
except:
raise Fail(fail_msg)
```


---

[GitHub] metron issue #754: METRON-1184 EC2 Deployment - Updating control_path to acc...

[as22323]

Github user as22323 commented on the issue:

https://github.com/apache/metron/pull/754
  
Hello, 
Just wondering if what needs to be done next to get this committed.




---

[GitHub] metron pull request #799: METRON-1249 Improve Metron MPack service checks

[nickwallen]

Github user nickwallen commented on a diff in the pull request:

https://github.com/apache/metron/pull/799#discussion_r144863737
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/enrichment_commands.py
 ---
@@ -232,3 +241,48 @@ def set_hbase_acls(self):
 
 Logger.info("Done setting HBase ACLs")
 self.set_hbase_acl_configured()
+
+def service_check(self, env):
+"""
+Performs a service check for Enrichment.
+:param env: Environment
+"""
+Logger.info("Checking for Geo database")
+metron_service.check_hdfs_file_exists(self.__params, 
self.__params.geoip_hdfs_dir + "/GeoLite2-City.mmdb.gz")
--- End diff --

Ok, I see it now under a global properties key "geo.hdfs.file".  There is 
nothing in Ambari MPack for it, which might complicate using it.  I am just 
thinking through, if we would need to first introduce it as an Ambari-managed 
configuration value.


---

[GitHub] metron pull request #799: METRON-1249 Improve Metron MPack service checks

[nickwallen]

Github user nickwallen commented on a diff in the pull request:

https://github.com/apache/metron/pull/799#discussion_r144858358
  
--- Diff: 
metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/enrichment_commands.py
 ---
@@ -232,3 +241,48 @@ def set_hbase_acls(self):
 
 Logger.info("Done setting HBase ACLs")
 self.set_hbase_acl_configured()
+
+def service_check(self, env):
+"""
+Performs a service check for Enrichment.
+:param env: Environment
+"""
+Logger.info("Checking for Geo database")
+metron_service.check_hdfs_file_exists(self.__params, 
self.__params.geoip_hdfs_dir + "/GeoLite2-City.mmdb.gz")
--- End diff --

I agree.  I don't like hard-coding "GeoLite2-City.mmdb.gz" here.  I just 
didn't see anywhere in the Mpack where we had that value parameterized already.

Where in the code do we have it parameterized?  I'd love to fix this.




---

[GitHub] metron pull request #712: METRON-1129 Management UI - Package Node Dependenc...

[juanjoDiaz]

Github user juanjoDiaz commented on a diff in the pull request:

https://github.com/apache/metron/pull/712#discussion_r144801898
  
--- Diff: metron-deployment/packaging/docker/rpm-docker/build.sh ---
@@ -36,7 +36,7 @@ if [ $? -ne 0 ] && [ $OWNER_UID -ne 0 ]; then
 fi
 
 rm -rf SRPMS/ RPMS/ && \
-rpmbuild -v -ba --define "_topdir $(pwd)" --define "_version ${VERSION}" 
--define "_prerelease ${PRERELEASE}" SPECS/metron.spec && \
+QA_SKIP_BUILD_ROOT=1 rpmbuild -v -ba --define "_topdir $(pwd)" --define 
"_version ${VERSION}" --define "_prerelease ${PRERELEASE}" SPECS/metron.spec && 
\
--- End diff --

Just saw this by chance. I can recommend using 
https://www.npmjs.com/package/removeNPMAbsolutePaths which is already use by 
other Apache projects like Cordova to remove the absolute paths created by NPM.


---