feat(anta): Added test case to verify dynamic vlans source #978

geetanjalimanegslab · 2024-12-24T06:13:06Z

Description

This test performs the following checks for specified dynamic VLAN(s):

  1. Ensures that dynamic VLAN(s) are properly configured in the system.
  2. Confirms that dynamic VLAN(s) are enabled on all the designated sources.
  3. When strict mode is enabled (`strict: true`):
    - The dynamic VLAN(s) are enabled on all the designated sources and disabled for non designated sources.

Expected Results
----------------
* Success: The test will pass if all of the following conditions are met:
    - The dynamic VLAN(s) are properly configured in the system.
    - The dynamic VLAN(s) are enabled for all of the designated sources.
    - In strict mode, The dynamic VLAN(s) are enabled on all the designated sources and disabled for non designated sources.
* Failure: The test will fail if any of the following conditions is met:
    - The dynamic VLAN(s) are disabled on any of designated sources.
    - In strict mode, dynamic VLAN(s) are disabled on any of the designated sources. or enabled for non designated sources.
* Skipped: The test will skip if the following conditions is met:
    - Dynamic VLAN(s) are not configured on the device.

Fixes #864

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)

codspeed-hq · 2024-12-24T06:56:20Z

CodSpeed Performance Report

Merging #978 will not alter performance

_{Comparing geetanjalimanegslab:test_dynamic_vlans (d547c26) with main (1f033f4)}

Summary

✅ 22 untouched benchmarks

anta/tests/vlan.py

tests/units/anta_tests/test_vlan.py

anta/tests/vlan.py

carl-baillargeon · 2025-01-07T19:27:04Z

anta/tests/vlan.py

+    * Failure: The test will fail if any of the following conditions is met:
+        - The dynamic VLAN(s) are disabled on all designated sources, or active on non designated sources.
+        - In strict mode, dynamic VLAN(s) are disabled on any of the designated sources.
+    * Skipped: The test will Skip if the following conditions is met:


Suggested change

* Skipped: The test will Skip if the following conditions is met:

* Skipped: The test will skip if the following conditions is met:

carl-baillargeon · 2025-01-07T19:28:18Z

anta/tests/vlan.py

+    class Input(AntaTest.Input):
+        """Input model for the VerifyDynamicVlanSource test."""
+
+        model_config = ConfigDict(extra="forbid")


Suggested change

model_config = ConfigDict(extra="forbid")

Not needed here since it is already in the parent class AntaTest.Input.

carl-baillargeon · 2025-01-07T19:28:30Z

anta/tests/vlan.py

+        source: list[DynamicVLANSource]
+        """The dynamic VLAN(s) source list."""
+        strict: bool = False
+        """If True, requires exact match of the provided dynamic VLAN(s) sources, Defaults to `False`"""


Suggested change

"""If True, requires exact match of the provided dynamic VLAN(s) sources, Defaults to `False`"""

"""If True, requires exact match of the provided dynamic VLAN(s) sources. Defaults to `False`."""

carl-baillargeon · 2025-01-07T19:37:14Z

anta/custom_types.py

@@ -238,3 +238,4 @@ def validate_regex(value: str) -> str:
    "Route Cache Route",
    "CBF Leaked Route",
 ]
+DynamicVLANSource = Literal["dmf", "dot1x", "dynvtep", "evpn", "mlag", "mlagsync", "mvpn", "swfwd", "vccbfd"]


Suggested change

DynamicVLANSource = Literal["dmf", "dot1x", "dynvtep", "evpn", "mlag", "mlagsync", "mvpn", "swfwd", "vccbfd"]

DynamicVlanSource = Literal["dmf", "dot1x", "dynvtep", "evpn", "mlag", "mlagsync", "mvpn", "swfwd", "vccbfd"]

carl-baillargeon · 2025-01-07T19:40:27Z

anta/tests/vlan.py

+        """Input model for the VerifyDynamicVlanSource test."""
+
+        model_config = ConfigDict(extra="forbid")
+        source: list[DynamicVLANSource]


Suggested change

source: list[DynamicVLANSource]

sources: list[DynamicVLANSource]

carl-baillargeon · 2025-01-07T20:02:02Z

anta/tests/vlan.py

+        str_actual_source = ", ".join(actual_source)
+
+        # If strict flag is True and dynamic VLAN(s) are disabled on any of the designated sources, test fails.
+        if self.inputs.strict and sorted(actual_source) != (expected_source):


This could lead to false negatives if expected_source is unsorted.

carl-baillargeon · 2025-01-07T20:21:50Z

anta/tests/vlan.py

+class VerifyDynamicVlanSource(AntaTest):
+    """Verifies dynamic VLAN(s) source.
+
+    This test performs the following checks for specified dynamic VLAN(s):
+
+      1. Ensures that dynamic VLAN(s) are properly configured in the system.
+      2. Confirms that dynamic VLAN(s) are enabled for any/all the designated sources and disabled for all others.
+      3. When strict mode is enabled (`strict: true`):
+        - Dynamic VLAN(s) are enabled for all designated sources.


Can you clarify the intent of the test?

It could be difficult for users to know which services will appear in the output. Also, a service (or source) being present in the dynamic VLANs output just means it's registered as a potential consumer and having no VLANs allocated (empty vlanIds list) doesn't necessarily mean it's an issue. Example: EVPN being "activated" but showing an empty vlanIds list is perfectly normal if it doesn't need any dynamic VLANs.

If the goal is to check that the provided services have dynamic VLANs registered, the test logic could be simplified a lot by looping over the input sources list, check if the source is present, if it is check if its vlanIds list is not empty.

The actual test criteria is If there are any dynamic VLANs, then these should only have source of "evpn" or "mlagsync" ie. all other sources must be "NONE".
We are taking the sources as input and providing an option for exact match. On the other hand, if strict is set to False(default option), the test will pass as long as the dynamic VLANs are assigned to any of the input sources.

Ok in that case we need to update the docstring and logic to reflect this because, in my opinion, a service with an empty vlanIds list doesn't mean it's not configured/enabled. It just means the service doesn't use any dynamic VLANs.

The test should pass if all provided source(s) have dynamic VLANs registered (vlanIds not empty), if not, we just say there are no dynamic VLANs for the source(s) concerned. If strict is set, the test would fail if other sources (not provided by the user) have dynamic VLANs registered.

Logic:

Strict: False

If all provided source(s) have dynamic VLANs registered, Test should pass

If not then passing the test as there are no dynamic VLANs for the source(s) concerned.

If dynamic vlans are assigned to sources other than provided, Test should fail.

Strict: True

If all provided source(s) have dynamic VLANs registered, Test should pass

If not, Then test should fail saying dynamic vlans are not assigned to given sources.

If dynamic vlans are assigned to sources other than provided, Test should fail.

Apologies if I was not super clear. Here's what I have in mind:

Strict: False

If all provided source(s) have dynamic VLANs registered, test should pass.

If not, then test should fail saying dynamic VLANs are not assigned to given sources.

If dynamic VLANs are assigned to sources other than provided, test should pass (we don't care about other sources not provided by the user, we just explicity check the provided ones).

Strict: True

If all provided source(s) have dynamic VLANs registered, test should pass.

If not, then test should fail saying dynamic VLANs are not assigned to given sources.

If dynamic VLANs are assigned to sources other than provided, test should fail (we do care about other sources now, we are not expecting other dynamic VLANs on other sources).

Thank you @carl-baillargeon for clarifying the additional context and explanation, makes things much clearer.

sonarqubecloud · 2025-01-08T12:33:52Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions bot assigned geetanjalimanegslab Dec 24, 2024

geetanjalimanegslab requested a review from vitthalmagadum December 24, 2024 06:13

geetanjalimanegslab changed the title ~~Issue_864:Added test case to verify dynamic vlans source~~ feat(anta)::Added test case to verify dynamic vlans source Dec 24, 2024

geetanjalimanegslab changed the title ~~feat(anta)::Added test case to verify dynamic vlans source~~ feat(anta):Added test case to verify dynamic vlans source Dec 24, 2024

geetanjalimanegslab changed the title ~~feat(anta):Added test case to verify dynamic vlans source~~ feat(anta): Added test case to verify dynamic vlans source Dec 24, 2024

vitthalmagadum requested changes Dec 24, 2024

View reviewed changes

geetanjalimanegslab force-pushed the test_dynamic_vlans branch from 29babe0 to 6651f5a Compare December 27, 2024 07:20

geetanjalimanegslab requested a review from vitthalmagadum December 27, 2024 07:21

vitthalmagadum requested changes Dec 30, 2024

View reviewed changes

anta/tests/vlan.py Outdated Show resolved Hide resolved

anta/tests/vlan.py Outdated Show resolved Hide resolved

anta/tests/vlan.py Outdated Show resolved Hide resolved

anta/tests/vlan.py Outdated Show resolved Hide resolved

geetanjalimanegslab requested a review from vitthalmagadum December 31, 2024 09:11

Geetanjali Mane added 7 commits January 3, 2025 23:01

Issue_864:Added test case to verify dynamic vlans source

cdaeda9

Issue_864: Remove duplicate unit testcases

ae88e8d

Issue_864:Added check for dynamic vlan source exact match and updated UT

0db6f7a

Issue_864: Formatted test description for mkdocs

ac52aa1

Issue_864: Updated docstrings and variable name

4871b79

Issue_864: Updated Test failure messages and TC description

7e24116

Issue_864: Updated test.yaml file

66ce5b0

geetanjalimanegslab force-pushed the test_dynamic_vlans branch from f4b60ad to 66ce5b0 Compare January 3, 2025 17:33

Issue_864: Removed duplucate testcases

8174dc9

vitthalmagadum approved these changes Jan 7, 2025

View reviewed changes

vitthalmagadum marked this pull request as ready for review January 7, 2025 09:48

vitthalmagadum requested review from gmuloc and carl-baillargeon January 7, 2025 09:48

carl-baillargeon requested changes Jan 7, 2025

View reviewed changes

geetanjalimanegslab added 2 commits January 8, 2025 08:49

Issue_864: Updated docstring and the testcase logic

ece4d84

Issue_864: Updated test failure message

d547c26

geetanjalimanegslab requested a review from carl-baillargeon January 8, 2025 12:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(anta): Added test case to verify dynamic vlans source #978

feat(anta): Added test case to verify dynamic vlans source #978

geetanjalimanegslab commented Dec 24, 2024 •

edited

Loading

codspeed-hq bot commented Dec 24, 2024 •

edited

Loading

carl-baillargeon Jan 7, 2025

carl-baillargeon Jan 7, 2025

carl-baillargeon Jan 7, 2025

carl-baillargeon Jan 7, 2025

carl-baillargeon Jan 7, 2025

carl-baillargeon Jan 7, 2025

carl-baillargeon Jan 7, 2025

vitthalmagadum Jan 8, 2025

carl-baillargeon Jan 8, 2025

vitthalmagadum Jan 8, 2025

carl-baillargeon Jan 8, 2025

vitthalmagadum Jan 8, 2025 •

edited

Loading

sonarqubecloud bot commented Jan 8, 2025

	* Skipped: The test will Skip if the following conditions is met:
	* Skipped: The test will skip if the following conditions is met:

	"""If True, requires exact match of the provided dynamic VLAN(s) sources, Defaults to `False`"""
	"""If True, requires exact match of the provided dynamic VLAN(s) sources. Defaults to `False`."""

	DynamicVLANSource = Literal["dmf", "dot1x", "dynvtep", "evpn", "mlag", "mlagsync", "mvpn", "swfwd", "vccbfd"]
	DynamicVlanSource = Literal["dmf", "dot1x", "dynvtep", "evpn", "mlag", "mlagsync", "mvpn", "swfwd", "vccbfd"]

	source: list[DynamicVLANSource]
	sources: list[DynamicVLANSource]

feat(anta): Added test case to verify dynamic vlans source #978

Are you sure you want to change the base?

feat(anta): Added test case to verify dynamic vlans source #978

Conversation

geetanjalimanegslab commented Dec 24, 2024 • edited Loading

Description

Checklist:

codspeed-hq bot commented Dec 24, 2024 • edited Loading

Merging #978 will not alter performance

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitthalmagadum Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 8, 2025

Quality Gate passed

geetanjalimanegslab commented Dec 24, 2024 •

edited

Loading

codspeed-hq bot commented Dec 24, 2024 •

edited

Loading

vitthalmagadum Jan 8, 2025 •

edited

Loading