-
Notifications
You must be signed in to change notification settings - Fork 30
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
fix(anta.tests): Updated testcase to support more address family #566
Conversation
anta/tests/routing/bgp.py
Outdated
@@ -290,7 +290,7 @@ def validate_inputs(self: BaseModel) -> BaseModel: | |||
""" | |||
Validate the inputs provided to the BgpAfi class. | |||
|
|||
If afi is either ipv4 or ipv6, safi must be provided. | |||
If afi is ipv4, ipv6 or sr-te, safi must be provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missed to update, will update back in next commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please update this. Also, can we add a note somewhere maybe in the docstring of the test class explaining the different syntax for sr-te?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
anta/tests/routing/bgp.py
Outdated
|
||
# Swaping AFI and SAFI in case of SR-TE address family | ||
if afi == "sr-te": | ||
afi, safi = safi, afi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should put that in the render
function no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes we are doing this in render function too in the line (https://github.com/arista-netdevops-community/anta/pull/566/files#:~:text=%23%20For%20SR%2DTE,.vrf)))
Reason of here again to change is to get correct afi and safi in failure. If we dont swap then for SR-TE in failure we get afi as sr-te but expected is ipv4 and safi as sr-te.
Failures: [{'afi': 'sr-te', 'safi': 'ipv4', 'vrfs': {'default': {'10.255.0.2': {'peerState': 'Connect', 'inMsgQueue': 0, 'outMsgQueue': 0}}}}]
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Conflicts have been resolved. A maintainer will review the pull request shortly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
@@ -218,8 +220,12 @@ def render(self, template: AntaTemplate) -> list[AntaCommand]: | |||
"""Render the template for each BGP address family in the input list.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this is a docstring that deserves more info given the "dark magic" in the rendering function
@@ -286,7 +298,7 @@ class Input(AntaTest.Input): | |||
"""List of BGP address families (BgpAfi).""" | |||
|
|||
class BgpAfi(BaseModel): | |||
"""Model for a BGP address family (AFI) and subsequent service family (SAFI).""" | |||
"""Model for a BGP address family (AFI) and subsequent address family (SAFI).""" | |||
|
|||
afi: Afi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we work to avoid the duplication of BgpAfi in every test?
Description
Updated VerifyBGPPeerCount, VerifyBGPPeersHealth and VerifyBGPSpecificPeers test to support more address families
Fixes #565
Checklist:
pre-commit run
)tox -e testenv
)