-
Notifications
You must be signed in to change notification settings - Fork 228
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(eos_designs): Add platform match criteria for network_ports #4798
base: devel
Are you sure you want to change the base?
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-4798
# Activate the virtual environment
source test-avd-pr-4798/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/kpbush30/avd.git@network_ports_4786#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/kpbush30/avd.git#/ansible_collections/arista/avd/,network_ports_4786 --force
# Optional: Install AVD examples
cd test-avd-pr-4798
ansible-playbook arista.avd.install_examples |
for more information, see https://pre-commit.ci
...ions/arista/avd/molecule/eos_designs_unit_tests/inventory/group_vars/NETWORK_PORTS_TESTS.yml
Show resolved
Hide resolved
python-avd/pyavd/_eos_designs/schema/schema_fragments/network_ports.schema.yml
Outdated
Show resolved
Hide resolved
...ions/arista/avd/molecule/eos_designs_unit_tests/inventory/group_vars/NETWORK_PORTS_TESTS.yml
Outdated
Show resolved
Hide resolved
@@ -20,6 +20,18 @@ keys: | |||
The regular expression must match the full hostname. | |||
items: | |||
type: str | |||
platforms: |
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.
let's add a description on the network_ports
stating that when switches
and platforms
are both set, then only devices matching both criteria will be configured.
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.
Added clarification in schema docs and in markdown above clarifying match criteria expectations.
@@ -72,7 +72,9 @@ def _filtered_network_ports(self: AvdStructuredConfigConnectedEndpoints) -> list | |||
for index, network_port in enumerate(self.inputs.network_ports): | |||
network_port._context = f"network_ports[{index}]" | |||
network_port_settings = self.shared_utils.get_merged_adapter_settings(network_port) | |||
if not self._match_regexes(network_port_settings.switches, self.shared_utils.hostname): | |||
if network_port_settings.switches and not self._match_regexes(network_port_settings.switches, self.shared_utils.hostname): |
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.
this change is changing the behavior. In the current implementation, if switches
is not set, then no network_port are configured on any device. The change, if neither switches
, nor platforms
are set would configure the network_ports on ALL devices.
you would need an extra if
to make sure that either switches
or platforms
are set + add a test case with network_ports without switches
not platforms
set to validate the behavior
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.
Fixed behavior to ensure match criteria is required, added test to ensure configuration is not generated if missing both switch hostname and platform match criteria.
...ions/arista/avd/molecule/eos_designs_unit_tests/inventory/group_vars/NETWORK_PORTS_TESTS.yml
Outdated
Show resolved
Hide resolved
python-avd/pyavd/_eos_designs/schema/schema_fragments/network_ports.schema.yml
Show resolved
Hide resolved
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Change Summary
Add platform match criteria for network_ports
Related Issue(s)
Fixes #4786
Component(s) name
arista.avd.eos_designs
Proposed changes
Add platform key to match to filter devices for
network_ports
How to test
Run molecule scenario for
eos_designs_uni_tests
forNETWORK_PORTS_TESTS
molecule converge -s eos_designs_unit_tests -- --limit NETWORK_PORTS_TESTS
Checklist
Repository Checklist