-
Notifications
You must be signed in to change notification settings - Fork 212
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 monitor server radius
#4595
base: devel
Are you sure you want to change the base?
Feat(eos_cli_config_gen): Add support for monitor server radius
#4595
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-4595
# Activate the virtual environment
source test-avd-pr-4595/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/laxmikantchintakindi/avd.git@feat/monitor-server-radius#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/laxmikantchintakindi/avd.git#/ansible_collections/arista/avd/,feat/monitor-server-radius --force
# Optional: Install AVD examples
cd test-avd-pr-4595
ansible-playbook arista.avd.install_examples |
e15c46b
to
d00db5d
Compare
python-avd/pyavd/_eos_cli_config_gen/schema/schema_fragments/monitor_server_radius.schema.yml
Outdated
Show resolved
Hide resolved
python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/monitor-server-radius.j2
Show resolved
Hide resolved
7c3f23b
to
64b925e
Compare
152b0a8
to
6ef46b1
Compare
python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/monitor-server-radius.j2
Outdated
Show resolved
Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/docs/tables/monitor-server-radius.md
Outdated
Show resolved
Hide resolved
python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/monitor-server-radius.j2
Outdated
Show resolved
Hide resolved
python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/monitor-server-radius.j2
Outdated
Show resolved
Hide resolved
| Threshold failure | {{ monitor_server_radius.probe.threshold_failure }} | | ||
{% endif %} | ||
{% if monitor_server_radius.probe.method is arista.avd.defined %} | ||
| Probe method | {{ monitor_server_radius.probe.method }} | |
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.
Would it make sense to expose username (not sure about password) in case we use method: access-request
?
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.
Is this file needed? There is no inventory host with such name
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.
nope it should be removed
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.
Removed
service dot1x | ||
probe interval 100 seconds | ||
probe threshold failure 100 | ||
probe method access-request username arista password 7 141600021F102B |
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 make sure this is removed from documentation by default using hide_password
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.
Done
{% if monitor_server_radius.probe.access_request.username is arista.avd.defined and | ||
monitor_server_radius.probe.access_request.password is arista.avd.defined %} | ||
{% set access_request = monitor_server_radius.probe.access_request %} | ||
probe method access-request username {{ access_request.username }} password {{ access_request.password_type | arista.avd.default("7") }} {{ access_request.password }} |
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.
probe method access-request username {{ access_request.username }} password {{ access_request.password_type | arista.avd.default("7") }} {{ access_request.password }} | |
probe method access-request username {{ access_request.username }} password {{ access_request.password_type | arista.avd.default("7") }} {{ access_request.password | arista.avd.hide_passwords(hide_passwords }} |
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.
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.
nope it should be removed
required: true | ||
password: | ||
type: str | ||
description: Type 7 password. |
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.
It is not a type-7 password if you select something else in password_type. Please look in other data models for inspiration for a better description.
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.
Removed the description.
keys: | ||
interval: | ||
type: int | ||
description: Server probe period in seconds. |
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.
description: Server probe period in seconds. | |
description: Server probe interval in seconds. |
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.
Done.
@@ -647,6 +647,12 @@ roles/eos_cli_config_gen/docs/tables/management-api-gnmi.md | |||
roles/eos_cli_config_gen/docs/tables/monitor-connectivity.md | |||
--8<-- | |||
|
|||
### Monitor Server Radius |
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.
### Monitor Server Radius | |
### Monitor server Radius |
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.
Done.
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 refactor the tests to add to host-1 and host-2 instead.
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.
Done.
for more information, see https://pre-commit.ci
092f828
to
893c34a
Compare
Quality Gate passedIssues Measures |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Change Summary
Add support for
monitor server radius
in eos_cli_config_gen.Related Issue(s)
Fixes #4273
Component(s) name
arista.avd.eos_cli_config_gen
Proposed changes
Add support for
monitor server radius
in eos_cli_config_gen.How to test
Checklist
User Checklist
Repository Checklist