METRON-1695: Expose pcap properties through Ambari

[anandsubbu]

Contributor Comments

This change add a new jinja template for the pcap properties and adds a new tab in Ambari Metron config for the PCAP properties. Here are the screenshots:

Testing Done
Built mpack out of these changes, deployed a multi-node cluster and validated that the pcap.properties is being updated during Parser startup scripts per the screenshot below:

Also validated that the pcap properties is updated with config changes (ZK quorum, kafka broker etc.)

[root@metron-12 config]# cat pcap.properties

##### Storm #####
topology.worker.childopts=
topology.auto-credentials=[]
topology.workers=1

##### Kafka #####
spout.kafka.topic.pcap=pcap
kafka.zk=metron-10:2181,metron-11:2181,metron-1:2181
hdfs.sync.every=1
hdfs.replication.factor=--1
kafka.security.protocol=PLAINTEXT

# One of EARLIEST, LATEST, UNCOMMITTED_EARLIEST, UNCOMMITTED_LATEST
kafka.pcap.start=UNCOMMITTED_EARLIEST

kafka.pcap.numPackets=1000
kafka.pcap.maxTimeMS=300000
kafka.pcap.ts_scheme=FROM_KEY
kafka.pcap.out=/apps/metron/pcap/input

##### Parallelism #####
kafka.pcap.ts_granularity=MICROSECONDS
kafka.spout.parallelism=1

Verification Steps

• Spin up full dev (or a multi-node cluster)
• Ensure that the PCAP tab in Ambari Metron config allows for values to be entered.
• Deploy cluster and validate that $METRON_HOME/config/pcap.properties file has the proper config values set.

Pull Request Checklist

Thank you for submitting a contribution to Apache Metron.
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines 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:

• Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
• Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
• Has your PR been rebased against the latest commit within the target branch (typically master)?

For code changes:

• Have you included steps to reproduce the behavior or problem that is being changed or addressed?

• Have you included steps or a guide to how the change may be verified and tested manually?

• Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:

  mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh 
  

• Have you written or updated unit tests and or integration tests to verify your changes?

• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

• Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

For documentation related changes:

• 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 is set up for your personal repository such that your branches are built there before submitting a pull request.

[anandsubbu]

I had earlier placed this under the indexing_master.py script, but felt that it is more related to parser than with indexing. I am happy to move this block to any other place that is deemed fit.

[anandsubbu]

The PCAP config readme does not talk more about the units for this parameter (whether seconds, or minutes or something else). I looked through the code as well, but was not able to find more info. If someone could clarify about this, I can add the units so it is clearer.

[mmiklavc]

@anandsubbu Thanks for contributing this! The only thing I'm unsure of after reviewing this is your comment regarding this belonging in parser_master.py or not. On one hand, you could make a case that this is a parser like any other with the exception that it is spout-only. On the other hand, we don't currently provide a mechanism (iirc) to start/stop/manage the PCAP topology from Ambari. Starting parsers doesn't start PCAP, does it?

It may be that PCAP deserves its own "master", e.g. pcap_master.py, and lifecycle as a proper first class citizen (topology) within Ambari. Below is the current list of components, and PCAP is not in it afaik, even as a parser (there's a start_pcap_topology.sh script that is NOT the same as start_parser_topology.sh). It seems like if we're going to add this to Ambari we should also provide the ability to manage it. What do you think of that approach?

[anandsubbu]

@mmiklavc thanks for the review. I concur with your observations. PCAP topology does deserve its own place in Ambari and the Management UI.

As far as the scope of this PR goes, this IMHO, would be a first cut towards it. This PR addresses the immediate need for exposing the parameters in pcap.properties via Ambari. It also addresses auto-populating the ZK quorum in pcap.properties thus simplifying the manual steps that would be otherwise required in a multi-node deployment.

Would you be okay if we create a separate JIRA to cover the broader change? Let me know your thoughts.

[simonellistonball]

+1 to that, PCAP should definitely get its own service, but agreed with @anandsubbu that should probably be a follow on item.

[mmiklavc]

I think I'm ok with this as a first step. Please document the lifecycle for how pcap.properties is updated in the README. It should be clear to users that they will need to restart parsers for the changes to take effect after the initial install is performed. Can you confirm, is that the way the properties are/will be deployed with this PR @anandsubbu?

#1134 should be included in the refactoring once we make PCAP a proper component.

[mmiklavc]

I'm +1 via inspection pending the requested documentation. Also, please create the follow-on Jira, have it linked to this Jira, and also link it here for reference in the comments on this PR. Both this Jira/PR and #1134 (comment) should link up.

[anandsubbu]

Please document the lifecycle for how pcap.properties is updated in the README. It should be clear to users that they will need to restart parsers for the changes to take effect after the initial install is performed. Can you confirm, is that the way the properties are/will be deployed with this PR @anandsubbu?

Hi @mmiklavc - there is no change in behavior from earlier to now. Nothing changes from a usage perspective. Let me clarify more with an example for a multi-node deployment...

Earlier

• User deploys metron
• pcap.properties file created with defaults under $METRON_HOME/config
• User hand edits the properties file (sets ZK quorum and other parameters as required)
• Starts the topology per the instructions here.

Now

• During deploy time, user is presented with a separate config tab in Ambari to configure/reconfigure PCAP properties. If user chooses to leave them untouched, the pcap.properties is initialized with appropriate defaults. ZK quorum is auto-populated as well.
• pcap.properties file is created during metron-parsers startup step.
• Starts the topology per the instructions here.

The steps in the README still holds.

Now, we can add a note in the README to indicate that the PCAP properties can be configured via the 'PCAP' tab in Ambari Metron config. But I noticed this was not explicitly mentioned for other components (e.g. REST, elasticsearch). Let me know if you prefer to have this added.

[anandsubbu]

Btw, I created METRON-1719 to track PCAP sensor being an independent service entity.

[mmiklavc]

@anandsubbu, considering the changes in this PR:

1. Let's say I manually make changes to pcap.properties. I restart the parser topologies. Later, I manually restart pcap with the shell command. What happens to my pcap.properties changes?
2. Similarly, I make changes via Ambari's new PCAP panel. Now I go start pcap using $METRON_HOME/bin/start_pcap_topology.sh. What does pcap.properties look like?

In 1, your changes are overwritten by the parsers' restart. In 2, you've made changes in Ambari but they don't get materialized in the properties file because you haven't restarted the parsers - I may be mistaken, but I'm pretty sure clicking "save" in Ambari does not deploy the new properties, and this is the reason I ask for a doc change. Previously, configuring pcap was all manual steps. Now it will be a combo of manual steps for start/stop, automated steps for the config updates. And more specifically for the config updates, you will only see them if you make them in Ambari and restart the parsers. Also, if you choose not to use Ambari for this config handling and instead make your config changes to pcap.properties locally, you will have to remember that restarting the parsers will overwrite your pcap config - something you're only likely to realize the next time you start/restart pcap. This is the behavior that I think we should document if we accept this. Does that make sense?

[anandsubbu]

Yup, @mmiklavc I see what you are saying. Make sense to me. Let me know if the latest README update conveys the message.

I also added a fix to prompt a service restart upon changes to the PCAP config settings. Thanks @MohanDV for the pointer!

[mmiklavc]

@anandsubbu thanks for the updates, I'm going merge your PR.

[merrimanr]

I don't see any good reason to put off making pcap it's own Ambari component. Having to restart the parsers component (which is unrelated to pcap) to propagate pcap changes is a deal breaker for me.

Had this been submitted against the pcap feature branch I would be fine with it as an incremental change but I don't feel like this puts master in a good state and I definitely would not want it in a release. Why not just do it here?

[mmiklavc]

@anandsubbu, I hate to give you whiplash, but @merrimanr may have a point here. There benefit of exposing the pcap properties via Ambari is lessened by now requiring the user to do 3 things instead of 2 to get the properties set for Pcap.

Old

1. Update properties
2. Restart pcap from command line

New

1. Update properties
2. Restart parsers to deploy the properties (users may not like this requirement)
3. Restart pcap from command line

The work is by no means throw away in its current form, but if we're going to do this it's probably worth going the whole way.

[anandsubbu]

This PR lays the foundation for exposing the properties to begin with. My thought was that it reduces one error prone manual step of hand-editing the pcap.properties file. I agree with your point about how users might be concerned about restarting all parsers after modifying PCAP config.

I did not create this earlier under the feature branch since this was not related specifically to the PCAP query panel, it was a more generic change.

I am fine to move this under the feature branch or I could wait until the fix for METRON-1709 (making the PCAP service its own) is available and then submit a fresh PR. Let me know which one works better.

[merrimanr]

If you want to do the work in a separate PR (METRON-1709) then that's fine. As long as they are tested and committed together that works for me. If it were me, I would just do the work in this PR and save the trouble of managing two different PRs. We have several different components already that you can use as a template. I don't think this is that much work.

Since this was developed against master I wouldn't switch to the feature branch.

[anandsubbu]

Sure @merrimanr , sounds good.

@MohanDV has already begun work on METRON-1709. I will wait for him to complete and then submit this pull request so it would be a natural fit.

[nickwallen]

@anandsubbu I am conflicted on this one. :) Can you de-conflict?

[anandsubbu]

Hey @nickwallen ... Let me close this PR. Will create one afresh based on the latest changes from #1201