-
Notifications
You must be signed in to change notification settings - Fork 240
Refactor(eos_designs): Improve /metadata/cv_pathfinder.py and improve pytest coverage #5310
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
base: devel
Are you sure you want to change the base?
Refactor(eos_designs): Improve /metadata/cv_pathfinder.py and improve pytest coverage #5310
Conversation
Review docs on Read the Docs To test this pull request: # Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-5310
# Activate the virtual environment
source test-avd-pr-5310/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/MaheshGSLAB/ansible-avd.git@cv-pathfinder-coverage#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/MaheshGSLAB/ansible-avd.git#/ansible_collections/arista/avd/,cv-pathfinder-coverage --force
# Optional: Install AVD examples
cd test-avd-pr-5310
ansible-playbook arista.avd.install_examples |
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. |
@@ -97,34 +97,27 @@ def _metadata_vrfs(self: AvdStructuredConfigMetadataProtocol) -> None: | |||
"""Set the metadata for VRFs by parsing the generated structured config and flatten it a bit (like hiding load-balance policies).""" | |||
avt_vrfs = self.structured_config.router_adaptive_virtual_topology.vrfs | |||
load_balance_policies = self.structured_config.router_path_selection.load_balance_policies | |||
if not avt_vrfs or not load_balance_policies: |
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.
To get an empty avt vrfs and empty load balance policies we need to set the wan_mode as autovpn but this function _metadata_vrfs trigger when the wan_mode is cvpathfinder.
avt_policies = self.structured_config.router_adaptive_virtual_topology.policies | ||
|
||
if self.shared_utils.is_wan_server: |
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.
The method _metadata_vrfs call when self.shared_utils.is_cv_pathfinder_server and under that we are already checking for self.shared_utils.is_wan_server. So there is no use of this condition.
|
||
for vrf in avt_vrfs: | ||
if not vrf.policy: |
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.
We always adding the policy in avt vrfs. But type hint is not happy. Need to check
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.
# pragma: no-cover
from typing import cast
[...]
vrf_policy = cast(str, vrf.policy)
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, now type hint is happy
if vrf_name == "default": | ||
return 1 | ||
|
||
msg = f"Unable to find the WAN VNI for VRF {vrf_name} during generation of cv_pathfinder metadata." |
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.
Unable to reproduce this error as vrf_name collected from _filtered_wan_vrfs which filter out the vrf from wan_virtual_topologies.vrfs.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Change Summary
Add pytest coverage for /metadata/cv_pathfinder.py
Related Issue(s)
Fixes #
Component(s) name
arista.avd.eos_designs
Proposed changes
Add pytest coverage for /metadata/cv_pathfinder.py. Coverage is 100%
How to test
Run tox
Checklist
User Checklist
Repository Checklist