Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add verification IPv4 address on LLDP conf Jinja2 Template #5699

Merged
merged 9 commits into from
Nov 7, 2020
Merged

Add verification IPv4 address on LLDP conf Jinja2 Template #5699

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d42ae43
Add verification IPv4 address on LLDP conf Jinja2 Template
bratashX Oct 22, 2020
bda9e7d
Code improvement
bratashX Oct 23, 2020
ad2001f
Fix config file for correct work test_j2files.py(test_lldp)
bratashX Oct 29, 2020
de774cb
Revert "Fix config file for correct work test_j2files.py(test_lldp)"
bratashX Oct 29, 2020
a5f32d2
Merge branch 'master' into fix_lldp_jinja2_template
bratashX Nov 5, 2020
ad58479
Add test for lldp.conf
bratashX Nov 5, 2020
0e81cc3
Code refactoring
bratashX Nov 5, 2020
8464938
Add new case to test_lldp and remove link to output files
bratashX Nov 6, 2020
3281d2f
Use json-file instead of minigraph
bratashX Nov 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions dockers/docker-lldp/lldpd.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
{% set mgmt_if = {} %}
{% if MGMT_INTERFACE %}
{% for interface in MGMT_INTERFACE.keys()|list %}
{% if interface[1]|ipv4 %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ipv4 [](start = 19, length = 4)

Could you add a test case similar to https://github.com/Azure/sonic-buildimage/blob/781188f54941a2eb9e4a23a96f05986ec51ff106/src/sonic-config-engine/tests/test_j2files.py#L69? So it will capture regression like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test was added, please review

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding test! I think another useful test case is 1 ipv4 + 1 ipv6 mgmt addresses, which are common to in normal use case.

In reply to: 515413789 [](ancestors = 515413789)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test must verify 2 cases:

  • IPv4 mgmt interface exists
  • IPv4 mgmt interface doesn't exist

But now I add 3 cases, when:

  • IPv4 mgmt interface only exists
  • IPv6 mgmt interface only exists
  • IPv4 + IPv6 mgmt interface exists

Please review

{% if mgmt_if.update({'port_name' : interface[0]}) %} {% endif %}
{% if mgmt_if.update({'ipv4' : interface[1].split('/')[0]}) %} {% endif %}
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% endfor %}
{% endif %}
{% if mgmt_if %}
{# If MGMT port alias is available, use it for port ID subtype, otherwise use port name #}
{% set mgmt_port_name = (MGMT_INTERFACE.keys()|list)[0][0] %}
{% set ipv4 = (MGMT_INTERFACE.keys()|list)[0][1].split('/')[0] %}
{% if MGMT_PORT and MGMT_PORT[mgmt_port_name] and MGMT_PORT[mgmt_port_name].alias %}
configure ports eth0 lldp portidsubtype local {{ MGMT_PORT[mgmt_port_name].alias }}
{% if MGMT_PORT and MGMT_PORT[mgmt_if.port_name] and MGMT_PORT[mgmt_if.port_name].alias %}
configure ports eth0 lldp portidsubtype local {{ MGMT_PORT[mgmt_if.port_name].alias }}
{% else %}
configure ports eth0 lldp portidsubtype local {{ mgmt_port_name }}
configure ports eth0 lldp portidsubtype local {{ mgmt_if.port_name }}
{% endif %}
configure system ip management pattern {{ ipv4 }}
configure system ip management pattern {{ mgmt_if.ipv4 }}
{% endif %}
configure system hostname {{ DEVICE_METADATA['localhost']['hostname'] }}