show ip interfaces: fix exception with BGP unnumbered

[bradh352]

What I did

Without this patch an exception is thrown when running show ip interfaces when BGP Unnumbered is configured:

root@sw1:~# show ip interfaces
Traceback (most recent call last):
  File "/usr/local/bin/ipintutil", line 280, in <module>
    main()
  File "/usr/local/bin/ipintutil", line 273, in main
    ip_intfs = get_ip_intfs(af, namespace, display)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/ipintutil", line 236, in get_ip_intfs
    ip_intfs_in_ns = get_ip_intfs_in_namespace(af, namespace, display)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/ipintutil", line 149, in get_ip_intfs_in_namespace
    bgp_peer = get_bgp_peer()
               ^^^^^^^^^^^^^^
  File "/usr/local/bin/ipintutil", line 51, in get_bgp_peer
    local_addr = data[neighbor_ip]['local_addr']
                 ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
KeyError: 'local_addr'

This patch will allow the command to complete successfully. It shouldn't be necessary to actually query FRR for neighbor details, the prior version didn't, it just echo'd back config details.

This is a slightly different patch to what #3565 does. It does not add blank values to the output parameter, it skips due to exception handling.

How I did it

Added try / except logic block around optional key/values.

How to verify it

Configure BGP Unnumbered via FRR Management framework and observe 'show ip interfaces' fails with above error. Apply patch and observe expected output.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

Fixes #3565
Fixes sonic-net/sonic-buildimage#19930
Signed-off-by: Brad House (@bradh352)

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[FengPan-Frank]

LGTM

[FengPan-Frank]

pls help to add exceptional test case, thanks

[bradh352]

@FengPan-Frank I'm not very familiar with the way the test infrastructure is designed, I can see the existing tests for show ip interfaces in show_ip_int_test.py, however its not clear how the running config that is used is loaded.

From some grepping for the IPs the existing test cases use it appears to be loaded via mock_tables/mock_single_asic.py ...

I don't immediately see how to augment the config db, I see some tests do things like:

from .mock_tables import dbconnector
jsonfile_config = os.path.join(mock_db_path, "config_db")
dbconnector.dedicated_dbs['CONFIG_DB'] = jsonfile_config
db = Db()

But I'm pretty sure that ends up replacing the entire config_db instead of augmenting it, is that true?

I guess I could do a bunch of runner.invoke(config.config.commands["bgp"].commands[... to configure it

[FengPan-Frank]

show_ip_int_test

we usually use mock config to do this, refer https://github.com/search?q=repo%3Asonic-net%2Fsonic-utilities%20int_ip_input&type=code

[bradh352]

show_ip_int_test

we usually use mock config to do this, refer https://github.com/search?q=repo%3Asonic-net%2Fsonic-utilities%20int_ip_input&type=code

Ok, that's the way i mentioned I wasn't sure about. Does that replace the entire CONFIG_DB, or does it merge into existing configuration?

I do see it sets os.environ['UTILITIES_UNIT_TESTING'] = "1" in the int_ip_input which is used to config tests instead of os.environ['UTILITIES_UNIT_TESTING'] = "2" which is used in show tests but I don't see any docs on what that actually means. Its not clear where to look in the code either for what it means.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[bradh352]

I've tried quite a few things here and I can't see a way for the ipintutil to ever read anything other than the single or multi asic mock tables. The only thing I've gotten to work is I saw there was a Vlan100 defined in that mock table with a link-local ipv6 address, so I assigned that interface to use BGP Unnumbered and should cover the exception. Hopefully this is good enough.

[bradh352]

@FengPan-Frank my solution appears to have brought coverage up to 100% of the changes, hopefully this is acceptable so it can be merged.