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 link state monitor task #71

Merged
merged 19 commits into from
Oct 12, 2023

Conversation

lunkwill42
Copy link
Member

@lunkwill42 lunkwill42 commented Aug 23, 2023

This implements the basics of a link state monitor task, along with some of the necessary changes to the surrounding codebase.

As a side-effect, this also introduces basic test coverage for the zino.tasks module, which had none.

Closes #75

@lunkwill42 lunkwill42 self-assigned this Aug 23, 2023
@github-actions
Copy link

github-actions bot commented Aug 23, 2023

Test results

    3 files      3 suites   45s ⏱️
113 tests 113 ✔️ 0 💤 0
339 runs  337 ✔️ 2 💤 0

Results for commit 0a488a6.

♻️ This comment has been updated with latest results.

@lunkwill42 lunkwill42 force-pushed the feature/linkstatetask branch from c8d6c2f to df35715 Compare August 24, 2023 11:48
@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #71 (0a488a6) into master (0ae693c) will increase coverage by 0.82%.
Report is 22 commits behind head on master.
The diff coverage is 97.60%.

@@            Coverage Diff             @@
##           master      #71      +/-   ##
==========================================
+ Coverage   94.06%   94.88%   +0.82%     
==========================================
  Files          22       24       +2     
  Lines        1144     1250     +106     
==========================================
+ Hits         1076     1186     +110     
+ Misses         68       64       -4     
Files Coverage Δ
src/zino/snmp.py 96.37% <100.00%> (+1.93%) ⬆️
src/zino/statemodels.py 100.00% <100.00%> (ø)
src/zino/tasks/__init__.py 70.00% <100.00%> (+47.78%) ⬆️
src/zino/compat.py 90.00% <90.00%> (ø)
src/zino/tasks/linkstatetask.py 97.94% <97.94%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lunkwill42 lunkwill42 force-pushed the feature/linkstatetask branch 2 times, most recently from c1f58ee to 9f5c354 Compare August 25, 2023 12:48
@lunkwill42 lunkwill42 force-pushed the feature/linkstatetask branch from 3934a0c to d4c81c8 Compare September 11, 2023 12:23
We shouldn't need to create the dictionary elsewhere, so its best to
default to starting with an empty dict that we can start using
immediately.
@lunkwill42 lunkwill42 force-pushed the feature/linkstatetask branch from d4c81c8 to 153afdc Compare September 11, 2023 13:32
We do not actually ever need the numerical values from the MIB in our
state handling code, as the values are translated to strings for us by
the MIB layer.  It is therefore better to use a StrEnum, which makes
translation from MIB string values easier, and makes the saved state
even more human-readable.

Also, at least one of the states that Zino 1 operates with, "adminDown"
isn't actually a state value from the MIB.  It is used to flag the case
where an interface operational state is down not because of an error or
missing link, but because the administrator has disabled the port on
purpose.

Because of this, the InterfaceOperState enum was also renamed to
InterfaceState (since it doesn't necessarily represent just operstate
values).
This makes it easier to annotate other methods that will return or use a
data structure that came from the sparsewalk method.
This is an initial re-implementation of the link state monitor parts of
Zino 1.  Some bits and pieces are still missing, and some TODO points a
strewn throughout.
According to Håvard Eidnes, we may need to keep the alias value for
non-watched interfaces, especially for PM functionality that comes
later.

That an interface is not watched only means we shouldn't generate events
for it.
These are used by the original Zino for this type of event.
This also factors out state updating and event generation to separate
methods.
Copy link
Contributor

@stveit stveit left a comment

Choose a reason for hiding this comment

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

Could increse coverage further by adding some more tests as codecov points out.

Would be nice with a couple small tests specifically for BaseInterfaceRow.is_sane

src/zino/tasks/linkstatetask.py Outdated Show resolved Hide resolved
src/zino/tasks/linkstatetask.py Show resolved Hide resolved
src/zino/tasks/linkstatetask.py Show resolved Hide resolved
As noted in code review, it's a good idea to log some details
whenever we skip updating an interface due to receiving faulty or
incomplete SNMP data.
This changes the code flow based on comments from @stveit: Processing a
single interface where the collected data is incomplete will raise an
exception, while when processing multiple interfaces, single interfaces
with issues are just logged and otherwise ignored to not interrupt the
flow.
@sonarcloud
Copy link

sonarcloud bot commented Oct 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@lunkwill42 lunkwill42 merged commit f54d20e into Uninett:master Oct 12, 2023
9 checks passed
@lunkwill42 lunkwill42 deleted the feature/linkstatetask branch October 12, 2023 13:24
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 basic linkstate monitor task
2 participants