Skip to content

Commit

Permalink
[config-engine]: Fix bug multiple ports connecting to same neighbor (#…
Browse files Browse the repository at this point in the history
…1005)

The current DEVICE_NEIGHBOR format doesn't support multiple different
ports connecting with same neighbor. Thus the lldpd.conf file is not
generated correctly, causing missing information for LAG members.

This fix reverts the data structure in the previous version of
minigraph parser - using local port as the key and remote port/device
as the value of the map. Sample format is:

DEVICE_NEIGHBOR['Ethernet124'] = {
    'name': 'ARISTA04T1',
    'port': 'Ethernet1/1'
}

The corresponding unit test in test_cfggen is updated.
Add one more unit test for lldpd.conf.j2 verification.

Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
  • Loading branch information
Shuotian Cheng authored Oct 3, 2017
1 parent 7c326e3 commit 72e9476
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 12 deletions.
4 changes: 2 additions & 2 deletions dockers/docker-lldp-sv2/lldpd.conf.j2
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{% for neighbor in DEVICE_NEIGHBOR %}
configure ports {{ DEVICE_NEIGHBOR[neighbor]['local_port'] }} lldp portidsubtype local {{ PORT[DEVICE_NEIGHBOR[neighbor]['local_port']]['alias'] }} description {{ neighbor }}:{{ DEVICE_NEIGHBOR[neighbor]['port'] }}
{% for local_port in DEVICE_NEIGHBOR %}
configure ports {{ local_port }} lldp portidsubtype local {{ PORT[local_port]['alias'] }} description {{ DEVICE_NEIGHBOR[local_port]['name'] }}:{{ DEVICE_NEIGHBOR[local_port]['port'] }}
{% endfor %}
10 changes: 3 additions & 7 deletions src/sonic-config-engine/minigraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,21 +91,17 @@ def parse_png(png, hname):
if enddevice == hname:
if port_alias_map.has_key(endport):
endport = port_alias_map[endport]
neighbors[startdevice] = {'local_port': endport, 'port': startport}
neighbors[endport] = {'name': startdevice, 'port': startport}
else:
if port_alias_map.has_key(startport):
startport = port_alias_map[startport]
neighbors[enddevice] = {'local_port': startport, 'port': endport}
neighbors[startport] = {'name': enddevice, 'port': endport}

if child.tag == str(QName(ns, "Devices")):
for device in child.findall(str(QName(ns, "Device"))):
(lo_prefix, mgmt_prefix, name, hwsku, d_type) = parse_device(device)
device_data = {'lo_addr': lo_prefix, 'type': d_type, 'mgmt_addr': mgmt_prefix, 'hwsku': hwsku }
name = name.replace('"', '')
if neighbors.has_key(name):
neighbors[name].update(device_data)
else:
devices[name] = device_data
devices[name] = device_data

if child.tag == str(QName(ns, "DeviceInterfaceLinks")):
for if_link in child.findall(str(QName(ns, 'DeviceLinkBase'))):
Expand Down
5 changes: 5 additions & 0 deletions src/sonic-config-engine/tests/sample_output/lldpd.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
configure ports Ethernet116 lldp portidsubtype local fortyGigE0/116 description ARISTA02T1:Ethernet1/1
configure ports Ethernet124 lldp portidsubtype local fortyGigE0/124 description ARISTA04T1:Ethernet1/1
configure ports Ethernet112 lldp portidsubtype local fortyGigE0/112 description ARISTA01T1:Ethernet1/1
configure ports Ethernet120 lldp portidsubtype local fortyGigE0/120 description ARISTA03T1:Ethernet1/1

4 changes: 2 additions & 2 deletions src/sonic-config-engine/tests/test_cfggen.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ def test_minigraph_portchannel_interfaces(self):
self.assertEqual(output.strip(), "[('PortChannel01', 'FC00::71/126'), ('PortChannel01', '10.0.0.56/31')]")

def test_minigraph_neighbors(self):
argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v "DEVICE_NEIGHBOR[\'ARISTA01T1\']"'
argument = '-m "' + self.sample_graph_t0 + '" -p "' + self.port_config + '" -v "DEVICE_NEIGHBOR[\'Ethernet124\']"'
output = self.run_script(argument)
self.assertEqual(output.strip(), "{'mgmt_addr': None, 'hwsku': 'Arista', 'lo_addr': None, 'local_port': 'Ethernet112', 'type': 'LeafRouter', 'port': 'Ethernet1/1'}")
self.assertEqual(output.strip(), "{'name': 'ARISTA04T1', 'port': 'Ethernet1/1'}")

def test_minigraph_bgp(self):
argument = '-m "' + self.sample_graph_bgp_speaker + '" -p "' + self.port_config + '" -v "BGP_NEIGHBOR[\'10.0.0.59\']"'
Expand Down
8 changes: 7 additions & 1 deletion src/sonic-config-engine/tests/test_j2files.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@ def test_interfaces(self):

def test_alias_map(self):
alias_map_template = os.path.join(self.test_dir, '..', '..', '..', 'dockers', 'docker-snmp-sv2', 'alias_map.j2')
argument = '-m "' + self.t0_minigraph + '" -p "' + self.t0_port_config + '" -t "' + alias_map_template + '"'
argument = '-m ' + self.t0_minigraph + ' -p ' + self.t0_port_config + ' -t ' + alias_map_template
output = self.run_script(argument)
data = json.loads(output)
self.assertEqual(data["Ethernet4"], "fortyGigE0/4")

def test_lldp(self):
lldpd_conf_template = os.path.join(self.test_dir, '..', '..', '..', 'dockers', 'docker-lldp-sv2', 'lldpd.conf.j2')
argument = '-m ' + self.t0_minigraph + ' -p ' + self.t0_port_config + ' -t ' + lldpd_conf_template + ' > ' + self.output_file
self.run_script(argument)
self.assertTrue(filecmp.cmp(os.path.join(self.test_dir, 'sample_output', 'lldpd.conf'), self.output_file))

def test_teamd(self):

Expand Down

0 comments on commit 72e9476

Please sign in to comment.