Add Multi ASIC support for apply-patch #3249

xincunli-sonic · 2024-03-28T23:02:58Z

What I did

Add Multi-ASIC GCU support apply-patch.

ADO

26574257

How I did it

Categorize configuration as JSON patch format per ASIC.
Apply patch per ASIC, including localhost.

How to verify it

Happy case

Reverse case

Failed case

Empty case

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)

xincunli-sonic · 2024-03-28T23:21:52Z

The old PR's conversation is here: #3219, the old PR was mistakenly deleting commits. #Resolved

tests/generic_config_updater/multiasic_generic_updater_test.py

tests/generic_config_updater/multiasic_change_applier_test.py

wen587

LGTM. Please check with other reviewer.

config/main.py

generic_config_updater/generic_updater.py

generic_config_updater/gu_common.py

config/main.py

generic_config_updater/change_applier.py

qiluo-msft · 2024-04-04T18:18:41Z

class ChangeApplier:

ChangeApplier could add a member self.config_db for the ConfigDBConnector. and rename existing self.config_db to self.running_config #Closed

Refers to: generic_config_updater/change_applier.py:73 in 60bbcfe. [](commit_id = 60bbcfe, deletion_comment = False)

config/main.py

generic_config_updater/change_applier.py

generic_config_updater/generic_updater.py

tests/config_test.py

xincunli-sonic · 2024-04-05T00:11:47Z

class ChangeApplier:

This is real db connector object, please ref this function

def set_config(config_db, tbl, key, data):
    config_db.set_entry(tbl, key, data)

In reply to: 2037887648

Refers to: generic_config_updater/change_applier.py:73 in 60bbcfe. [](commit_id = 60bbcfe, deletion_comment = False)

xincunli-sonic · 2024-04-12T04:31:46Z

Hi @xincunli-sonic , I think of this after todays meeting. Single asic can utilize empty patch for YANG validation, can multi-asic also support empty patch validation?

admin@bjw-can-7260-11:~$ cat empty.json 
[]
admin@bjw-can-7260-11:~$ sudo config apply-patch empty.json 
Patch Applier: Patch application starting.
Patch Applier: Patch: []
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating all JsonPatch operations are permitted on the specified fields
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Patch Applier: The patch was converted into 0 changes.
Patch Applier: Applying 0 changes in order.
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.

Hi @xincunli-sonic , I think of this after todays meeting. Single asic can utilize empty patch for YANG validation, can multi-asic also support empty patch validation?

admin@bjw-can-7260-11:~$ cat empty.json 
[]
admin@bjw-can-7260-11:~$ sudo config apply-patch empty.json 
Patch Applier: Patch application starting.
Patch Applier: Patch: []
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating all JsonPatch operations are permitted on the specified fields
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Patch Applier: The patch was converted into 0 changes.
Patch Applier: Applying 0 changes in order.
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.

Thanks, Good point, added empty case handle which can drive YANG validation for host and all ASICs. #Resolved

JunhongMao · 2024-04-16T18:02:40Z

Hello， @xincunli-sonic， I'm working on the unit test codes development for this PR. Now I couldn't compile an image including your modification. The error is below. Could you help provide some hints on this? And what's the sonic-buildimage's URL and branch that you are using while developing and testing this PR?
Thanks.

Using /sonic/src/sonic-utilities/.eggs/responses-0.25.0-py3.9.egg
running egg_info
writing sonic_utilities.egg-info/PKG-INFO
writing dependency_links to sonic_utilities.egg-info/dependency_links.txt
writing entry points to sonic_utilities.egg-info/entry_points.txt
writing requirements to sonic_utilities.egg-info/requires.txt
writing top-level names to sonic_utilities.egg-info/top_level.txt
reading manifest file 'sonic_utilities.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching '*' under directory 'data'
writing manifest file 'sonic_utilities.egg-info/SOURCES.txt'
running build_ext
ImportError while loading conftest '/sonic/src/sonic-utilities/tests/conftest.py'.
tests/conftest.py:23: in <module>
    import config.main as config
config/main.py:28: in <module>
    from sonic_py_common.general import getstatusoutput_noshell
E   ImportError: cannot import name 'getstatusoutput_noshell' from 'sonic_py_common.general' (/var/jumao/.local/lib/python3.9/site-packages/sonic_py_common/general.py)
[  FAIL LOG END  ] [ target/python-wheels/bullseye/sonic_utilities-1.2-py3-none-any.whl ]
make: *** [slave.mk:766: target/python-wheels/bullseye/sonic_utilities-1.2-py3-none-any.whl] Error 1
make[1]: *** [Makefile.work:448: target/sonic-broadcom.bin] Error 2
make[1]: Leaving directory '/home/jumao/m/msft-2205-ndk-20240402/sonic-buildimage'
make: *** [Makefile:33: target/sonic-broadcom.bin] Error 2

``` #Resolved

xincunli-sonic · 2024-04-18T18:13:14Z

Hello， @xincunli-sonic， I'm working on the unit test codes development for this PR. Now I couldn't compile an image including your modification. The error is below. Could you help provide some hints on this? And what's the sonic-buildimage's URL and branch that you are using while developing and testing this PR? Thanks.

Using /sonic/src/sonic-utilities/.eggs/responses-0.25.0-py3.9.egg
running egg_info
writing sonic_utilities.egg-info/PKG-INFO
writing dependency_links to sonic_utilities.egg-info/dependency_links.txt
writing entry points to sonic_utilities.egg-info/entry_points.txt
writing requirements to sonic_utilities.egg-info/requires.txt
writing top-level names to sonic_utilities.egg-info/top_level.txt
reading manifest file 'sonic_utilities.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching '*' under directory 'data'
writing manifest file 'sonic_utilities.egg-info/SOURCES.txt'
running build_ext
ImportError while loading conftest '/sonic/src/sonic-utilities/tests/conftest.py'.
tests/conftest.py:23: in <module>
    import config.main as config
config/main.py:28: in <module>
    from sonic_py_common.general import getstatusoutput_noshell
E   ImportError: cannot import name 'getstatusoutput_noshell' from 'sonic_py_common.general' (/var/jumao/.local/lib/python3.9/site-packages/sonic_py_common/general.py)
[  FAIL LOG END  ] [ target/python-wheels/bullseye/sonic_utilities-1.2-py3-none-any.whl ]
make: *** [slave.mk:766: target/python-wheels/bullseye/sonic_utilities-1.2-py3-none-any.whl] Error 1
make[1]: *** [Makefile.work:448: target/sonic-broadcom.bin] Error 2
make[1]: Leaving directory '/home/jumao/m/msft-2205-ndk-20240402/sonic-buildimage'
make: *** [Makefile:33: target/sonic-broadcom.bin] Error 2

What I forked the branch is an old version, if you want to test, please do some manual work to align with the latest change.

JunhongMao · 2024-04-19T15:39:14Z

There is a problem: the command "config apply-patch" caused some containers to exit.

admin@ixre-egl-board40:~$ docker ps
CONTAINER ID   IMAGE                             COMMAND                  CREATED       STATUS         PORTS     NAMES
baeabdcd38a9   70dd58e31088                      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             macsec0
bfe465afc0aa   docker-router-advertiser:latest   "/usr/bin/docker-ini…"   2 hours ago   Up 2 minutes             radv
b710a81b05b9   70dd58e31088                      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             macsec1
3a8ddbc204ad   docker-fpm-frr:latest             "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp0
e553479d15b1   docker-fpm-frr:latest             "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp1
4e35cf0f962f   docker-syncd-brcm-dnx:latest      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd0
14f9109c93cc   docker-teamd:latest               "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             teamd0
f77fd9c53cd7   docker-syncd-brcm-dnx:latest      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd1
57209995a85b   docker-teamd:latest               "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             teamd1
fd030291d9fa   docker-orchagent:latest           "/usr/bin/docker-ini…"   2 hours ago   Up 2 minutes             swss1
cc0377006913   docker-eventd:latest              "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             eventd
ae7c871d4e37   docker-database:latest            "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database1
aeb234180f94   docker-database:latest            "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database0
f0ed626a8152   docker-database:latest            "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database
admin@ixre-egl-board40:~$ cat empty.json
[]

admin@ixre-egl-board40:~$ sudo config apply-patch empty.json -d
** DRY RUN EXECUTION **
Patch Applier: Patch application starting.
Patch Applier: Patch: []
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating all JsonPatch operations are permitted on the specified fields
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Patch Applier: The patch was converted into 0 changes.
Patch Applier: Applying 0 changes in order.
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.
admin@ixre-egl-board40:~$ docker ps
CONTAINER ID   IMAGE                          COMMAND                  CREATED       STATUS         PORTS     NAMES
3a8ddbc204ad   docker-fpm-frr:latest          "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp0
e553479d15b1   docker-fpm-frr:latest          "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp1
4e35cf0f962f   docker-syncd-brcm-dnx:latest   "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd0
f77fd9c53cd7   docker-syncd-brcm-dnx:latest   "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd1
cc0377006913   docker-eventd:latest           "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             eventd
ae7c871d4e37   docker-database:latest         "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database1
aeb234180f94   docker-database:latest         "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database0
f0ed626a8152   docker-database:latest         "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database
admin@ixre-egl-board40:~$

generic_config_updater/change_applier.py

xincunli-sonic · 2024-04-23T18:35:27Z

There is a problem: the command "config apply-patch" caused some containers to exit.

admin@ixre-egl-board40:~$ docker ps
CONTAINER ID   IMAGE                             COMMAND                  CREATED       STATUS         PORTS     NAMES
baeabdcd38a9   70dd58e31088                      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             macsec0
bfe465afc0aa   docker-router-advertiser:latest   "/usr/bin/docker-ini…"   2 hours ago   Up 2 minutes             radv
b710a81b05b9   70dd58e31088                      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             macsec1
3a8ddbc204ad   docker-fpm-frr:latest             "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp0
e553479d15b1   docker-fpm-frr:latest             "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp1
4e35cf0f962f   docker-syncd-brcm-dnx:latest      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd0
14f9109c93cc   docker-teamd:latest               "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             teamd0
f77fd9c53cd7   docker-syncd-brcm-dnx:latest      "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd1
57209995a85b   docker-teamd:latest               "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             teamd1
fd030291d9fa   docker-orchagent:latest           "/usr/bin/docker-ini…"   2 hours ago   Up 2 minutes             swss1
cc0377006913   docker-eventd:latest              "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             eventd
ae7c871d4e37   docker-database:latest            "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database1
aeb234180f94   docker-database:latest            "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database0
f0ed626a8152   docker-database:latest            "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database
admin@ixre-egl-board40:~$ cat empty.json
[]

admin@ixre-egl-board40:~$ sudo config apply-patch empty.json -d
** DRY RUN EXECUTION **
Patch Applier: Patch application starting.
Patch Applier: Patch: []
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating all JsonPatch operations are permitted on the specified fields
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Patch Applier: The patch was converted into 0 changes.
Patch Applier: Applying 0 changes in order.
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.
admin@ixre-egl-board40:~$ docker ps
CONTAINER ID   IMAGE                          COMMAND                  CREATED       STATUS         PORTS     NAMES
3a8ddbc204ad   docker-fpm-frr:latest          "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp0
e553479d15b1   docker-fpm-frr:latest          "/usr/bin/docker_ini…"   2 hours ago   Up 2 minutes             bgp1
4e35cf0f962f   docker-syncd-brcm-dnx:latest   "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd0
f77fd9c53cd7   docker-syncd-brcm-dnx:latest   "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             syncd1
cc0377006913   docker-eventd:latest           "/usr/local/bin/supe…"   2 hours ago   Up 2 minutes             eventd
ae7c871d4e37   docker-database:latest         "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database1
aeb234180f94   docker-database:latest         "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database0
f0ed626a8152   docker-database:latest         "/usr/local/bin/dock…"   2 hours ago   Up 2 minutes             database
admin@ixre-egl-board40:~$

This has been identified by a known issue when we change a conflicting lanes for a port, but it is not related with GCU change self.

config/main.py

generic_config_updater/change_applier.py

generic_config_updater/gu_common.py

* Add Multi ASIC support for apply-patch * Add more test cases. * Ignore mock diff exception * Address comments. * Fix errors * Add empty case handle * Refactor extract scope * Fix UT * Fix extract for single asic * Adding localhost into log if scope is empty * Fix format in log * Fix log * Fix log * Fix variable

Add Multi ASIC support for apply-patch

a8f993d

xincunli-sonic requested review from isabelmsft, wen587, qiluo-msft and ganglyu March 28, 2024 23:24

xincunli-sonic marked this pull request as ready for review March 28, 2024 23:26

wen587 previously approved these changes Apr 2, 2024

View reviewed changes

wen587 reviewed Apr 2, 2024

View reviewed changes

tests/generic_config_updater/multiasic_generic_updater_test.py Show resolved Hide resolved

wen587 reviewed Apr 2, 2024

View reviewed changes

tests/generic_config_updater/multiasic_change_applier_test.py Outdated Show resolved Hide resolved

Add more test cases.

84e53ee

xincunli-sonic dismissed wen587’s stale review via 84e53ee April 2, 2024 21:33

Ignore mock diff exception

60bbcfe

wen587 previously approved these changes Apr 3, 2024

View reviewed changes

isabelmsft reviewed Apr 3, 2024

View reviewed changes

config/main.py Outdated Show resolved Hide resolved

generic_config_updater/generic_updater.py Outdated Show resolved Hide resolved

generic_config_updater/gu_common.py Show resolved Hide resolved

generic_config_updater/gu_common.py Show resolved Hide resolved