Feat(eos_designs): Improve `ptp` settings for `p2p_links` #4612

laxmikantchintakindi · 2024-10-18T07:24:52Z

Change Summary

Improve ptp settings for p2p_links

Related Issue(s)

Fixes #2698

Component(s) name

arista.avd.eos_designs

Proposed changes

PTP knobs are available in connected_endpoints:

    ptp:
      enable: < true | false | default -> false | Required >
      # These are the default settings:
      # - The global PTP profile parameters will be applied to all connected endpoints where ptp is manually enabled.
      # - `ptp role master` is set to ensure control over the PTP topology.
      endpoint_role: < follower | bmca | default -> follower >
      profile: < aes67 | smpte2059-2 | aes67-r16-2016 | default -> aes67-r16-2016 >

These would be valuable to have also in l3_edge p2p_links for L3 devices.

How to test

Checklist

User Checklist

N/A

Repository Checklist

My code has been rebased from devel before I start
I have read the CONTRIBUTING document.
My change requires a change to the documentation and documentation have been updated accordingly.
I have updated molecule CI testing accordingly. (check the box if not applicable)

github-actions · 2024-10-18T07:25:06Z

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-4612
# Activate the virtual environment
source test-avd-pr-4612/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/laxmikantchintakindi/avd.git@feat/ptp-p2p-links#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/ptp-p2p-links --force
# Optional: Install AVD examples
cd test-avd-pr-4612
ansible-playbook arista.avd.install_examples

python-avd/pyavd/_eos_designs/structured_config/core_interfaces_and_l3_edge/utils.py

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_p2p_links.schema.yml

...-avd/pyavd/_eos_designs/structured_config/core_interfaces_and_l3_edge/ethernet_interfaces.py

MaheshGSLAB · 2024-10-24T09:42:24Z

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_p2p_links.schema.yml

+              type: str
+              default: "bmca"
+              valid_values:
+                - "bmca"


can you confirm the role bmca. I see dynamic and master in eos

I added this as per comment from the issue.

cc: @ClausHolbechArista

use can add dynamic and master here. But then also please update the valid values for the endpoints to also accept dynamic (without removing bmca, but add deprecated in the description for bmca). Dynamic == bmca

Added new valid value dynamic updated the change in description.

...llections/arista/avd/molecule/eos_designs_unit_tests/intended/configs/custom-ptp-profile.cfg

MaheshGSLAB · 2024-10-24T09:47:44Z

python-avd/pyavd/_eos_designs/structured_config/core_interfaces_and_l3_edge/utils.py

        # Apply PTP profile config
        if self.shared_utils.ptp_enabled:
            ptp_config.update(self.shared_utils.ptp_profile)

        ptp_config["enable"] = True
+
+        if get(p2p_link, "role") != "bmca":


it means default role is master which is not correct

I saw similar logic in connected_endpoints.utils.

We should have different assumptions for endpoints and p2p links.

This logic behaves different than the schema says. The schema says the default is "bmca", but your logic requires "bmca" to be set directly.

Changed bmca to dynamic and corrected the default output.

carlbuchmann

Please add test coverage in eos_designs_unit_tests scenario

laxmikantchintakindi · 2024-10-30T06:53:28Z

Please add test coverage in eos_designs_unit_tests scenario

Added.

github-actions · 2024-11-12T09:52:44Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2024-11-15T06:17:47Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

ClausHolbechArista · 2024-11-18T07:00:50Z

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_adapter_config.schema.yml

@@ -135,11 +135,13 @@ $defs:
            default: false
          endpoint_role:
            type: str
+            description: Set `dynamic` instead of `bmca`(deprecated).


Suggested change

description: Set `dynamic` instead of `bmca`(deprecated).

description: |-

PTP role of the endpoint.

`follower` will configure the switch port as `ptp role master`.

`dynamic` will use BMCA.

`default` is deprecated in favor of `follower`.

`bmca` is deprecated in favor of `dynamic`.

Updated the description.

ClausHolbechArista · 2024-11-18T07:01:50Z

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_adapter_config.schema.yml

              - "bmca"
              - "default"
              - "follower"
+              - "dynamic"


Let's reorder so default is first and deprecated is last.

Suggested change

- "bmca"

- "default"

- "follower"

- "dynamic"

- "follower"

- "dynamic"

- "bmca"

- "default"

python-avd/pyavd/_eos_designs/structured_config/connected_endpoints/utils.py

ClausHolbechArista · 2024-11-18T07:09:35Z

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_p2p_links.schema.yml

@@ -132,6 +132,20 @@ $defs:
              type: bool
              default: false
              description: Enable PTP.
+            role:
+              type: str
+              description: EOS default is `dynamic`.


Since this is applied on a link with two ends, it would not make sense to use "master" on both ends. We need to change this to a list similar to the ip_addresses etc. and then apply it on each device based on the index.

for more information, see https://pre-commit.ci

…iles

…oints/utils.py Co-authored-by: Claus Holbech <[email protected]>

for more information, see https://pre-commit.ci

sonarcloud · 2024-11-18T10:04:34Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions bot assigned laxmikantchintakindi Oct 18, 2024

github-actions bot added state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated role: eos_designs issue related to eos_designs role labels Oct 18, 2024

laxmikantchintakindi changed the title ~~Feat(eos_desings): Improve ptp settings for p2p_links~~ Feat(eos_designs): Improve ptp settings for p2p_links Oct 18, 2024

carlbuchmann requested a review from sugetha24 October 18, 2024 12:33

ClausHolbechArista reviewed Oct 18, 2024

View reviewed changes

python-avd/pyavd/_eos_designs/structured_config/core_interfaces_and_l3_edge/utils.py Outdated Show resolved Hide resolved

laxmikantchintakindi force-pushed the feat/ptp-p2p-links branch 3 times, most recently from 5504ee0 to 3477fa5 Compare October 24, 2024 06:13

MaheshGSLAB reviewed Oct 24, 2024

View reviewed changes

laxmikantchintakindi marked this pull request as ready for review October 28, 2024 10:13

laxmikantchintakindi requested review from a team as code owners October 28, 2024 10:13

carlbuchmann requested changes Oct 28, 2024

View reviewed changes

laxmikantchintakindi marked this pull request as draft October 29, 2024 12:14

laxmikantchintakindi force-pushed the feat/ptp-p2p-links branch from cf1360c to 6b67c93 Compare October 30, 2024 06:29

laxmikantchintakindi marked this pull request as ready for review October 30, 2024 06:36

laxmikantchintakindi force-pushed the feat/ptp-p2p-links branch from 6b67c93 to fee17c7 Compare November 6, 2024 07:02

laxmikantchintakindi requested a review from ClausHolbechArista November 6, 2024 09:12

laxmikantchintakindi marked this pull request as draft November 7, 2024 16:24

github-actions bot added the state: conflict PR with conflict label Nov 12, 2024

laxmikantchintakindi force-pushed the feat/ptp-p2p-links branch from fee17c7 to de3c1d7 Compare November 15, 2024 06:16

github-actions bot removed the state: conflict PR with conflict label Nov 15, 2024

laxmikantchintakindi force-pushed the feat/ptp-p2p-links branch 2 times, most recently from ab63faf to 50527f8 Compare November 15, 2024 07:18

laxmikantchintakindi marked this pull request as ready for review November 15, 2024 07:18

laxmikantchintakindi force-pushed the feat/ptp-p2p-links branch from 282d4fd to 6e97b70 Compare November 18, 2024 05:34

ClausHolbechArista requested changes Nov 18, 2024

View reviewed changes

laxmikantchintakindi marked this pull request as draft November 18, 2024 07:13

Laxmikant Chintakindi and others added 16 commits November 18, 2024 15:30

Feat(eos_desings): Improve settings for

489a83a

[pre-commit.ci] auto fixes from pre-commit.com hooks

b8be494

for more information, see https://pre-commit.ci

Run molecule

aa5d3a2

Handle ptp role

d73b0ab

Update error msg and add negative test

3196633

Remove context from ethernet_interfaces.py

5032327

Adding profile in ptp_profiles and fix the bugs observed

4480972

[pre-commit.ci] auto fixes from pre-commit.com hooks

2b5d4a2

for more information, see https://pre-commit.ci

Adding tests for ptp settings through p2p_links_profiles and ptp_prof…

6335f91

…iles

Fix

0a89a07

Fix CI.

9f334b0

Correct code for role settings

6dab4cc

Replace bmca to dynamic in connected endpoints role

a7ddc48

Remove ptp role dynamic from output

e98b06f

Update python-avd/pyavd/_eos_designs/structured_config/connected_endp…

dcbc12e

…oints/utils.py Co-authored-by: Claus Holbech <[email protected]>

Make role a list

b508d37

laxmikantchintakindi force-pushed the feat/ptp-p2p-links branch from 1a29ea4 to b508d37 Compare November 18, 2024 10:00

laxmikantchintakindi marked this pull request as ready for review November 18, 2024 10:02

[pre-commit.ci] auto fixes from pre-commit.com hooks

027491d

for more information, see https://pre-commit.ci

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(eos_designs): Improve `ptp` settings for `p2p_links` #4612

Feat(eos_designs): Improve `ptp` settings for `p2p_links` #4612

laxmikantchintakindi commented Oct 18, 2024 •

edited

Loading

github-actions bot commented Oct 18, 2024

MaheshGSLAB Oct 24, 2024

laxmikantchintakindi Oct 24, 2024

MaheshGSLAB Oct 25, 2024

ClausHolbechArista Nov 6, 2024

laxmikantchintakindi Nov 15, 2024

MaheshGSLAB Oct 24, 2024

laxmikantchintakindi Oct 24, 2024

ClausHolbechArista Nov 6, 2024

ClausHolbechArista Nov 6, 2024

laxmikantchintakindi Nov 15, 2024

carlbuchmann left a comment

laxmikantchintakindi commented Oct 30, 2024

github-actions bot commented Nov 12, 2024

github-actions bot commented Nov 15, 2024

ClausHolbechArista Nov 18, 2024

laxmikantchintakindi Nov 18, 2024

ClausHolbechArista Nov 18, 2024

laxmikantchintakindi Nov 18, 2024

ClausHolbechArista Nov 18, 2024

laxmikantchintakindi Nov 18, 2024

sonarcloud bot commented Nov 18, 2024

-            description: Set `dynamic` instead of `bmca`(deprecated).
+            description: |-
+              PTP role of the endpoint.
+              `follower` will configure the switch port as `ptp role master`.
+              `dynamic` will use BMCA.
+              `default` is deprecated in favor of `follower`.
+              `bmca` is deprecated in favor of `dynamic`.

Feat(eos_designs): Improve ptp settings for p2p_links #4612

Are you sure you want to change the base?

Feat(eos_designs): Improve ptp settings for p2p_links #4612

Conversation

laxmikantchintakindi commented Oct 18, 2024 • edited Loading

Change Summary

Related Issue(s)

Component(s) name

Proposed changes

How to test

Checklist

User Checklist

Repository Checklist

github-actions bot commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlbuchmann left a comment

Choose a reason for hiding this comment

laxmikantchintakindi commented Oct 30, 2024

github-actions bot commented Nov 12, 2024

github-actions bot commented Nov 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 18, 2024

Quality Gate passed

Feat(eos_designs): Improve `ptp` settings for `p2p_links` #4612

Feat(eos_designs): Improve `ptp` settings for `p2p_links` #4612

laxmikantchintakindi commented Oct 18, 2024 •

edited

Loading