Multi-asic support for ApplyPatchDb API for gNMI

[agadia-cisco]

• gNMI uses Dbus ApplyPatchDb API. Currently it doesn't use namespace & it has to be updated.

• mixed_db_client.go calls ApplyPatchDb here, ApplyPatchDb definition here

• How this was discovered?

  • when creating config checkpoint by using Dbus API; then configuration returned is the namespace config-db.
    dbus-send --system --dest=org.SONiC.HostService.gcu --type=method_call --print-reply /org/SONiC/HostService/gcu org.SONiC.HostService.gcu.create_checkpoint string:'checkpoint1'

  • but when run ApplyPatchDb API, then it isn't considering the PORTS viz in the namespace config-db & if given the port name which exists in config_db (not the namespace one), then also ApplyPatchDb API fails.

    • dbus-send --system --dest=org.SONiC.HostService.gcu --type=method_call --print-reply /org/SONiC/HostService/gcu org.SONiC.HostService.gcu.apply_patch_db string:'[{"op": "add", "path": "/PORT/Ethernet0/admin_status", "value": "down"}]' => fails with error "'PORT' not found in ....(returns entire config_db.json)"

    • dbus-send --system --dest=org.SONiC.HostService.gcu --type=method_call --print-reply /org/SONiC/HostService/gcu org.SONiC.HostService.gcu.apply_patch_db string:'[{"op": "add", "path": "/localhost/PORT/Eth0~10~10~10/admin_status", "value": "down"}]' => "Error: Failed to apply patch on the following scopes:"

[levhuan]

Per this change GCU apply-patch to support multi-asic, the namespace/asic ID is the prefix in the path in JSON file. Will need to change MixedDbClient.ConvertToJsonPatch() to include the asic namespace in the path.

Is this a known issue? If not, Cisco can work on a PR for this.

[rlhui]

Per this change GCU apply-patch to support multi-asic, the namespace/asic ID is the prefix in the path in JSON file. Will need to change MixedDbClient.ConvertToJsonPatch() to include the asic namespace in the path.

Is this a known issue? If not, Cisco can work on a PR for this.

@ganglyu please help answer? thanks.

[ganglyu]

We haven't verified this feature with a multi-ASIC device. Your assistance in resolving this issue would be greatly appreciated.

[rlhui]

@xincunli-sonic is this on your radar? Please coordinate with cisco as you see fit, thanks.

[xincunli-sonic]

@rlhui Yes, I will be working it, ADO: 27231493.

[xincunli-sonic]

Hi @levhuan , @agadia-cisco, please let me know if you would already have a plan to fix this. The main change will be in ConvertToJsonPatch and its caller to append given scope or more works on each test case.

Here are some references:

1. DB Design for multi-ASIC HLD.
2. Add scope into JSON patch of existing GCU testcases for Multi ASIC.

[krishnagorrepati]

Hi @levhuan , @agadia-cisco, please let me know if you would already have a plan to fix this. The main change will be in ConvertToJsonPatch and its caller to append given scope or more works on each test case.

Here are some references:

1. DB Design for multi-ASIC HLD.
2. Add scope into JSON patch of existing GCU testcases for Multi ASIC.

@xincunli-sonic, Cisco is working on this. The fix is adding namespace/scope in JSon path. We made fix based on this understanding and evaluating the fix internally. Will update you if we face any issues with this approach.

[agadia-cisco]

@xincunli-sonic We have raised sonic-gnmi PR - sonic-net/sonic-gnmi#356

& sonic-mgmt PRs -

• Added Multi-Asic Handling for test_gnmi_configdb_full_01 TC sonic-mgmt#15572
• Added Multi-Asic Handling for test_gnmi_configdb script sonic-mgmt#16827

Out of 2 TCs failing, 1 passes now, and other one viz failing is not due to gnmi commands failure, instead its due to missing multi-asic support for config_reload causing gnoi_reboot to fail & gnoi_reboot is used in test_gnmi_configdb.py::test_gnmi_configdb_full_01 TC & issue details are logged here - sonic-net/sonic-host-services#212

We have validated our sonic-gnmi PR changes with our sonic-mgmt PR changes on our T2 setup & the fix solves the issue raised.

cc - @krishnagorrepati