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

feat(anta): Added the test case to Verify SNMP Notification Host #838

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

vitthalmagadum
Copy link
Contributor

Description

Verifies the SNMP notification host (SNMP manager) configurations.

Fixes #822

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have run pre-commit for code linting and typing (pre-commit run)
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (tox -e testenv)

@vitthalmagadum vitthalmagadum marked this pull request as draft September 26, 2024 11:37
@vitthalmagadum vitthalmagadum marked this pull request as ready for review September 27, 2024 06:21

Expected Results
----------------
* Success: The test will pass if the SNMP PDU counter(s) are non-zero/greater than zero.
Copy link
Contributor

Choose a reason for hiding this comment

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

please correct the success and failure message



class VerifySNMPNotificationHost(AntaTest):
"""Verifies the SNMP notification host (SNMP manager) configurations.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have more brief description about what configuration we are verifying

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated docstring.

"""Model for a SNMP Manager."""

hostname: IPv4Address
"""IP address of the SNMP notification host."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""IP address of the SNMP notification host."""
"""IPv4 address of the SNMP notification host."""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.


# Verify SNMP host details.
if not (host_details := get_item(self.instance_commands[0].json_output["hosts"], "hostname", hostname)):
failures += f"Details not found for SNMP host '{hostname}'.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
failures += f"Details not found for SNMP host '{hostname}'.\n"
failures += f"SNMP host '{hostname}' is not configured.\n"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

user = host.user

# Verify SNMP host details.
if not (host_details := get_item(self.instance_commands[0].json_output["hosts"], "hostname", hostname)):
Copy link
Contributor

Choose a reason for hiding this comment

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

we can get empty list in case no SNMP host configured so in that case just break with a general message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added check. Thanks!!

Copy link

codspeed-hq bot commented Oct 1, 2024

CodSpeed Performance Report

Merging #838 will not alter performance

Comparing vitthalmagadum:issue_822 (da8ba27) with main (fc644a1)

Summary

✅ 4 untouched benchmarks

"""List of SNMP hosts."""

class SNMPHost(BaseModel):
"""Model for a SNMP Manager."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Model for a SNMP Manager."""
"""Model for a SNMP Host."""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

"""Optional VRF for SNMP Hosts. If not provided, it defaults to `default`."""
notification_type: Literal["trap", "inform"]
"""Type of SNMP notification (trap or inform)."""
version: Literal["v1", "v2c", "v3"]
Copy link
Contributor

Choose a reason for hiding this comment

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

i think the snmp version you can move to custom type as we can use it for many other general snmp testcases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to custom types.


# Verify SNMP host details.
if not (snmp_hosts := get_value(self.instance_commands[0].json_output, "hosts")):
self.result.is_failure("SNMP is not configured.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.result.is_failure("SNMP is not configured.")
self.result.is_failure("No SNMP host is configured.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Thanks!!

Copy link

sonarcloud bot commented Oct 3, 2024

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.

Add the test case to Verify SNMP Notification Host
2 participants