search

sonic-net / sonic-dbsyncd

Python library for sonic/redis database syncing
Other
6 stars 47 forks source link

Fix issue#14 lldp-syncd throws exception from get_sys_capability_list #22

Closed chenkelly closed 5 years ago

chenkelly commented 5 years ago

What I did To fix issue https://github.com/Azure/sonic-dbsyncd/issues/14 and add test cases to parse sys_capability_list for remote devices.

Why I did it Current get_sys_capability_list don't consider the system name of remote device is null case. If devices don't configure host name, it will send out LLDPPDU whose System Name TLV is null string. When sonic device learned the remote device, the chassis object of lldp_json does not include system name string. The capability_list does not get successfully by applying current statement, capability_list = if_attributes['chassis'].values()[0]['capability']. We can execute lldpctl command to see the difference. root@sonic:~# docker exec -it lldp bash root@sonic:/# /usr/sbin/lldpctl -f json [Remote device without system name]

"chassis": { ... "capability": [ { "type": "Bridge", "enabled": true }, { "type": "Router", "enabled": true } ] } [Remote device with system name]

"chassis": { "system_name": { ...... "capability": [ { "type": "Bridge", "enabled": true }, { "type": "Router", "enabled": true } ] } } How I verified it Add test case.

sonic-dbsyncd$ pytest -v ================================================= test session starts ================================================== platform linux2 -- Python 2.7.15rc1, pytest-3.3.2, py-1.5.2, pluggy-0.6.0 -- /usr/bin/python2 cachedir: .cache rootdir: /mnt/d/lldp_syncd/sonic-dbsyncd, inifile: collected 9 items

tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_loc_chassis PASSED                                        [ 11%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_multiple_mgmt_ip PASSED                                   [ 22%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_parse_json PASSED                                         [ 33%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_parse_short PASSED                                        [ 44%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_parse_short_short PASSED                                  [ 55%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_remote_sys_capability_list PASSED                         [ 66%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_single_mgmt_ip PASSED                                     [ 77%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_sync_roundtrip PASSED                                     [ 88%]
tests/test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_timeparse PASSED                                          [100%]

Signed-off-by: kelly_chen kelly_chen@edge-core.com

chenkelly commented 5 years ago

Hi @qiluo-msft, could you help to review this modification? Thanks very much.

qiluo-msft commented 5 years ago

@nikos-github Could you help review?