fix show interface neighbor exp issue

[jcaiMR]

Why I did it

With the commit of #11894.
device neighbor metadata in config_db.json changed a little bit when "lo_addr"/"mgmt_addr" are None.

CLI parse code for "show interface neighbor expected" is not strong enough to handle None case.
It always assume 'lo_addr' and 'mgmt_addr' exists, and will finally cause KeyError, show command will not print the information.

for port in natsorted(list(neighbor_dict.keys())):
            try:
                device = neighbor_dict[port]['name']
                body.append([port,
                             device,
                             neighbor_dict[port]['port'],
                             neighbor_metadata_dict[device]['lo_addr'],
                             neighbor_metadata_dict[device]['mgmt_addr'],
                             neighbor_metadata_dict[device]['type']])
            except KeyError:
                pass

How I did it

Since in code many places don't check the existence of 'lo_addr' and 'mgmt_addr', for safety always add 'lo_addr' and 'mgmt_addr' in minigraph.py

How to verify it

"show interface neighbor expected" with fix image

Which release branch to backport (provide reason below if selected)

• 201811
• 201911
• 202006
• 202012
• 202106
• 202111
• 202205

fixes #12301

[ganglyu]

Why are they must fields?
And we should use "mandatory true" in yang models for must field.

[jcaiMR]

Thanks for comments, maybe can't say must fields. Seems for historical reason, code never check if 'lo_addr' and 'mgmt_addr' keys are exists or not. In minigraph, 'lo_addr' and 'mgmt_addr' sometime may not exists, but after deploy mg we generated config_db.json, code parse config_db.json will access 'lo_addr' and 'mgmt_addr' without confirm if keys are exists.
Another fix may find all places referring 'lo_addr' and 'mgmt_addr', but they are many places and for safety I decided to change the behavior back.

The description "# 'lo_addr', ..." here seems a little bit confuse, I will remove this description.

[ganglyu]

When 'lo_addr' and 'mgmt_addr' keys do not exists in minigraph, the value in config_db will be ‘None’.
How does consumer for this table handle this value?

[jcaiMR]

minigraph.py will first save 'lo_addr':None to DEVICE_NEIGHBOE_METADATA

if asic_name is None:
        results['DEVICE_NEIGHBOR_METADATA'] = { key:devices[key] for key in devices if key.lower() != hostname.lower() }
    else:
        results['DEVICE_NEIGHBOR_METADATA'] = { key:devices[key] for key in devices if key in {device['name'] for device in neighbors.values()} }

Then results to json will treat None as "None" in final config_db.json, and in show cli python code, it will access the value in config_db.json, that is "lo_addr":"None". So the final show result is something like this:

admin@str-dx010-acs-5:~$ show interface neighbor exp
LocalPort    Neighbor    NeighborPort    NeighborLoopback    NeighborMgmt    NeighborType
-----------  ----------  --------------  ------------------  --------------  --------------
Ethernet112  ARISTA01T1  Ethernet1       None                10.64.247.220   LeafRouter
Ethernet116  ARISTA02T1  Ethernet1       None                10.64.247.221   LeafRouter
Ethernet120  ARISTA03T1  Ethernet1       None                10.64.247.222   LeafRouter
Ethernet124  ARISTA04T1  Ethernet1       None                10.64.247.223   LeafRouter

[StormLiangMS]

/azp run sonic-buildimage

[azure-pipelines]

No pipelines are associated with this pull request.

[jcaiMR]

Hold on the commit, team make a discussion to have another solution which will make code change in sonic-utilities repo.
Related PR:
https://github.com/sonic-net/sonic-utilities/pull/2465/files

[jcaiMR]

Resolved by https://github.com/sonic-net/sonic-utilities/pull/2465/files
Close this PR。