[Generic Config Updater] apply-patch ignore-path path is not working

[DavidZagury]

Description

When using apply-patch to update the DB there is an hidden option 'ignore-path' that should be used to ignore validation for config specified by given path.

The validation on apply-path does not ignore the tables given with this flag.

Steps to reproduce the issue:

1. Verify using dynamic buffer.
2. Create a patch to shutdown a port:

admin@r-tigon-21:~$ cat test_down
[
        {
            "op": "replace",
            "path": "/PORT/Ethernet0/admin_status",
            "value": "down"
        }
]

3. Verify the interface status is up:

admin@r-tigon-21:~$ show interface status Ethernet0
  Interface    Lanes    Speed    MTU    FEC    Alias            Vlan    Oper    Admin             Type    Asym PFC
-----------  -------  -------  -----  -----  -------  --------------  ------  -------  ---------------  ----------
  Ethernet0      0,1      50G   9100    N/A    etp1a  PortChannel102      up       up  QSFP28 or later         off

4. Apply the patch

Describe the results you received:

admin@r-tigon-21:~$ sudo config apply-patch -i /BUFFER_PG test_down
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "replace", "path": "/PORT/Ethernet0/admin_status", "value": "down"}]
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
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 sorted into 1 change:
Patch Applier:   * [{"op": "replace", "path": "/PORT/Ethernet0/admin_status", "value": "down"}]
Patch Applier: Applying 1 change in order:
Patch Applier:   * [{"op": "replace", "path": "/PORT/Ethernet0/admin_status", "value": "down"}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Logs show:

Jun 7 07:58:49.028286 r-tigon-21 ERR config: Change Applier: run_data vs expected_data: {'BUFFER_PG': {'Ethernet0|3': {'profile'
Jun 7 07:58:49.028426 r-tigon-21 ERR config: Change Applier: Failed to apply Json change

Describe the results you expected:

Should pass.

Output of show version:

(paste your output here)

Output of show techsupport:

(paste your output here or download and attach the file here )

Additional information you deem important (e.g. issue happens only occasionally):

The BUFFER_PG table does change when shutting down a port:

root@r-tigon-21:/home/admin# config interface startup Ethernet0
root@r-tigon-21:/home/admin# redis-cli -n 4 keys "BUFFER_PG|Ethernet0*"

1. "BUFFER_PG|Ethernet0|0"
2. "BUFFER_PG|Ethernet0|3-4"
   root@r-tigon-21:/home/admin# config interface shutdown Ethernet0
   root@r-tigon-21:/home/admin# redis-cli -n 4 keys "BUFFER_PG|Ethernet0*"
3. "BUFFER_PG|Ethernet0|0"

But it seems that the -i flag doesn't work and doesn't ignore tables giving to it.
After update is done on src/sonic-utilities/generic_config_updater/change_applier.py in function apply(self, change)

There is a validation:
https://github.com/Azure/sonic-utilities/blob/ac2f55306e9f5f0f9fad60f814bcc777f685d2b4/generic_config_updater/change_applier.py#L145

            if upd_data != run_data:
                self._report_mismatch(run_data, upd_data)
                ret = -1

Which take the new db and compare it to the expected db, there is no mention of what we gave it on the ignore flag, which results in a the error.

[ghooo]

This seems like a problem in the box, shutting down a port has a side effect of changing BUFFER_PG table. @qiluo-msft do you have suggestions about who can help with this?

ignore_path i.e -i option is used for ignoring tables or fields from YANG validation. This is used to unblock E2E testing of the GCU feature especially the YANG models are not complete and the test DUTS they have different YANG validation issues which are not related to E2E testing. Also ignore_path should not be used for production, it is a great risk to skip validation while working on real devices. This is the reason why the feature is hidden. Please check the PR adding ignore_path for more info. sonic-net/sonic-utilities#1929

[qiluo-msft]

@DavidZagury The ERR syslog is generated by this code line https://github.com/Azure/sonic-utilities/blob/6de18a1ded43a5bec02af5732fc4c64e551d8a42/generic_config_updater/change_applier.py#L129 and it is truncated. Could you just add some debug output and capture run_data, upd_data there?

@renukamanavalan Could you please help check also?

[renukamanavalan]

@ghooo is correct w.r.t YANG validation.
It appears that the patch did not get applied to the running config.
Someone has to repro and debug.

[wen587]

I think when use GCU to shut down the port, GCU will compare the running config and target config to make sure it makes the change and only make the change to the Ethernet0's admin_status.
The operation will fail if changes other than admin_status are changed.

In this example(check Additional information in this issue), shutting down the port will result in "BUFFER_PG|Ethernet0|3-4" disappearing. The BUFFER_PG seems to have dependent changes to port admin_status. I guess this is why the GCU fail.

[DavidZagury]

@DavidZagury The ERR syslog is generated by this code line https://github.com/Azure/sonic-utilities/blob/6de18a1ded43a5bec02af5732fc4c64e551d8a42/generic_config_updater/change_applier.py#L129 and it is truncated. Could you just add some debug output and capture run_data, upd_data there?

@renukamanavalan Could you please help check also?

I have add a breakpoint and printed the run_data and the upd_data, since it is the full db it is quite long to add as a comment here, I added it to here:
run_data:
https://justpaste.it/3ag9p
upd_data:
https://justpaste.it/5hso8

As I said, the difference is only on the BUFFER_PG where a profile was removed.
The patch did get updated and the port did updated as it should have, the error was only due to this change in the buffer table.

[stephenxs]

Hi
We understand applying patches via using sudo config apply-patch -i /BUFFER_PG test_down can fail if there is a difference in CONFIG_DB between and after the patch is applied.
But by design, the traditional buffer manager will update the BUFFER_PG and BUFFER_PROFILE table on ports' speed or admin_status updated.
As a result, it will fail to apply a patch containing speed and admin_status updates in the PORT table.
We just want to clarify what's the scope of GCU.
For example, is it designed for updating the configurations which have corresponding CLI.

[zhangyanzhao]

@qiluo-msft can you please find someone in your team to take a look at this issue?

[qiluo-msft]

@wen587 will work on a fix.