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

Implement a BFDTrapObserver #305

Merged
merged 4 commits into from
Jul 8, 2024
Merged

Implement a BFDTrapObserver #305

merged 4 commits into from
Jul 8, 2024

Conversation

lunkwill42
Copy link
Member

@lunkwill42 lunkwill42 commented Jul 8, 2024

Scope and purpose

Fixes #292. Dependent on #303 to be merged first.

bfdSessUp and bfdSessDown traps only tell which BFD sessions have changed state and why, but does not actually include the new state value for the session. This means most of this new observer's work is to just offload everything to BFDTask to poll the individual session(s) that have changed state and ensure state and events are updated accordingly.

This pull request

  • Adds a new BFD Trap observer to the default Zino trap listener setup.
  • Adds the optional ability for the existing BFDTask to fetch and update single BFD sessions, used by the trap observer.

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Zino can be found in the
README.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation
  • Linted/formatted the code with black, ruff and isort, easiest by using pre-commit
  • The first line of the commit message continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/

This makes `BFDTask` able to fetch and update the BFD state of a single
session, by adding a `session_index` argument to `LinkState`.

The purpose of this is to enable updating the state of single sessions
on incoming BFD traps: BFD traps do not contain the actual state
value changes, they only contain information about the set of BFD
sessions that changed state to something up-like or something down-like.
The changed state values need to be re-polled, but there is no need
to poll the full table in these cases.
@lunkwill42 lunkwill42 self-assigned this Jul 8, 2024
Copy link

github-actions bot commented Jul 8, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 86 0 1.43s
✅ PYTHON isort 86 0 0.31s
✅ PYTHON ruff 86 0 0.03s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link

github-actions bot commented Jul 8, 2024

Test results

    3 files      3 suites   1m 15s ⏱️
  432 tests   432 ✅ 0 💤 0 ❌
1 296 runs  1 294 ✅ 2 💤 0 ❌

Results for commit ec71030.

♻️ This comment has been updated with latest results.

Copy link

sonarcloud bot commented Jul 8, 2024

@lunkwill42 lunkwill42 marked this pull request as ready for review July 8, 2024 13:06
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 98.11321% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.69%. Comparing base (a75b0e8) to head (ec71030).

Files Patch % Lines
src/zino/tasks/bfdtask.py 94.74% 1 Missing ⚠️
Additional details and impacted files
@@                           Coverage Diff                            @@
##           feature/snmp-get2-multiple-variables     #305      +/-   ##
========================================================================
- Coverage                                 98.70%   98.69%   -0.01%     
========================================================================
  Files                                        53       54       +1     
  Lines                                      7055     7100      +45     
========================================================================
+ Hits                                       6963     7007      +44     
- Misses                                       92       93       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@johannaengland johannaengland left a comment

Choose a reason for hiding this comment

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

But I would like to have an updated news fragment - act is a strange choice here. IMO it makes more sense to simply remove Act to from it.

Base automatically changed from feature/snmp-get2-multiple-variables to master July 8, 2024 18:02
@lunkwill42
Copy link
Member Author

But I would like to have an updated news fragment - act is a strange choice here. IMO it makes more sense to simply remove Act to from it.

As release manager I'm still responsible for editing the final changelog, so I'll just keep that in mind when the time comes :) Thank you!

@lunkwill42 lunkwill42 merged commit 223ba0f into master Jul 8, 2024
10 of 11 checks passed
@lunkwill42 lunkwill42 deleted the feature/bfd-traps branch July 8, 2024 18:04
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.

Implement handling of BFD traps (BFD-STD-MIB::bfdSessUp and BFD-STD-MIB::bfdSessDown)
2 participants