[generic-config-updater] Adding non-strict mode

[ghooo]

What I did

Added non-strict mode to patch sorting which is useful while building the E2E tests for the framework. It helps developers avoid the incompleteness of YANG models.

Was implementing by adding 2 options:

• Ignore tables without yang: This flag will ignore from validation all tables that does not have YANG model defined yet
• Ignore path: This flag can explicitly ignore a path from validation, which can ignore any config. It will be useful for ignoring configs with YANG but the YANG is not up-to-date.

⚠️ The non-strict mode is only meant for helping with E2E testing and not meant for production

How I did it

Added flags ignore-non-yang-tables and ignore-path to the config commands:

config apply-patch
config replace
config rollback

Added NonStrictSorter, together with the older one which is the StrictSorter.

NonStrictSorter groups configs into 2 groups, YANG covered configs, and Non-YANG covered configs

• Non-YANG covered configs are the tables without YANG models, and the fields/tables ignored explicitly by the -i CLI option. The JsonPatch between Non-YANG current and Non-YANG target configs is generated and is clubbed together as a single JsonChange i.e. we will make a single call to the ChangeApplier for Non-YANG changes
• YANG covered configs are the rest of the configs. They are handled using the normal sorter.

Check implementation for further details

How to verify it

unit-test

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

admin@vlab-01:~$ sudo config apply-patch -h
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH

  Apply given patch of updates to Config. A patch is a JsonPatch which
  follows rfc6902. This command can be used do partial updates to the config
  with minimum disruption to running processes. It allows addition as well
  as deletion of configs. The patch file represents a diff of ConfigDb(ABNF)
  format or SonicYang format.

  <patch-file-path>: Path to the patch file on the file-system.

Options:
  -f, --format [CONFIGDB|SONICYANG]
                                  format of config of the patch is either
                                  ConfigDb(ABNF) or SonicYang
  -d, --dry-run                   test out the command without affecting
                                  config state
  -v, --verbose                   print additional details of what the
                                  operation is doing
  -h, -?, --help                  Show this message and exit.
admin@vlab-01:~$ sudo config replace -h
Usage: config replace [OPTIONS] TARGET_FILE_PATH

  Replace the whole config with the specified config. The config is replaced
  with minimum disruption e.g. if ACL config is different between current
  and target config only ACL config is updated, and other config/services
  such as DHCP will not be affected.

  **WARNING** The target config file should be the whole config, not just
  the part intended to be updated.

  <target-file-path>: Path to the target file on the file-system.

Options:
  -f, --format [CONFIGDB|SONICYANG]
                                  format of target config is either
                                  ConfigDb(ABNF) or SonicYang
  -d, --dry-run                   test out the command without affecting
                                  config state
  -v, --verbose                   print additional details of what the
                                  operation is doing
  -h, -?, --help                  Show this message and exit.
admin@vlab-01:~$ sudo config rollback -h
Usage: config rollback [OPTIONS] CHECKPOINT_NAME

  Rollback the whole config to the specified checkpoint. The config is
  rolled back with minimum disruption e.g. if ACL config is different
  between current and checkpoint config only ACL config is updated, and
  other config/services such as DHCP will not be affected.

  <checkpoint-name>: The checkpoint name, use `config list-checkpoints`
  command to see available checkpoints.

Options:
  -d, --dry-run   test out the command without affecting config state
  -v, --verbose   print additional details of what the operation is doing
  -?, -h, --help  Show this message and exit.
admin@vlab-01:~$ 

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

Same as old command output since the options are hidden

The new added options are:

--n --ignore-non-yang-tables        ignore validation for tables without YANG models
--i --ignore-path                   ignore validation for config specified by given path which is a JsonPointer

Example of usages:

• KVM SONiC image that has multiple YANG errors in NEIGHBOR_BGP, DEVICE_METADATA, and VLAN tables.

Applying empty-patch without any non-strict mode flags

admin@vlab-01:~$ sudo config apply-patch empty.json-patch 
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: Sorting patch updates.
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
sonic_yang(3):All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['10.0.0.57', '10.0.0.59', '10.0.0.61', '10.0.0.63', 'fc00::72', 'fc00::76', 'fc00::7a', 'fc00::7e'])
sonic_yang(3):exceptionList:["'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", 'Value not found for vrf_name neighbor in 10.0.0.57', 'Value not found for vrf_name neighbor in 10.0.0.59', 'Value not found for vrf_name neighbor in 10.0.0.61', 'Value not found for vrf_name neighbor in 10.0.0.63', 'Value not found for vrf_name neighbor in fc00::72', 'Value not found for vrf_name neighbor in fc00::76', 'Value not found for vrf_name neighbor in fc00::7a', 'Value not found for vrf_name neighbor in fc00::7e']
sonic_yang(3):Data Loading Failed:All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['10.0.0.57', '10.0.0.59', '10.0.0.61', '10.0.0.63', 'fc00::72', 'fc00::76', 'fc00::7a', 'fc00::7e'])
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: Given patch is not valid because it will result in an invalid config
admin@vlab-01:~

Applying command and ignoring the tables/fields with invalid YANG

admin@vlab-01:~$ sudo config apply-patch empty.json-patch -i /BGP_NEIGHBOR -i /DEVICE_METADATA -i /VLAN/Vlan1000/dhcpv6_servers -i /VLAN/Vlan1000/members
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 target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
Patch Applier: The patch was sorted 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@vlab-01:~$

• Patch updating a mix of tables with YANG models and others without YANG models

Applying command without any non-strict mode flags ... fails because it is updating tables without YANG

admin@vlab-01:~$ sudo config apply-patch mix.json-patch
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "remove", "path": "/ACL_TABLE/DATAACL"}, {"op": "remove", "path": "/SYSLOG_SERVER"}]
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.
Note: Below table(s) have no YANG models:
SYSLOG_SERVER, 
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: Given patch is not valid because it has changes to tables without YANG models
admin@vlab-01:~$ 

Adding -n also fails because the tables with YANG models have validation violations

admin@vlab-01:~$ sudo config apply-patch mix.json-patch -n
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "remove", "path": "/ACL_TABLE/DATAACL"}, {"op": "remove", "path": "/SYSLOG_SERVER"}]
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.
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
sonic_yang(3):All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['10.0.0.57', '10.0.0.59', '10.0.0.61', '10.0.0.63', 'fc00::72', 'fc00::76', 'fc00::7a', 'fc00::7e'])
sonic_yang(3):exceptionList:["'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", "'admin_status'", 'Value not found for vrf_name neighbor in 10.0.0.57', 'Value not found for vrf_name neighbor in 10.0.0.59', 'Value not found for vrf_name neighbor in 10.0.0.61', 'Value not found for vrf_name neighbor in 10.0.0.63', 'Value not found for vrf_name neighbor in fc00::72', 'Value not found for vrf_name neighbor in fc00::76', 'Value not found for vrf_name neighbor in fc00::7a', 'Value not found for vrf_name neighbor in fc00::7e']
sonic_yang(3):Data Loading Failed:All Keys are not parsed in BGP_NEIGHBOR
dict_keys(['10.0.0.57', '10.0.0.59', '10.0.0.61', '10.0.0.63', 'fc00::72', 'fc00::76', 'fc00::7a', 'fc00::7e'])
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: Given patch is not valid because it will result in an invalid config
admin@vlab-01:~$ 

Adding -n option and ignoring the tables with YANG validation violations, works

admin@vlab-01:~$ sudo config apply-patch mix.json-patch -n -i /BGP_NEIGHBOR -i /DEVICE_METADATA -i /VLAN/Vlan1000/dhcpv6_servers -i /VLAN/Vlan1000/members
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "remove", "path": "/ACL_TABLE/DATAACL"}, {"op": "remove", "path": "/SYSLOG_SERVER"}]
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.
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
libyang[0]: Invalid JSON data (unexpected value). (path: /sonic-acl:sonic-acl/ACL_TABLE/ACL_TABLE_LIST[ACL_TABLE_NAME='DATAACL']/ports)
sonic_yang(3):Data Loading Failed:Invalid JSON data (unexpected value).
libyang[0]: Invalid JSON data (unexpected value). (path: /sonic-acl:sonic-acl/ACL_TABLE/ACL_TABLE_LIST[ACL_TABLE_NAME='DATAACL']/ports)
sonic_yang(3):Data Loading Failed:Invalid JSON data (unexpected value).
libyang[0]: Missing required element "type" in "ACL_TABLE_LIST". (path: /sonic-acl:sonic-acl/ACL_TABLE/ACL_TABLE_LIST[ACL_TABLE_NAME='DATAACL'])
sonic_yang(3):Data Loading Failed:Missing required element "type" in "ACL_TABLE_LIST".
libyang[0]: Missing required element "type" in "ACL_TABLE_LIST". (path: /sonic-acl:sonic-acl/ACL_TABLE/ACL_TABLE_LIST[ACL_TABLE_NAME='DATAACL'])
sonic_yang(3):Data Loading Failed:Missing required element "type" in "ACL_TABLE_LIST".
Patch Applier: The patch was sorted into 8 changes:
Patch Applier:   * [{"op": "remove", "path": "/SYSLOG_SERVER"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/policy_desc"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports/0"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports/0"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports/0"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/stage"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL"}]
Patch Applier: Applying 8 changes in order:
Patch Applier:   * [{"op": "remove", "path": "/SYSLOG_SERVER"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/policy_desc"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports/0"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports/0"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports/0"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/stage"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL/ports"}]
Patch Applier:   * [{"op": "remove", "path": "/ACL_TABLE/DATAACL"}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.
admin@vlab-01:~$

BONUS

We can ignore the validation for the whole config which practically mean skip directly to the change applier, can be useful for testing the change applier

admin@vlab-01:~$ sudo config apply-patch mix.json-patch -i ''
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "remove", "path": "/ACL_TABLE/DATAACL"}, {"op": "remove", "path": "/SYSLOG_SERVER"}]
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.
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, DSCP_TO_TC_MAP, FEATURE, KDUMP, MAP_PFC_PRIORITY_TO_QUEUE, PORT_QOS_MAP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TC_TO_PRIORITY_GROUP_MAP, TC_TO_QUEUE_MAP, TELEMETRY, WRED_PROFILE, 
Patch Applier: The patch was sorted into 1 change:
Patch Applier:   * [{"op": "remove", "path": "/SYSLOG_SERVER"}, {"op": "remove", "path": "/ACL_TABLE/DATAACL"}]
Patch Applier: Applying 1 change in order:
Patch Applier:   * [{"op": "remove", "path": "/SYSLOG_SERVER"}, {"op": "remove", "path": "/ACL_TABLE/DATAACL"}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.
admin@vlab-01:~$

[lgtm-com]

This pull request introduces 2 alerts when merging 9bc2e4951608cce16322500dba4725ad407d9f4c into a3e34e3 - view on LGTM.com

new alerts:

• 1 for Testing equality to None
• 1 for Unused local variable

[lgtm-com]

This pull request introduces 1 alert when merging ee45b235ff6747f523ecbcae3f6a6c53785cd22a into fdedcbf - view on LGTM.com

new alerts:

• 1 for Unused local variable

[lgtm-com]

This pull request introduces 1 alert when merging 85ac95f5bf54df6fa3311dfd10a6002d354b31bf into c05845d - view on LGTM.com

new alerts:

• 1 for Unused local variable

[lgtm-com]

This pull request introduces 1 alert when merging aec608985dbe966a9461ce9f330d856c1e8b2344 into c05845d - view on LGTM.com

new alerts:

• 1 for Unused local variable

[qiluo-msft]

ignore_more_list

From your implementation, I see there is a dependency between ignore_more_list and non_strict.

However I see they are independent. I feel there is a usage that all the yang models are available, but we need to ignore yang validation error with existing yang models. #Closed

[qiluo-msft]

This concern will further lead to my comment on command option long names.

[ghooo]

how do you suggest we make them independent?

non-strict: ignores validation for tables without YANG
ignore-more-...: ignores validation for more tables/fields besides the already ignored by going into the non-strict mode

[ghooo]

In the case of all yang models are available we have no problem using the -n mode which will do nothing since all tables have yang models already. Then we can use -i to ignore more, it is a little inconvenient though.

maybe we can have something like:

--non-yang-tables-allowed  -n
--ignore-from-yang-validation -i

These flags are independent but if any of them exist they activate the non-strict mode. What do you think?

[ghooo]

Another idea is:

--ignore-path -i    will ignore the give-path from yang validation
--ignore-all-non-yang-tables -n will ignore all tables without yang

they are also independent and if any of them is used, we use the non-strict mode

[ghooo]

Made the options independent.

The options now are:

--n --ignore-non-yang-tables        ignore validation for tables without YANG models
--i --ignore-path                   ignore validation for config specified by given path which is a JsonPointer

[qiluo-msft]

rollback

Could you use ctx.obj['db'] in the rollback function? this comment is applicable to other functions like replace.

[ghooo]

Why is this needed?

[qiluo-msft]

@ghooo proposed:
We can just add the config_db in its constructor of ChangeApplier, no need to change the Rollback and Replace APIs.