-
Notifications
You must be signed in to change notification settings - Fork 231
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): Only enable PTP on certain uplinks #4819
base: devel
Are you sure you want to change the base?
Feat(eos_designs): Only enable PTP on certain uplinks #4819
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-4819
# Activate the virtual environment
source test-avd-pr-4819/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/laxmikantchintakindi/avd.git@feat/eos_designs/restrict-ptp-on-certain-uplinks#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/eos_designs/restrict-ptp-on-certain-uplinks --force
# Optional: Install AVD examples
cd test-avd-pr-4819
ansible-playbook arista.avd.install_examples |
76735f7
to
0d4d76d
Compare
e05a8b9
to
1eea385
Compare
7c16b15
to
40d27e5
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
8710e69
to
4827d07
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
8edf396
to
8d69da1
Compare
8d69da1
to
2051b31
Compare
ptp: | ||
enabled: true |
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.
Can we add a testcase where "ptp: enabled" is not defined but uplinks
is defined.
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.
ptp_settings
in PTP_TESTS.yml group vars is set to true. So even ptp:enabled
is not set, there will be no change in configs.
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 understand, but having a testcase for it would make it more clear.
You can mention in comment above testcase what it is for.
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.
Can you add more tests like uplinks under defaults and node group
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.
Addressed both comments.
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.
request changes
d7ea276
to
3633c52
Compare
for more information, see https://pre-commit.ci
Co-authored-by: Vibhu-gslab <[email protected]>
Change Summary
Only enable PTP on certain uplinks.
Related Issue(s)
Fixes #4009
Component(s) name
arista.avd.eos_designs
Proposed changes
Provide a mechanism to control which uplinks are enabled for PTP.
Could be either a list of uplink interfaces or maybe an algorithm like modulus on the node ID.
How to test
Checklist
User Checklist
Repository Checklist