-
Notifications
You must be signed in to change notification settings - Fork 236
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_cli_config_gen): Add support for segment-routing traffic-engineering #1486
Feat(eos_cli_config_gen): Add support for segment-routing traffic-engineering #1486
Conversation
6837f86
to
6b7ea94
Compare
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.
Hello and thanks for this great contribution, it looks very solid! I had a few notes on the data model and about aligning to our templating conventions.
In summary: We are trying to migrate away from variable dict keys such as 1.2.3.4 and 70810 in your example and make list of dicts instead which will be easier to validate with a schema down the line. Also use of our arista.avd.default filter is encouraged where appropriate.
...ns/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/router-traffic-engineering.yml
Outdated
Show resolved
Hide resolved
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
13521b7
to
85efa38
Compare
85efa38
to
aba418b
Compare
Thanks @emilarista ! Fixed with suggestions via aba418b |
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!
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!
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
Missing documentation in the eos_cli_config_gen README.md |
...ns/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/router-traffic-engineering.yml
Outdated
Show resolved
Hide resolved
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
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.
Please adjust the blank lines in the documentation template according to the comments from @tgodaA
c564741
to
c87ae36
Compare
c87ae36
to
7a748d7
Compare
Thanks for taking the time to review this! The necessary changes have been made. |
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
..._collections/arista/avd/roles/eos_cli_config_gen/templates/eos/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
..._collections/arista/avd/roles/eos_cli_config_gen/templates/eos/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/README.md
Outdated
Show resolved
Hide resolved
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
..._collections/arista/avd/roles/eos_cli_config_gen/templates/eos/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Show resolved
Hide resolved
..._collections/arista/avd/roles/eos_cli_config_gen/templates/eos/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
...ns/arista/avd/roles/eos_cli_config_gen/templates/documentation/router-traffic-engineering.j2
Outdated
Show resolved
Hide resolved
...ns/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/router-traffic-engineering.yml
Outdated
Show resolved
Hide resolved
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
Feat(eos_cli_config_gen): Add support for segment-routing traffic-engineering
Change Summary
Add support for configuring SRTE policy under the router traffic-engineering configuration sub-section. This allows addition of multiple SRTE policies
Related Issue(s)
Fixes #1480
Component(s) name
arista.avd.eos_cli_config_gen
Proposed changes
How to test
molecule test --scenario-name eos_cli_config_gen
Add SRTE policy under structured config to generate cfg and markdown for the relevant config section
Checklist
User Checklist
Repository Checklist