-
Notifications
You must be signed in to change notification settings - Fork 202
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
Refactor(eos_cli_config_gen): Add support for ingress in system.control_plane.ipv4/6_access_group #4481
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-4481
# Activate the virtual environment
source test-avd-pr-4481/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/Vibhu-gslab/avd.git@ingress#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/Vibhu-gslab/avd.git#/ansible_collections/arista/avd/,ingress --force
# Optional: Install AVD examples
cd test-avd-pr-4481
ansible-playbook arista.avd.install_examples |
c3a5661
to
b0e17eb
Compare
@@ -36,6 +36,10 @@ keys: | |||
type: str | |||
convert_types: | |||
- int | |||
description: "'vrf' and 'ingress_default' are mutual exclusive." | |||
ingress_default: |
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 have two option under ingress i.e. default
and enabled
.
Because we can have vrf with ingress but without default.
system control-plane
ip access-group ingress default acl4_4
ip access-group acl4_3 in
ip access-group ingress vrf mahesh in <-----------
ip access-group acl4_2 vrf red in
ip access-group acl4_2 vrf red_1 in
ip access-group acl4_5 vrf red_3 in
ip access-group ingress vrf red_4 in
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.
or you can add the description for acl_name to use ingress as acl since it is required key
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 description.
@@ -20,20 +20,28 @@ system control-plane | |||
{% endif %} | |||
{# control_plane access_groups ipv4 #} | |||
{% for acl_set in system.control_plane.ipv4_access_groups | arista.avd.natural_sort %} |
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.
ipv4_access_groups is list of dicts so we should have the key name base on we need to sort but again there is no required key so we will have issue. Can you please check then do we need to remove sorting here?
b0e17eb
to
ae22994
Compare
…l_plane.ipv4/6_access_group
ae22994
to
b6f6e0f
Compare
Quality Gate passedIssues Measures |
Change Summary
Add support for ingress in system.control_plane.ipv4/6_access_group
Related Issue(s)
Fixes #4467
Component(s) name
arista.avd.eos_cli_config_gen
How to test
CI
Checklist
Repository Checklist