Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add igmp and mroute Services #762

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

scetron
Copy link

@scetron scetron commented Jul 13, 2022

Signed-off-by: scetron [email protected]

Test inclusion requirements

In case the PR contains an enhancement or a new platform/service support, some tests have to be added for the new functionality:

  • any fix or enhancement SHOULD include relevant new tests or test updates, if any tests need updating.
  • a new platform support MUST include the relevant input files similar to what we have in tests/integration/sqcmds/-input directories, along with the relevant tests in the tests/integration/sqcmds/-samples dir. That list MUST include the all.yml file fully filled out.
  • any new service (or table) addition MUST include comments about what network OS are supported (along with version) with this command along with test samples for those platforms and input files in the *-input dir

For additional information about tests, follow this link

Related Issue

Fixes #377

Description

Would like to add support for collection mroute and igmp information to increase the observability of suzieq to the multicast side of the network. This will increase the usefulness and applicability of suzieq in video and financial related networks as well as lay a foundation for VXLAN overlay networks.

Type of change

  • New feature (non-breaking change which adds functionality)

New Behavior

Add mroute and igmp services. Enable collection of data for igmp and mroute services for EOS and NXOS.
...

Contrast to Current Behavior

No current collection for the above two services.

Discussion: Benefits and Drawbacks

...

Changes to the Documentation

...

Proposed Release Note Entry

Add mroute and igmp service with support for NXOS and EOS.

Comments

Double Check

  • I have read the comments and followed the CONTRIBUTING.md.
  • I have explained my PR according to the information in the comments or in a linked issue.
  • My PR source branch is created from the develop branch.
  • My PR targets the develop branch.
  • All my commits have --signoff applied

Copy link
Collaborator

@LucaNicosia LucaNicosia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Stephen, thanks for submitting the PR.
There are still some problems you should fix:

  • the .output files you checked in for igmp seems not correct. To update the files you should delete the content of tests/integration/sqcmds/igmp-input/ and run sq-poller --run-once=gather --output-dir tests/integration/sqcmds/igmp-input/.
  • add mroute and igmp marks in the pytest.ini file
  • To update also the parquet testing data correctly, execute SUZIEQ_POLLER=data pytest tests/integration/test_update_data.py -m "mroute" and SUZIEQ_POLLER=data pytest tests/integration/test_update_data.py -m "igmp"
  • delete the mroutes.output file from the tests/integration/sqcmds/eos-input/. Since you didn't run the mroute on that topology, the data won't be correct.
  • please check the linting errors running pylint suzieq and flake8 suzieq

suzieq/config/igmp.yml Outdated Show resolved Hide resolved
suzieq/config/mroutes.yml Outdated Show resolved Hide resolved
suzieq/config/igmp.yml Outdated Show resolved Hide resolved
suzieq/engines/pandas/igmp.py Outdated Show resolved Hide resolved
suzieq/engines/pandas/mroutes.py Outdated Show resolved Hide resolved
suzieq/restServer/query.py Show resolved Hide resolved
suzieq/config/schema/igmp.avsc Show resolved Hide resolved
suzieq/config/schema/mroutes.avsc Show resolved Hide resolved
suzieq/engines/pandas/igmp.py Show resolved Hide resolved
suzieq/engines/pandas/mroutes.py Show resolved Hide resolved
suzieq/config/igmp.yml Show resolved Hide resolved
suzieq/config/mroutes.yml Show resolved Hide resolved
suzieq/config/schema/igmp.avsc Outdated Show resolved Hide resolved
suzieq/config/schema/mroutes.avsc Outdated Show resolved Hide resolved
suzieq/engines/pandas/igmp.py Outdated Show resolved Hide resolved
suzieq/engines/pandas/mroutes.py Outdated Show resolved Hide resolved
suzieq/poller/worker/services/igmp.py Outdated Show resolved Hide resolved
@scetron
Copy link
Author

scetron commented Oct 24, 2022

Hi Stephen, thanks for submitting the PR. There are still some problems you should fix:

  • the .output files you checked in for igmp seems not correct. To update the files you should delete the content of tests/integration/sqcmds/igmp-input/ and run sq-poller --run-once=gather --output-dir tests/integration/sqcmds/igmp-input/.
  • add mroute and igmp marks in the pytest.ini file
  • To update also the parquet testing data correctly, execute SUZIEQ_POLLER=data pytest tests/integration/test_update_data.py -m "mroute" and SUZIEQ_POLLER=data pytest tests/integration/test_update_data.py -m "igmp"
  • delete the mroutes.output file from the tests/integration/sqcmds/eos-input/. Since you didn't run the mroute on that topology, the data won't be correct.
  • please check the linting errors running pylint suzieq and flake8 suzieq
  • add summarize for igmp
  • add summarize for mroutes
  • add example textfsm output for igmp and mroutes

@scetron
Copy link
Author

scetron commented Oct 24, 2022

@LucaNicosia After running SUZIEQ_POLLER=data pytest tests/integration/test_update_data.py -m "mroutes" I get the output

(suzieq-RFE1XB0N-py3.8) ➜  suzieq git:(scetron-mroutes-and-igmp) ✗ SUZIEQ_POLLER=data pytest tests/integration/test_update_data.py -m "mroutes" 
============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /home/scetron/suzieq, configfile: pytest.ini
plugins: forked-1.4.0, asyncio-0.18.3, mock-3.7.0, Faker-4.18.0, depends-1.0.1, icdiff-0.5, xdist-2.5.0, cov-3.0.0
asyncio: mode=auto
gw0 [1] / gw1 [1] / gw2 [1] / gw3 [1] / gw4 [1] / gw5 [1] / gw6 [1] / gw7 [1]
.                                                                                                                                                                                                          [100%]
=============================================================================================== 1 passed in 37.93s ====

However I do not see any updated files? I believe I might be missing a marker here?

@LucaNicosia
Copy link
Collaborator

Hi @scetron, I clone your fork and executed the command you posted in the other comment and the new files are actually added.
I see now stuff about mroutes inside tests/data/parquet directory. Did you checked there if you already have mroutes data?

@scetron scetron force-pushed the scetron-mroutes-and-igmp branch from d0e4230 to 5f8a6a1 Compare January 12, 2024 22:43
@ryanmerolle
Copy link
Contributor

@scetron how can I assist here? I'd be happy to add some eos and ios parsing logic (textfsm templates). Should I PR vs your fork?

@scetron
Copy link
Author

scetron commented Jul 10, 2024

Hello @ryanmerolle! sorry for the late reply. I am having some issues with my lab and making sure we are collecting the correct data. But I just recently have my lab back up and some comments from Dinesh I need to work on.

If you'd like to adjust the templates/collection that is included for IOS that'd be great. I believe that we have EOS covered here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multicast Tables
4 participants