From 7ceccd750872d6dc99895f996033a7b611dff438 Mon Sep 17 00:00:00 2001 From: Mohamed Ghoneim Date: Wed, 8 Dec 2021 09:32:29 -0800 Subject: [PATCH] [generic-config-updater] Adding non-strict mode (#1929) #### 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. > :warning: **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. : 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. : 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. : 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 ```sh 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:~$ ``` --- config/main.py | 19 +- generic_config_updater/generic_updater.py | 72 ++-- generic_config_updater/gu_common.py | 12 + generic_config_updater/patch_sorter.py | 254 ++++++++++++- tests/config_test.py | 64 +++- .../generic_updater_test.py | 169 ++++++--- .../generic_config_updater/gu_common_test.py | 71 ++++ .../patch_sorter_test.py | 355 ++++++++++++++++++ 8 files changed, 893 insertions(+), 123 deletions(-) diff --git a/config/main.py b/config/main.py index 4faddb1c8bd9..45fd5525a1cb 100644 --- a/config/main.py +++ b/config/main.py @@ -1170,9 +1170,11 @@ def load(filename, yes): default=ConfigFormat.CONFIGDB.name, help='format of config of the patch is either ConfigDb(ABNF) or SonicYang') @click.option('-d', '--dry-run', is_flag=True, default=False, help='test out the command without affecting config state') +@click.option('-n', '--ignore-non-yang-tables', is_flag=True, default=False, help='ignore validation for tables without YANG models', hidden=True) +@click.option('-i', '--ignore-path', multiple=True, help='ignore validation for config specified by given path which is a JsonPointer', hidden=True) @click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing') @click.pass_context -def apply_patch(ctx, patch_file_path, format, dry_run, verbose): +def apply_patch(ctx, patch_file_path, format, dry_run, ignore_non_yang_tables, ignore_path, verbose): """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) @@ -1186,8 +1188,7 @@ def apply_patch(ctx, patch_file_path, format, dry_run, verbose): patch = jsonpatch.JsonPatch(patch_as_json) config_format = ConfigFormat[format.upper()] - - GenericUpdater().apply_patch(patch, config_format, verbose, dry_run) + GenericUpdater().apply_patch(patch, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path) click.secho("Patch applied successfully.", fg="cyan", underline=True) except Exception as ex: @@ -1200,9 +1201,11 @@ def apply_patch(ctx, patch_file_path, format, dry_run, verbose): default=ConfigFormat.CONFIGDB.name, help='format of target config is either ConfigDb(ABNF) or SonicYang') @click.option('-d', '--dry-run', is_flag=True, default=False, help='test out the command without affecting config state') +@click.option('-n', '--ignore-non-yang-tables', is_flag=True, default=False, help='ignore validation for tables without YANG models', hidden=True) +@click.option('-i', '--ignore-path', multiple=True, help='ignore validation for config specified by given path which is a JsonPointer', hidden=True) @click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing') @click.pass_context -def replace(ctx, target_file_path, format, dry_run, verbose): +def replace(ctx, target_file_path, format, dry_run, ignore_non_yang_tables, ignore_path, verbose): """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. @@ -1217,7 +1220,7 @@ def replace(ctx, target_file_path, format, dry_run, verbose): config_format = ConfigFormat[format.upper()] - GenericUpdater().replace(target_config, config_format, verbose, dry_run) + GenericUpdater().replace(target_config, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path) click.secho("Config replaced successfully.", fg="cyan", underline=True) except Exception as ex: @@ -1227,16 +1230,18 @@ def replace(ctx, target_file_path, format, dry_run, verbose): @config.command() @click.argument('checkpoint-name', type=str, required=True) @click.option('-d', '--dry-run', is_flag=True, default=False, help='test out the command without affecting config state') +@click.option('-n', '--ignore-non-yang-tables', is_flag=True, default=False, help='ignore validation for tables without YANG models', hidden=True) +@click.option('-i', '--ignore-path', multiple=True, help='ignore validation for config specified by given path which is a JsonPointer', hidden=True) @click.option('-v', '--verbose', is_flag=True, default=False, help='print additional details of what the operation is doing') @click.pass_context -def rollback(ctx, checkpoint_name, dry_run, verbose): +def rollback(ctx, checkpoint_name, dry_run, ignore_non_yang_tables, ignore_path, verbose): """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. : The checkpoint name, use `config list-checkpoints` command to see available checkpoints.""" try: - GenericUpdater().rollback(checkpoint_name, verbose, dry_run) + GenericUpdater().rollback(checkpoint_name, verbose, dry_run, ignore_non_yang_tables, ignore_path) click.secho("Config rolled back successfully.", fg="cyan", underline=True) except Exception as ex: diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index f4f1040db182..a4ea6f5ee6cf 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -3,7 +3,8 @@ from enum import Enum from .gu_common import GenericConfigUpdaterError, ConfigWrapper, \ DryRunConfigWrapper, PatchWrapper, genericUpdaterLogging -from .patch_sorter import PatchSorter +from .patch_sorter import StrictPatchSorter, NonStrictPatchSorter, ConfigSplitter, \ + TablesWithoutYangConfigSplitter, IgnorePathsFromYangConfigSplitter from .change_applier import ChangeApplier CHECKPOINTS_DIR = "/etc/sonic/checkpoints" @@ -32,18 +33,13 @@ def __init__(self, self.logger = genericUpdaterLogging.get_logger(title="Patch Applier", print_all_to_console=True) self.config_wrapper = config_wrapper if config_wrapper is not None else ConfigWrapper() self.patch_wrapper = patch_wrapper if patch_wrapper is not None else PatchWrapper() - self.patchsorter = patchsorter if patchsorter is not None else PatchSorter(self.config_wrapper, self.patch_wrapper) + self.patchsorter = patchsorter if patchsorter is not None else StrictPatchSorter(self.config_wrapper, self.patch_wrapper) self.changeapplier = changeapplier if changeapplier is not None else ChangeApplier() def apply(self, patch): self.logger.log_notice("Patch application starting.") self.logger.log_notice(f"Patch: {patch}") - # validate patch is only updating tables with yang models - self.logger.log_notice("Validating patch is not making changes to tables without YANG models.") - if not(self.patch_wrapper.validate_config_db_patch_has_yang_models(patch)): - raise ValueError(f"Given patch is not valid because it has changes to tables without YANG models") - # Get old config self.logger.log_notice("Getting current config db.") old_config = self.config_wrapper.get_config_db_as_json() @@ -62,11 +58,6 @@ def apply(self, patch): "which is not allowed in ConfigDb. " \ f"Table{'s' if len(empty_tables) != 1 else ''}: {empty_tables_txt}") - # Validate target config according to YANG models - self.logger.log_notice("Validating target config according to YANG models.") - if not(self.config_wrapper.validate_config_db_config(target_config)): - raise ValueError(f"Given patch is not valid because it will result in an invalid config") - # Generate list of changes to apply self.logger.log_notice("Sorting patch updates.") changes = self.patchsorter.sort(patch) @@ -102,10 +93,6 @@ def replace(self, target_config): self.logger.log_notice("Config replacement starting.") self.logger.log_notice(f"Target config length: {len(json.dumps(target_config))}.") - self.logger.log_notice("Validating target config according to YANG models.") - if not(self.config_wrapper.validate_config_db_config(target_config)): - raise ValueError(f"The given target config is not valid") - self.logger.log_notice("Getting current config db.") old_config = self.config_wrapper.get_config_db_as_json() @@ -156,11 +143,6 @@ def checkpoint(self, checkpoint_name): self.logger.log_notice("Getting current config db.") json_content = self.config_wrapper.get_config_db_as_json() - # if current config are not valid, we might not be able to rollback to it. So fail early by not taking checkpoint at all. - self.logger.log_notice("Validating current config according to YANG models.") - if not self.config_wrapper.validate_config_db_config(json_content): - raise ValueError(f"Running configs on the device are not valid.") - self.logger.log_notice("Getting checkpoint full-path.") path = self._get_checkpoint_full_path(checkpoint_name) @@ -314,14 +296,12 @@ def execute_write_action(self, action, *args): self.config_lock.release_lock() class GenericUpdateFactory: - def create_patch_applier(self, config_format, verbose, dry_run): + def create_patch_applier(self, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_paths): self.init_verbose_logging(verbose) - config_wrapper = self.get_config_wrapper(dry_run) - - patch_applier = PatchApplier(config_wrapper=config_wrapper) - patch_wrapper = PatchWrapper(config_wrapper) + patch_sorter = self.get_patch_sorter(ignore_non_yang_tables, ignore_paths, config_wrapper, patch_wrapper) + patch_applier = PatchApplier(config_wrapper=config_wrapper, patchsorter=patch_sorter, patch_wrapper=patch_wrapper) if config_format == ConfigFormat.CONFIGDB: pass @@ -336,14 +316,13 @@ def create_patch_applier(self, config_format, verbose, dry_run): return patch_applier - def create_config_replacer(self, config_format, verbose, dry_run): + def create_config_replacer(self, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_paths): self.init_verbose_logging(verbose) config_wrapper = self.get_config_wrapper(dry_run) - - patch_applier = PatchApplier(config_wrapper=config_wrapper) - patch_wrapper = PatchWrapper(config_wrapper) + patch_sorter = self.get_patch_sorter(ignore_non_yang_tables, ignore_paths, config_wrapper, patch_wrapper) + patch_applier = PatchApplier(config_wrapper=config_wrapper, patchsorter=patch_sorter, patch_wrapper=patch_wrapper) config_replacer = ConfigReplacer(patch_applier=patch_applier, config_wrapper=config_wrapper) if config_format == ConfigFormat.CONFIGDB: @@ -359,12 +338,14 @@ def create_config_replacer(self, config_format, verbose, dry_run): return config_replacer - def create_config_rollbacker(self, verbose, dry_run=False): + def create_config_rollbacker(self, verbose, dry_run=False, ignore_non_yang_tables=False, ignore_paths=[]): self.init_verbose_logging(verbose) config_wrapper = self.get_config_wrapper(dry_run) + patch_wrapper = PatchWrapper(config_wrapper) + patch_sorter = self.get_patch_sorter(ignore_non_yang_tables, ignore_paths, config_wrapper, patch_wrapper) + patch_applier = PatchApplier(config_wrapper=config_wrapper, patchsorter=patch_sorter, patch_wrapper=patch_wrapper) - patch_applier = PatchApplier(config_wrapper=config_wrapper) config_replacer = ConfigReplacer(config_wrapper=config_wrapper, patch_applier=patch_applier) config_rollbacker = FileSystemConfigRollbacker(config_wrapper = config_wrapper, config_replacer = config_replacer) @@ -382,21 +363,36 @@ def get_config_wrapper(self, dry_run): else: return ConfigWrapper() + def get_patch_sorter(self, ignore_non_yang_tables, ignore_paths, config_wrapper, patch_wrapper): + if not ignore_non_yang_tables and not ignore_paths: + return StrictPatchSorter(config_wrapper, patch_wrapper) + + inner_config_splitters = [] + if ignore_non_yang_tables: + inner_config_splitters.append(TablesWithoutYangConfigSplitter(config_wrapper)) + + if ignore_paths: + inner_config_splitters.append(IgnorePathsFromYangConfigSplitter(ignore_paths, config_wrapper)) + + config_splitter = ConfigSplitter(config_wrapper, inner_config_splitters) + + return NonStrictPatchSorter(config_wrapper, patch_wrapper, config_splitter) + class GenericUpdater: def __init__(self, generic_update_factory=None): self.generic_update_factory = \ generic_update_factory if generic_update_factory is not None else GenericUpdateFactory() - def apply_patch(self, patch, config_format, verbose, dry_run): - patch_applier = self.generic_update_factory.create_patch_applier(config_format, verbose, dry_run) + def apply_patch(self, patch, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_paths): + patch_applier = self.generic_update_factory.create_patch_applier(config_format, verbose, dry_run, ignore_non_yang_tables, ignore_paths) patch_applier.apply(patch) - def replace(self, target_config, config_format, verbose, dry_run): - config_replacer = self.generic_update_factory.create_config_replacer(config_format, verbose, dry_run) + def replace(self, target_config, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_paths): + config_replacer = self.generic_update_factory.create_config_replacer(config_format, verbose, dry_run, ignore_non_yang_tables, ignore_paths) config_replacer.replace(target_config) - def rollback(self, checkpoint_name, verbose, dry_run): - config_rollbacker = self.generic_update_factory.create_config_rollbacker(verbose, dry_run) + def rollback(self, checkpoint_name, verbose, dry_run, ignore_non_yang_tables, ignore_paths): + config_rollbacker = self.generic_update_factory.create_config_rollbacker(verbose, dry_run, ignore_non_yang_tables, ignore_paths) config_rollbacker.rollback(checkpoint_name) def checkpoint(self, checkpoint_name, verbose): diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 3e775f1bb260..ea4954ba248c 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -28,6 +28,9 @@ def __init__(self, patch): def apply(self, config): return self.patch.apply(config) + def __repr__(self): + return str(self) + def __str__(self): return f'{self.patch}' @@ -141,6 +144,12 @@ def get_empty_tables(self, config): empty_tables.append(key) return empty_tables + def remove_empty_tables(self, config): + config_with_non_empty_tables = {} + for table in config: + if config[table]: + config_with_non_empty_tables[table] = copy.deepcopy(config[table]) + return config_with_non_empty_tables class DryRunConfigWrapper(ConfigWrapper): # TODO: implement DryRunConfigWrapper @@ -236,6 +245,9 @@ def get_path_tokens(self, path): def create_path(self, tokens): return JsonPointer.from_parts(tokens).path + def has_path(self, doc, path): + return JsonPointer(path).get(doc, default=None) is not None + def get_xpath_tokens(self, xpath): """ Splits the given xpath into tokens by '/'. diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index f781576fab91..b53024e17380 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -3,7 +3,8 @@ import jsonpatch from collections import deque from enum import Enum -from .gu_common import OperationWrapper, OperationType, GenericConfigUpdaterError, JsonChange, PathAddressing +from .gu_common import OperationWrapper, OperationType, GenericConfigUpdaterError, \ + JsonChange, PathAddressing, genericUpdaterLogging class Diff: """ @@ -1014,6 +1015,253 @@ def create(self, algorithm=Algorithm.DFS): return sorter +class StrictPatchSorter: + def __init__(self, config_wrapper, patch_wrapper, inner_patch_sorter=None): + self.logger = genericUpdaterLogging.get_logger(title="Patch Sorter - Strict", print_all_to_console=True) + self.config_wrapper = config_wrapper + self.patch_wrapper = patch_wrapper + self.inner_patch_sorter = inner_patch_sorter if inner_patch_sorter else PatchSorter(config_wrapper, patch_wrapper) + + def sort(self, patch, algorithm=Algorithm.DFS): + current_config = self.config_wrapper.get_config_db_as_json() + + # Validate patch is only updating tables with yang models + self.logger.log_info("Validating patch is not making changes to tables without YANG models.") + if not(self.patch_wrapper.validate_config_db_patch_has_yang_models(patch)): + raise ValueError(f"Given patch is not valid because it has changes to tables without YANG models") + + target_config = self.patch_wrapper.simulate_patch(patch, current_config) + + # Validate target config + self.logger.log_info("Validating target config according to YANG models.") + if not(self.config_wrapper.validate_config_db_config(target_config)): + raise ValueError(f"Given patch is not valid because it will result in an invalid config") + + # Generate list of changes to apply + self.logger.log_info("Sorting patch updates.") + changes = self.inner_patch_sorter.sort(patch, algorithm) + + return changes + +class TablesWithoutYangConfigSplitter: + def __init__(self, config_wrapper): + self.config_wrapper = config_wrapper + + def split_yang_non_yang_distinct_field_path(self, config): + config_with_yang = self.config_wrapper.crop_tables_without_yang(config) + config_without_yang = {} + + for key in config: + if key not in config_with_yang: + config_without_yang[key] = copy.deepcopy(config[key]) + + return config_with_yang, config_without_yang + +class IgnorePathsFromYangConfigSplitter: + def __init__(self, ignore_paths_from_yang_list, config_wrapper): + self.ignore_paths_from_yang_list = ignore_paths_from_yang_list + self.config_wrapper = config_wrapper + + def split_yang_non_yang_distinct_field_path(self, config): + config_with_yang = copy.deepcopy(config) + config_without_yang = {} + + path_addressing = PathAddressing() + # ignore more config from config_with_yang + for path in self.ignore_paths_from_yang_list: + if not path_addressing.has_path(config_with_yang, path): + continue + if path == '': # whole config to be ignored + return {}, copy.deepcopy(config) + + # Add to config_without_yang from config_with_yang + tokens = path_addressing.get_path_tokens(path) + add_move = JsonMove(Diff(config_without_yang, config_with_yang), OperationType.ADD, tokens, tokens) + config_without_yang = add_move.apply(config_without_yang) + + # Remove from config_with_yang + remove_move = JsonMove(Diff(config_with_yang, {}), OperationType.REMOVE, tokens) + config_with_yang = remove_move.apply(config_with_yang) + + # Splitting the config based on 'ignore_paths_from_yang_list' can result in empty tables. + # Remove empty tables because they are not allowed in ConfigDb + config_with_yang_without_empty_tables = self.config_wrapper.remove_empty_tables(config_with_yang) + config_without_yang_without_empty_tables = self.config_wrapper.remove_empty_tables(config_without_yang) + return config_with_yang_without_empty_tables, config_without_yang_without_empty_tables + +class ConfigSplitter: + def __init__(self, config_wrapper, inner_config_splitters): + self.config_wrapper = config_wrapper + self.inner_config_splitters = inner_config_splitters + + def split_yang_non_yang_distinct_field_path(self, config): + empty_tables = self.config_wrapper.get_empty_tables(config) + empty_tables_txt = ", ".join(empty_tables) + if empty_tables: + raise ValueError(f"Given config has empty tables. Table{'s' if len(empty_tables) != 1 else ''}: {empty_tables_txt}") + + # Start by assuming all config should be YANG covered + config_with_yang = copy.deepcopy(config) + config_without_yang = {} + + for config_splitter in self.inner_config_splitters: + config_with_yang, additional_config_without_yang = config_splitter.split_yang_non_yang_distinct_field_path(config_with_yang) + config_without_yang = self.merge_configs_with_distinct_field_path(config_without_yang, additional_config_without_yang) + + return config_with_yang, config_without_yang + + def merge_configs_with_distinct_field_path(self, config1, config2): + merged_config = copy.deepcopy(config1) + self.__recursive_append(merged_config, config2) + return merged_config + + def __recursive_append(self, target, additional, path=""): + if not isinstance(target, dict): + raise ValueError(f"Found a field that exist in both config1 and config2. Path: {path}") + for key in additional: + if key not in target: + target[key] = copy.deepcopy(additional[key]) + else: + self.__recursive_append(target[key], additional[key], f"{path}/{key}") + +class ChangeWrapper: + def __init__(self, patch_wrapper, config_splitter): + self.patch_wrapper = patch_wrapper + self.config_splitter = config_splitter + + def adjust_changes(self, assumed_changes, assumed_curr_config, remaining_distinct_curr_config): + """ + The merging of 'assumed_curr_config' and 'remaining_distinct_curr_config' will generate the full config. + The list of 'assumed_changes' are applicable to 'assumed_curr_config' but they cannot be applied directly to the full config. + 'assumed_changes' can blindly alter existing config in 'remaining_distinct_curr_config' but they should not. Check example below. + + Example: + assumed_curr_config: + { + "ACL_TABLE": + { + "Everflow": { "type": "L3" } + } + } + + remaining_distinct_curr_config: + { + "ACL_TABLE": + { + "Everflow": { "policy_desc": "some-description" } + } + } + + assumed_changes (these are only applicable to assumed_curr_config): + { + [{"op":"replace", "path":"/ACL_TABLE/EVERFLOW", "value":{"type":"MIRROR"}}] + } + + The merging of assumed_curr_config and remaining_distinct_curr_config to get the full config is: + { + "ACL_TABLE": + { + "Everflow": { "type": "L3", "policy_desc": "some-description" } + } + } + + Applying changes to the merging i.e. full config will result in: + { + "ACL_TABLE": + { + "Everflow": { "type": "MIRROR" } + } + } + + This is not correct, as we have deleted /ACL_TABLE/EVERFLOW/policy_desc + This problem happend because we used 'assumed_changes' for 'assumed_curr_config' on the full config. + + The solution is to adjust the 'assumed_changes' list to be: + { + [{"op":"replace", "path":"/ACL_TABLE/EVERFLOW/type", "value":"MIRROR"}] + } + + This method adjust the given 'assumed_changes' to be applicable to the full config. + + Check unit-test for more examples. + """ + adjusted_changes = [] + assumed_curr_config = copy.deepcopy(assumed_curr_config) + for change in assumed_changes: + assumed_target_config = change.apply(assumed_curr_config) + + adjusted_curr_config = self.config_splitter.merge_configs_with_distinct_field_path(assumed_curr_config, remaining_distinct_curr_config) + adjusted_target_config = self.config_splitter.merge_configs_with_distinct_field_path(assumed_target_config, remaining_distinct_curr_config) + + adjusted_patch = self.patch_wrapper.generate_patch(adjusted_curr_config, adjusted_target_config) + + adjusted_change = JsonChange(adjusted_patch) + adjusted_changes.append(adjusted_change) + + assumed_curr_config = assumed_target_config + + return adjusted_changes + +class NonStrictPatchSorter: + def __init__(self, config_wrapper, patch_wrapper, config_splitter, change_wrapper=None, patch_sorter=None): + self.logger = genericUpdaterLogging.get_logger(title="Patch Sorter - Non-Strict", print_all_to_console=True) + self.config_wrapper = config_wrapper + self.patch_wrapper = patch_wrapper + self.config_splitter = config_splitter + self.change_wrapper = change_wrapper if change_wrapper else ChangeWrapper(patch_wrapper, config_splitter) + self.inner_patch_sorter = patch_sorter if patch_sorter else PatchSorter(config_wrapper, patch_wrapper) + + def sort(self, patch, algorithm=Algorithm.DFS): + current_config = self.config_wrapper.get_config_db_as_json() + target_config = self.patch_wrapper.simulate_patch(patch, current_config) + + # Splitting current/target config based on YANG covered vs non-YANG covered configs + self.logger.log_info("Splitting current/target config based on YANG covered vs non-YANG covered configs.") + current_config_yang, current_config_non_yang = self.config_splitter.split_yang_non_yang_distinct_field_path(current_config) + target_config_yang, target_config_non_yang = self.config_splitter.split_yang_non_yang_distinct_field_path(target_config) + + # Validate YANG covered target config + self.logger.log_info("Validating YANG covered target config according to YANG models.") + if not(self.config_wrapper.validate_config_db_config(target_config_yang)): + raise ValueError(f"Given patch is not valid because it will result in an invalid config") + + # Generating changes associated with non-YANG covered configs + self.logger.log_info("Sorting non-YANG covered configs patch updates.") + non_yang_patch = self.patch_wrapper.generate_patch(current_config_non_yang, target_config_non_yang) + non_yang_changes = [JsonChange(non_yang_patch)] if non_yang_patch else [] + changes_len = len(non_yang_changes) + self.logger.log_debug(f"The Non-YANG covered config update was sorted into {changes_len} " \ + f"change{'s' if changes_len != 1 else ''}{':' if changes_len > 0 else '.'}") + for change in non_yang_changes: + self.logger.log_debug(f" * {change}") + + # Regenerating patch for YANG covered configs + self.logger.log_info("Regenerating patch for YANG covered configs only.") + yang_patch = self.patch_wrapper.generate_patch(current_config_yang, target_config_yang) + self.logger.log_info(f"Generated patch {yang_patch}") + + # Validate YANG covered config patch is only updating tables with yang models + self.logger.log_info("Validating YANG covered config patch is not making changes to tables without YANG models.") + if not(self.patch_wrapper.validate_config_db_patch_has_yang_models(yang_patch)): + raise ValueError(f"Given YANG covered config patch is not valid because it has changes to tables without YANG models") + + # Generating changes associated with YANG covered configs + self.logger.log_info("Sorting YANG-covered configs patch updates.") + yang_changes = self.inner_patch_sorter.sort(yang_patch, algorithm, current_config_yang) + changes_len = len(yang_changes) + self.logger.log_debug(f"The YANG covered config update was sorted into {changes_len} " \ + f"change{'s' if changes_len != 1 else ''}{':' if changes_len > 0 else '.'}") + for change in yang_changes: + self.logger.log_debug(f" * {change}") + + # Merging non-YANG and YANG covered changes. + self.logger.log_info("Merging non-YANG and YANG covered changes.") + adjusted_non_yang_changes = self.change_wrapper.adjust_changes(non_yang_changes, current_config_non_yang, current_config_yang) + adjusted_yang_changes = self.change_wrapper.adjust_changes(yang_changes, current_config_yang, target_config_non_yang) + changes = adjusted_non_yang_changes + adjusted_yang_changes + + return changes + class PatchSorter: def __init__(self, config_wrapper, patch_wrapper, sort_algorithm_factory=None): self.config_wrapper = config_wrapper @@ -1023,8 +1271,8 @@ def __init__(self, config_wrapper, patch_wrapper, sort_algorithm_factory=None): self.sort_algorithm_factory = sort_algorithm_factory if sort_algorithm_factory else \ SortAlgorithmFactory(self.operation_wrapper, config_wrapper, self.path_addressing) - def sort(self, patch, algorithm=Algorithm.DFS): - current_config = self.config_wrapper.get_config_db_as_json() + def sort(self, patch, algorithm=Algorithm.DFS, preloaded_current_config=None): + current_config = preloaded_current_config if preloaded_current_config else self.config_wrapper.get_config_db_as_json() target_config = self.patch_wrapper.simulate_patch(patch, current_config) cropped_current_config = self.config_wrapper.crop_tables_without_yang(current_config) diff --git a/tests/config_test.py b/tests/config_test.py index 28cea8c519b2..873f9889b44d 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -474,7 +474,7 @@ def test_apply_patch__only_required_params__default_values_used_for_optional_par # Arrange expected_exit_code = 0 expected_output = "Patch applied successfully" - expected_call_with_default_values = mock.call(self.any_patch, ConfigFormat.CONFIGDB, False, False) + expected_call_with_default_values = mock.call(self.any_patch, ConfigFormat.CONFIGDB, False, False, False, ()) mock_generic_updater = mock.Mock() with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_patch_as_text)): @@ -492,7 +492,9 @@ def test_apply_patch__all_optional_params_non_default__non_default_values_used(s # Arrange expected_exit_code = 0 expected_output = "Patch applied successfully" - expected_call_with_non_default_values = mock.call(self.any_patch, ConfigFormat.SONICYANG, True, True) + expected_ignore_path_tuple = ('/ANY_TABLE', '/ANY_OTHER_TABLE/ANY_FIELD', '') + expected_call_with_non_default_values = \ + mock.call(self.any_patch, ConfigFormat.SONICYANG, True, True, True, expected_ignore_path_tuple) mock_generic_updater = mock.Mock() with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_patch_as_text)): @@ -502,6 +504,10 @@ def test_apply_patch__all_optional_params_non_default__non_default_values_used(s [self.any_path, "--format", ConfigFormat.SONICYANG.name, "--dry-run", + "--ignore-non-yang-tables", + "--ignore-path", "/ANY_TABLE", + "--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD", + "--ignore-path", "", "--verbose"], catch_exceptions=False) @@ -532,13 +538,19 @@ def test_apply_patch__exception_thrown__error_displayed_error_code_returned(self def test_apply_patch__optional_parameters_passed_correctly(self): self.validate_apply_patch_optional_parameter( ["--format", ConfigFormat.SONICYANG.name], - mock.call(self.any_patch, ConfigFormat.SONICYANG, False, False)) + mock.call(self.any_patch, ConfigFormat.SONICYANG, False, False, False, ())) self.validate_apply_patch_optional_parameter( ["--verbose"], - mock.call(self.any_patch, ConfigFormat.CONFIGDB, True, False)) + mock.call(self.any_patch, ConfigFormat.CONFIGDB, True, False, False, ())) self.validate_apply_patch_optional_parameter( ["--dry-run"], - mock.call(self.any_patch, ConfigFormat.CONFIGDB, False, True)) + mock.call(self.any_patch, ConfigFormat.CONFIGDB, False, True, False, ())) + self.validate_apply_patch_optional_parameter( + ["--ignore-non-yang-tables"], + mock.call(self.any_patch, ConfigFormat.CONFIGDB, False, False, True, ())) + self.validate_apply_patch_optional_parameter( + ["--ignore-path", "/ANY_TABLE"], + mock.call(self.any_patch, ConfigFormat.CONFIGDB, False, False, False, ("/ANY_TABLE",))) def validate_apply_patch_optional_parameter(self, param_args, expected_call): # Arrange @@ -587,7 +599,7 @@ def test_replace__only_required_params__default_values_used_for_optional_params( # Arrange expected_exit_code = 0 expected_output = "Config replaced successfully" - expected_call_with_default_values = mock.call(self.any_target_config, ConfigFormat.CONFIGDB, False, False) + expected_call_with_default_values = mock.call(self.any_target_config, ConfigFormat.CONFIGDB, False, False, False, ()) mock_generic_updater = mock.Mock() with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_target_config_as_text)): @@ -605,7 +617,9 @@ def test_replace__all_optional_params_non_default__non_default_values_used(self) # Arrange expected_exit_code = 0 expected_output = "Config replaced successfully" - expected_call_with_non_default_values = mock.call(self.any_target_config, ConfigFormat.SONICYANG, True, True) + expected_ignore_path_tuple = ('/ANY_TABLE', '/ANY_OTHER_TABLE/ANY_FIELD', '') + expected_call_with_non_default_values = \ + mock.call(self.any_target_config, ConfigFormat.SONICYANG, True, True, True, expected_ignore_path_tuple) mock_generic_updater = mock.Mock() with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_target_config_as_text)): @@ -615,6 +629,10 @@ def test_replace__all_optional_params_non_default__non_default_values_used(self) [self.any_path, "--format", ConfigFormat.SONICYANG.name, "--dry-run", + "--ignore-non-yang-tables", + "--ignore-path", "/ANY_TABLE", + "--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD", + "--ignore-path", "", "--verbose"], catch_exceptions=False) @@ -645,13 +663,19 @@ def test_replace__exception_thrown__error_displayed_error_code_returned(self): def test_replace__optional_parameters_passed_correctly(self): self.validate_replace_optional_parameter( ["--format", ConfigFormat.SONICYANG.name], - mock.call(self.any_target_config, ConfigFormat.SONICYANG, False, False)) + mock.call(self.any_target_config, ConfigFormat.SONICYANG, False, False, False, ())) self.validate_replace_optional_parameter( ["--verbose"], - mock.call(self.any_target_config, ConfigFormat.CONFIGDB, True, False)) + mock.call(self.any_target_config, ConfigFormat.CONFIGDB, True, False, False, ())) self.validate_replace_optional_parameter( ["--dry-run"], - mock.call(self.any_target_config, ConfigFormat.CONFIGDB, False, True)) + mock.call(self.any_target_config, ConfigFormat.CONFIGDB, False, True, False, ())) + self.validate_replace_optional_parameter( + ["--ignore-non-yang-tables"], + mock.call(self.any_target_config, ConfigFormat.CONFIGDB, False, False, True, ())) + self.validate_replace_optional_parameter( + ["--ignore-path", "/ANY_TABLE"], + mock.call(self.any_target_config, ConfigFormat.CONFIGDB, False, False, False, ("/ANY_TABLE",))) def validate_replace_optional_parameter(self, param_args, expected_call): # Arrange @@ -700,7 +724,7 @@ def test_rollback__only_required_params__default_values_used_for_optional_params # Arrange expected_exit_code = 0 expected_output = "Config rolled back successfully" - expected_call_with_default_values = mock.call(self.any_checkpoint_name, False, False) + expected_call_with_default_values = mock.call(self.any_checkpoint_name, False, False, False, ()) mock_generic_updater = mock.Mock() with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): # Act @@ -716,7 +740,9 @@ def test_rollback__all_optional_params_non_default__non_default_values_used(self # Arrange expected_exit_code = 0 expected_output = "Config rolled back successfully" - expected_call_with_non_default_values = mock.call(self.any_checkpoint_name, True, True) + expected_ignore_path_tuple = ('/ANY_TABLE', '/ANY_OTHER_TABLE/ANY_FIELD', '') + expected_call_with_non_default_values = \ + mock.call(self.any_checkpoint_name, True, True, True, expected_ignore_path_tuple) mock_generic_updater = mock.Mock() with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): @@ -724,6 +750,10 @@ def test_rollback__all_optional_params_non_default__non_default_values_used(self result = self.runner.invoke(config.config.commands["rollback"], [self.any_checkpoint_name, "--dry-run", + "--ignore-non-yang-tables", + "--ignore-path", "/ANY_TABLE", + "--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD", + "--ignore-path", "", "--verbose"], catch_exceptions=False) @@ -753,10 +783,16 @@ def test_rollback__exception_thrown__error_displayed_error_code_returned(self): def test_rollback__optional_parameters_passed_correctly(self): self.validate_rollback_optional_parameter( ["--verbose"], - mock.call(self.any_checkpoint_name, True, False)) + mock.call(self.any_checkpoint_name, True, False, False, ())) self.validate_rollback_optional_parameter( ["--dry-run"], - mock.call(self.any_checkpoint_name, False, True)) + mock.call(self.any_checkpoint_name, False, True, False, ())) + self.validate_rollback_optional_parameter( + ["--ignore-non-yang-tables"], + mock.call(self.any_checkpoint_name, False, False, True, ())) + self.validate_rollback_optional_parameter( + ["--ignore-path", "/ACL_TABLE"], + mock.call(self.any_checkpoint_name, False, False, False, ("/ACL_TABLE",))) def validate_rollback_optional_parameter(self, param_args, expected_call): # Arrange diff --git a/tests/generic_config_updater/generic_updater_test.py b/tests/generic_config_updater/generic_updater_test.py index ef6a2192430f..1a8151f398d3 100644 --- a/tests/generic_config_updater/generic_updater_test.py +++ b/tests/generic_config_updater/generic_updater_test.py @@ -6,19 +6,13 @@ from .gutest_helpers import create_side_effect_dict, Files import generic_config_updater.generic_updater as gu +import generic_config_updater.patch_sorter as ps # import sys # sys.path.insert(0,'../../generic_config_updater') # import generic_updater as gu class TestPatchApplier(unittest.TestCase): - def test_apply__invalid_patch_updating_tables_without_yang_models__failure(self): - # Arrange - patch_applier = self.__create_patch_applier(valid_patch_only_tables_with_yang_models=False) - - # Act and assert - self.assertRaises(ValueError, patch_applier.apply, Files.MULTI_OPERATION_CONFIG_DB_PATCH) - def test_apply__invalid_patch_producing_empty_tables__failure(self): # Arrange patch_applier = self.__create_patch_applier(valid_patch_does_not_produce_empty_tables=False) @@ -26,13 +20,6 @@ def test_apply__invalid_patch_producing_empty_tables__failure(self): # Act and assert self.assertRaises(ValueError, patch_applier.apply, Files.MULTI_OPERATION_CONFIG_DB_PATCH) - def test_apply__invalid_config_db__failure(self): - # Arrange - patch_applier = self.__create_patch_applier(valid_config_db=False) - - # Act and assert - self.assertRaises(ValueError, patch_applier.apply, Files.MULTI_OPERATION_CONFIG_DB_PATCH) - def test_apply__json_not_fully_updated__failure(self): # Arrange patch_applier = self.__create_patch_applier(verified_same_config=False) @@ -49,13 +36,9 @@ def test_apply__no_errors__update_successful(self): patch_applier.apply(Files.MULTI_OPERATION_CONFIG_DB_PATCH) # Assert - patch_applier.patch_wrapper.validate_config_db_patch_has_yang_models.assert_has_calls( - [call(Files.MULTI_OPERATION_CONFIG_DB_PATCH)]) patch_applier.config_wrapper.get_config_db_as_json.assert_has_calls([call(), call()]) patch_applier.patch_wrapper.simulate_patch.assert_has_calls( [call(Files.MULTI_OPERATION_CONFIG_DB_PATCH, Files.CONFIG_DB_AS_JSON)]) - patch_applier.config_wrapper.validate_config_db_config.assert_has_calls( - [call(Files.CONFIG_DB_AFTER_MULTI_PATCH)]) patch_applier.patchsorter.sort.assert_has_calls([call(Files.MULTI_OPERATION_CONFIG_DB_PATCH)]) patch_applier.changeapplier.apply.assert_has_calls([call(changes[0]), call(changes[1])]) patch_applier.patch_wrapper.verify_same_json.assert_has_calls( @@ -63,23 +46,16 @@ def test_apply__no_errors__update_successful(self): def __create_patch_applier(self, changes=None, - valid_patch_only_tables_with_yang_models=True, - valid_config_db=True, valid_patch_does_not_produce_empty_tables=True, verified_same_config=True): config_wrapper = Mock() config_wrapper.get_config_db_as_json.side_effect = \ [Files.CONFIG_DB_AS_JSON, Files.CONFIG_DB_AFTER_MULTI_PATCH] - config_wrapper.validate_config_db_config.side_effect = \ - create_side_effect_dict({(str(Files.CONFIG_DB_AFTER_MULTI_PATCH),): valid_config_db}) empty_tables = [] if valid_patch_does_not_produce_empty_tables else ["AnyTable"] config_wrapper.get_empty_tables.side_effect = \ create_side_effect_dict({(str(Files.CONFIG_DB_AFTER_MULTI_PATCH),): empty_tables}) patch_wrapper = Mock() - patch_wrapper.validate_config_db_patch_has_yang_models.side_effect = \ - create_side_effect_dict( - {(str(Files.MULTI_OPERATION_CONFIG_DB_PATCH),): valid_patch_only_tables_with_yang_models}) patch_wrapper.simulate_patch.side_effect = \ create_side_effect_dict( {(str(Files.MULTI_OPERATION_CONFIG_DB_PATCH), str(Files.CONFIG_DB_AS_JSON)): @@ -100,13 +76,6 @@ def __create_patch_applier(self, return gu.PatchApplier(patchsorter, changeapplier, config_wrapper, patch_wrapper) class TestConfigReplacer(unittest.TestCase): - def test_replace__invalid_config_db__failure(self): - # Arrange - config_replacer = self.__create_config_replacer(valid_config_db=False) - - # Act and assert - self.assertRaises(ValueError, config_replacer.replace, Files.CONFIG_DB_AFTER_MULTI_PATCH) - def test_replace__json_not_fully_updated__failure(self): # Arrange config_replacer = self.__create_config_replacer(verified_same_config=False) @@ -122,8 +91,6 @@ def test_replace__no_errors__update_successful(self): config_replacer.replace(Files.CONFIG_DB_AFTER_MULTI_PATCH) # Assert - config_replacer.config_wrapper.validate_config_db_config.assert_has_calls( - [call(Files.CONFIG_DB_AFTER_MULTI_PATCH)]) config_replacer.config_wrapper.get_config_db_as_json.assert_has_calls([call(), call()]) config_replacer.patch_wrapper.generate_patch.assert_has_calls( [call(Files.CONFIG_DB_AS_JSON, Files.CONFIG_DB_AFTER_MULTI_PATCH)]) @@ -131,10 +98,8 @@ def test_replace__no_errors__update_successful(self): config_replacer.patch_wrapper.verify_same_json.assert_has_calls( [call(Files.CONFIG_DB_AFTER_MULTI_PATCH, Files.CONFIG_DB_AFTER_MULTI_PATCH)]) - def __create_config_replacer(self, changes=None, valid_config_db=True, verified_same_config=True): + def __create_config_replacer(self, changes=None, verified_same_config=True): config_wrapper = Mock() - config_wrapper.validate_config_db_config.side_effect = \ - create_side_effect_dict({(str(Files.CONFIG_DB_AFTER_MULTI_PATCH),): valid_config_db}) config_wrapper.get_config_db_as_json.side_effect = \ [Files.CONFIG_DB_AS_JSON, Files.CONFIG_DB_AFTER_MULTI_PATCH] @@ -201,13 +166,6 @@ def test_checkpoint__checkpoints_dir_does_not_exist__checkpoint_created(self): self.assertTrue(os.path.isdir(self.checkpoints_dir)) self.assertEqual(self.any_config, self.get_checkpoint(self.any_checkpoint_name)) - def test_checkpoint__config_not_valid__failure(self): - # Arrange - rollbacker = self.create_rollbacker(valid_config=False) - - # Act and assert - self.assertRaises(ValueError, rollbacker.checkpoint, self.any_checkpoint_name) - def test_checkpoint__checkpoints_dir_exists__checkpoint_created(self): # Arrange self.create_checkpoints_dir() @@ -340,13 +298,12 @@ def check_checkpoint_exists(self, name): path=os.path.join(self.checkpoints_dir, f"{name}{self.checkpoint_ext}") return os.path.isfile(path) - def create_rollbacker(self, valid_config=True): + def create_rollbacker(self): replacer = Mock() replacer.replace.side_effect = create_side_effect_dict({(str(self.any_config),): 0}) config_wrapper = Mock() config_wrapper.get_config_db_as_json.return_value = self.any_config - config_wrapper.validate_config_db_config.return_value = valid_config return gu.FileSystemConfigRollbacker(checkpoints_dir=self.checkpoints_dir, config_replacer=replacer, @@ -356,14 +313,21 @@ class TestGenericUpdateFactory(unittest.TestCase): def setUp(self): self.any_verbose=True self.any_dry_run=True + self.any_ignore_non_yang_tables=True + self.any_ignore_paths=[""] def test_create_patch_applier__invalid_config_format__failure(self): # Arrange factory = gu.GenericUpdateFactory() # Act and assert - self.assertRaises( - ValueError, factory.create_patch_applier, "INVALID_FORMAT", self.any_verbose, self.any_dry_run) + self.assertRaises(ValueError, + factory.create_patch_applier, + "INVALID_FORMAT", + self.any_verbose, + self.any_dry_run, + self.any_ignore_non_yang_tables, + self.any_ignore_paths) def test_create_patch_applier__different_options(self): # Arrange @@ -376,6 +340,8 @@ def test_create_patch_applier__different_options(self): gu.ConfigFormat.CONFIGDB: None, } }, + {"ignore_non_yang_tables": {True: None, False: None}}, + {"ignore_paths": {(): None, ("", "/ACL_TABLE"): None}}, ] # Act and assert @@ -386,8 +352,13 @@ def test_create_config_replacer__invalid_config_format__failure(self): factory = gu.GenericUpdateFactory() # Act and assert - self.assertRaises( - ValueError, factory.create_config_replacer, "INVALID_FORMAT", self.any_verbose, self.any_dry_run) + self.assertRaises(ValueError, + factory.create_config_replacer, + "INVALID_FORMAT", + self.any_verbose, + self.any_dry_run, + self.any_ignore_non_yang_tables, + self.any_ignore_paths) def test_create_config_replacer__different_options(self): # Arrange @@ -400,6 +371,8 @@ def test_create_config_replacer__different_options(self): gu.ConfigFormat.CONFIGDB: None, } }, + {"ignore_non_yang_tables": {True: None, False: None}}, + {"ignore_paths": {(): None, ("", "/ACL_TABLE"): None}}, ] # Act and assert @@ -409,7 +382,9 @@ def test_create_config_rollbacker__different_options(self): # Arrange options = [ {"verbose": {True: None, False: None}}, - {"dry_run": {True: None, False: gu.ConfigLockDecorator}} + {"dry_run": {True: None, False: gu.ConfigLockDecorator}}, + {"ignore_non_yang_tables": {True: None, False: None}}, + {"ignore_paths": {(): None, ("", "/ACL_TABLE"): None}}, ] # Act and assert @@ -432,7 +407,11 @@ def recursively_test_create_func(self, options, cur_option, params, expected_dec def validate_create_patch_applier(self, params, expected_decorators): factory = gu.GenericUpdateFactory() - patch_applier = factory.create_patch_applier(params["config_format"], params["verbose"], params["dry_run"]) + patch_applier = factory.create_patch_applier(params["config_format"], + params["verbose"], + params["dry_run"], + params["ignore_non_yang_tables"], + params["ignore_paths"]) for decorator_type in expected_decorators: self.assertIsInstance(patch_applier, decorator_type) @@ -444,9 +423,25 @@ def validate_create_patch_applier(self, params, expected_decorators): else: self.assertIsInstance(patch_applier.config_wrapper, gu.ConfigWrapper) + if params["ignore_non_yang_tables"] or params["ignore_paths"]: + self.assertIsInstance(patch_applier.patchsorter, ps.NonStrictPatchSorter) + expected_config_splitters = [] + if params["ignore_non_yang_tables"]: + expected_config_splitters.append(ps.TablesWithoutYangConfigSplitter.__name__) + if params["ignore_paths"]: + expected_config_splitters.append(ps.IgnorePathsFromYangConfigSplitter.__name__) + actual_config_splitters = [type(splitter).__name__ for splitter in patch_applier.patchsorter.config_splitter.inner_config_splitters] + self.assertCountEqual(expected_config_splitters, actual_config_splitters) + else: + self.assertIsInstance(patch_applier.patchsorter, ps.StrictPatchSorter) + def validate_create_config_replacer(self, params, expected_decorators): factory = gu.GenericUpdateFactory() - config_replacer = factory.create_config_replacer(params["config_format"], params["verbose"], params["dry_run"]) + config_replacer = factory.create_config_replacer(params["config_format"], + params["verbose"], + params["dry_run"], + params["ignore_non_yang_tables"], + params["ignore_paths"]) for decorator_type in expected_decorators: self.assertIsInstance(config_replacer, decorator_type) @@ -460,9 +455,22 @@ def validate_create_config_replacer(self, params, expected_decorators): self.assertIsInstance(config_replacer.config_wrapper, gu.ConfigWrapper) self.assertIsInstance(config_replacer.patch_applier.config_wrapper, gu.ConfigWrapper) + if params["ignore_non_yang_tables"] or params["ignore_paths"]: + self.assertIsInstance(config_replacer.patch_applier.patchsorter, ps.NonStrictPatchSorter) + expected_config_splitters = [] + if params["ignore_non_yang_tables"]: + expected_config_splitters.append(ps.TablesWithoutYangConfigSplitter.__name__) + if params["ignore_paths"]: + expected_config_splitters.append(ps.IgnorePathsFromYangConfigSplitter.__name__) + actual_config_splitters = [type(splitter).__name__ for splitter in + config_replacer.patch_applier.patchsorter.config_splitter.inner_config_splitters] + self.assertCountEqual(expected_config_splitters, actual_config_splitters) + else: + self.assertIsInstance(config_replacer.patch_applier.patchsorter, ps.StrictPatchSorter) + def validate_create_config_rollbacker(self, params, expected_decorators): factory = gu.GenericUpdateFactory() - config_rollbacker = factory.create_config_rollbacker(params["verbose"], params["dry_run"]) + config_rollbacker = factory.create_config_rollbacker(params["verbose"], params["dry_run"], params["ignore_non_yang_tables"], params["ignore_paths"]) for decorator_type in expected_decorators: self.assertIsInstance(config_rollbacker, decorator_type) @@ -480,6 +488,19 @@ def validate_create_config_rollbacker(self, params, expected_decorators): self.assertIsInstance( config_rollbacker.config_replacer.patch_applier.config_wrapper, gu.ConfigWrapper) + if params["ignore_non_yang_tables"] or params["ignore_paths"]: + self.assertIsInstance(config_rollbacker.config_replacer.patch_applier.patchsorter, ps.NonStrictPatchSorter) + expected_config_splitters = [] + if params["ignore_non_yang_tables"]: + expected_config_splitters.append(ps.TablesWithoutYangConfigSplitter.__name__) + if params["ignore_paths"]: + expected_config_splitters.append(ps.IgnorePathsFromYangConfigSplitter.__name__) + actual_config_splitters = [type(splitter).__name__ for splitter in + config_rollbacker.config_replacer.patch_applier.patchsorter.config_splitter.inner_config_splitters] + self.assertCountEqual(expected_config_splitters, actual_config_splitters) + else: + self.assertIsInstance(config_rollbacker.config_replacer.patch_applier.patchsorter, ps.StrictPatchSorter) + class TestGenericUpdater(unittest.TestCase): def setUp(self): self.any_checkpoint_name = "anycheckpoint" @@ -488,6 +509,8 @@ def setUp(self): self.any_config_format = gu.ConfigFormat.SONICYANG self.any_verbose = True self.any_dry_run = True + self.any_ignore_non_yang_tables = True + self.any_ignore_paths = ["", "/ACL_TABLE"] def test_apply_patch__creates_applier_and_apply(self): # Arrange @@ -497,13 +520,21 @@ def test_apply_patch__creates_applier_and_apply(self): factory = Mock() factory.create_patch_applier.side_effect = \ create_side_effect_dict( - {(str(self.any_config_format), str(self.any_verbose), str(self.any_dry_run),): patch_applier}) + {(str(self.any_config_format), + str(self.any_verbose), + str(self.any_dry_run), + str(self.any_ignore_non_yang_tables), + str(self.any_ignore_paths)): patch_applier}) generic_updater = gu.GenericUpdater(factory) # Act - generic_updater.apply_patch( - Files.SINGLE_OPERATION_SONIC_YANG_PATCH, self.any_config_format, self.any_verbose, self.any_dry_run) + generic_updater.apply_patch(Files.SINGLE_OPERATION_SONIC_YANG_PATCH, + self.any_config_format, + self.any_verbose, + self.any_dry_run, + self.any_ignore_non_yang_tables, + self.any_ignore_paths) # Assert patch_applier.apply.assert_has_calls([call(Files.SINGLE_OPERATION_SONIC_YANG_PATCH)]) @@ -516,12 +547,21 @@ def test_replace__creates_replacer_and_replace(self): factory = Mock() factory.create_config_replacer.side_effect = \ create_side_effect_dict( - {(str(self.any_config_format), str(self.any_verbose), str(self.any_dry_run),): config_replacer}) + {(str(self.any_config_format), + str(self.any_verbose), + str(self.any_dry_run), + str(self.any_ignore_non_yang_tables), + str(self.any_ignore_paths)): config_replacer}) generic_updater = gu.GenericUpdater(factory) # Act - generic_updater.replace(Files.SONIC_YANG_AS_JSON, self.any_config_format, self.any_verbose, self.any_dry_run) + generic_updater.replace(Files.SONIC_YANG_AS_JSON, + self.any_config_format, + self.any_verbose, + self.any_dry_run, + self.any_ignore_non_yang_tables, + self.any_ignore_paths) # Assert config_replacer.replace.assert_has_calls([call(Files.SONIC_YANG_AS_JSON)]) @@ -533,12 +573,19 @@ def test_rollback__creates_rollbacker_and_rollback(self): factory = Mock() factory.create_config_rollbacker.side_effect = \ - create_side_effect_dict({(str(self.any_verbose), str(self.any_dry_run),): config_rollbacker}) + create_side_effect_dict({(str(self.any_verbose), + str(self.any_dry_run), + str(self.any_ignore_non_yang_tables), + str(self.any_ignore_paths)): config_rollbacker}) generic_updater = gu.GenericUpdater(factory) # Act - generic_updater.rollback(self.any_checkpoint_name, self.any_verbose, self.any_dry_run) + generic_updater.rollback(self.any_checkpoint_name, + self.any_verbose, + self.any_dry_run, + self.any_ignore_non_yang_tables, + self.any_ignore_paths) # Assert config_rollbacker.rollback.assert_has_calls([call(self.any_checkpoint_name)]) diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index 842e71baaa31..56cebe786b94 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -181,6 +181,39 @@ def test_get_empty_tables__multiple_empty_tables__returns_multiple_tables(self): # Assert self.assertCountEqual(["another_table", "yet_another_table"], empty_tables) + def test_remove_empty_tables__no_empty_tables__returns_whole_config(self): + # Arrange + config_wrapper = gu_common.ConfigWrapper() + config = {"any_table": {"key": "value"}} + + # Act + actual = config_wrapper.remove_empty_tables(config) + + # Assert + self.assertDictEqual({"any_table": {"key": "value"}}, actual) + + def test_remove_empty_tables__single_empty_tables__returns_config_without_empty_table(self): + # Arrange + config_wrapper = gu_common.ConfigWrapper() + config = {"any_table": {"key": "value"}, "another_table":{}} + + # Act + actual = config_wrapper.remove_empty_tables(config) + + # Assert + self.assertDictEqual({"any_table": {"key": "value"}}, actual) + + def test_remove_empty_tables__multiple_empty_tables__returns_config_without_empty_tables(self): + # Arrange + config_wrapper = gu_common.ConfigWrapper() + config = {"any_table": {"key": "value"}, "another_table":{}, "yet_another_table":{}} + + # Act + actual = config_wrapper.remove_empty_tables(config) + + # Assert + self.assertDictEqual({"any_table": {"key": "value"}}, actual) + class TestPatchWrapper(unittest.TestCase): def setUp(self): self.config_wrapper_mock = gu_common.ConfigWrapper() @@ -666,3 +699,41 @@ def check(xpath, path, config=None): path="/PORTCHANNEL_INTERFACE/PortChannel0001|1.1.1.1~124", config=Files.CONFIG_DB_WITH_PORTCHANNEL_INTERFACE) + def test_has_path(self): + def check(config, path, expected): + actual=self.path_addressing.has_path(config, path) + self.assertEqual(expected, actual) + + check(config={}, + path="", + expected=True) + check(config={"TABLE":{}}, + path="", + expected=True) + check(config={}, + path="/TABLE", + expected=False) + check(config={"TABLE":{}}, + path="/ANOTHER_TABLE", + expected=False) + check(config={"TABLE":{}}, + path="/ANOTHER_TABLE", + expected=False) + check(config={"TABLE":{"key1":{"key11":{"key111":"value111"}}}}, + path="/TABLE/key1/key11/key111", + expected=True) + check(config={"TABLE":{"key1":{"key11":{"key111":"value111"}}}}, + path="/TABLE/key1", + expected=True) + check(config={"TABLE":{"key1":{"key11":{"key111":"value111"}}}}, + path="/TABLE/key1/key1", + expected=False) + check(config={"ANOTHER_TABLE": {}, "TABLE":{"key1":{"key11":{"key111":"value111"}}}}, + path="/TABLE/key1/key11", + expected=True) + check(config={"ANOTHER_TABLE": {}, "TABLE":{"key1":{"key11":{"key111":[1,2,3,4,5]}}}}, + path="/TABLE/key1/key11/key111/4", + expected=True) + check(config={"ANOTHER_TABLE": {}, "TABLE":{"key1":{"key11":{"key111":[1,2,3,4,5]}}}}, + path="/TABLE/key1/key11/key111/5", + expected=False) diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index c51733abe72b..b1764a8dfc68 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -1865,3 +1865,358 @@ def verify(self, cc_ops=[], tc_ops=[]): simulated_config = move.apply(simulated_config) self.assertTrue(config_wrapper.validate_config_db_config(simulated_config)) self.assertEqual(target_config, simulated_config) + +class TestChangeWrapper(unittest.TestCase): + def setUp(self): + config_splitter = ps.ConfigSplitter(ConfigWrapper(), []) + self.wrapper = ps.ChangeWrapper(PatchWrapper(), config_splitter) + + def test_adjust_changes(self): + def check(changes, assumed, remaining, expected): + actual = self.wrapper.adjust_changes(changes, assumed, remaining) + self.assertEqual(len(expected), len(actual)) + + for idx in range(len(expected)): + self.assertCountEqual(expected[idx].patch, actual[idx].patch, f"JsonChange idx {idx} did not match") + + check([], {}, {}, []) + # Add table to empty config + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE1", "value":{}}]))], + assumed={}, + remaining={}, + expected=[JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE1", "value":{}}]))]) + # Add table, while tables exist in assumed and remaining + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3", "value":{}}]))], + assumed={"TABLE1":{}}, + remaining={"TABLE2":{}}, + expected=[JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3", "value":{}}]))]) + # Add table with single field, while table has multiple fields in remaining + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3", "value":{"key3":"value3"}}]))], + assumed={"TABLE1":{}}, + remaining={"TABLE2":{}, "TABLE3":{"key1":"value1", "key2":"value2"}}, + expected=[JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3/key3", "value":"value3"}]))]) + # Remove table to empty the config + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE1"}]))], + assumed={"TABLE1":{}}, + remaining={}, + expected=[JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE1"}]))]) + # Remove table, while other tables exist in assumed and remaining + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE3"}]))], + assumed={"TABLE1":{}, "TABLE3":{}}, + remaining={"TABLE2":{}}, + expected=[JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE3"}]))]) + # Remove table with single field, while table has multiple fields in remaining + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE3"}]))], + assumed={"TABLE1":{}, "TABLE3":{"key3":"value3"}}, + remaining={"TABLE2":{}, "TABLE3":{"key1":"value1", "key2":"value2"}}, + expected=[JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE3/key3"}]))]) + # Change that does nothing + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"replace", "path":"/TABLE1", "value":{}}]))], + assumed={"TABLE1":{}}, + remaining={}, + expected=[JsonChange(jsonpatch.JsonPatch([]))]) + # Replace table that exist in remaining + check(changes=[JsonChange(jsonpatch.JsonPatch( + [{"op":"replace", "path":"/TABLE2", "value":{"key3":"value3", "key4":"value4"}}]))], + assumed={"TABLE1":{}, "TABLE2":{}}, + remaining={"TABLE2":{"key1":"value1", "key2":"value2"}}, + expected=[JsonChange(jsonpatch.JsonPatch( + [{"op":"add", "path":"/TABLE2/key3", "value":"value3"}, + {"op":"add", "path":"/TABLE2/key4", "value":"value4"}]))]) + # Multiple changes + check(changes=[JsonChange(jsonpatch.JsonPatch([{"op":"replace", "path":"/TABLE1", "value":{}}])), + JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3", "value":{"key34":"value34"}}])), + JsonChange(jsonpatch.JsonPatch([{"op":"replace", "path":"/TABLE3", "value":{}}])), + JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3/key33", "value":"value33"}])), + JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE3"}]))], + assumed={"TABLE1":{},"TABLE3":{}}, + remaining={"TABLE3":{"key31":"value31", "key32":"value32"}}, + expected=[JsonChange(jsonpatch.JsonPatch([])), + JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3/key34", "value":"value34"}])), + JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE3/key34"}])), + JsonChange(jsonpatch.JsonPatch([{"op":"add", "path":"/TABLE3/key33", "value":"value33"}])), + JsonChange(jsonpatch.JsonPatch([{"op":"remove", "path":"/TABLE3/key33"}]))]) + +class TestConfigSplitter(unittest.TestCase): + def test_split_yang_non_yang_distinct_field_path(self): + def check(config, expected_yang, expected_non_yang, ignore_paths_list=[], ignore_tables_without_yang=False): + config_wrapper = ConfigWrapper() + inner_config_splitters = [] + if ignore_tables_without_yang: + inner_config_splitters.append(ps.TablesWithoutYangConfigSplitter(config_wrapper)) + if ignore_paths_list: + inner_config_splitters.append(ps.IgnorePathsFromYangConfigSplitter(ignore_paths_list, config_wrapper)) + + # ConfigWrapper() loads yang models from YANG_DIR + splitter = ps.ConfigSplitter(ConfigWrapper(), inner_config_splitters) + actual_yang, actual_non_yang = splitter.split_yang_non_yang_distinct_field_path(config) + + self.assertDictEqual(expected_yang, actual_yang) + self.assertDictEqual(expected_non_yang, actual_non_yang) + + # test no flags + check({}, {}, {}) + check(config={"ACL_TABLE":{"key1":"value1"}, "NON_YANG":{"key2":"value2"}, "VLAN":{"key31":"value31"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_yang={"ACL_TABLE":{"key1":"value1"}, "VLAN":{"key31":"value31"}, "NON_YANG":{"key2":"value2"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_non_yang={}) + + # test ignore_tables_without_yang + check({}, {}, {}, [], True) + self.assertRaises(ValueError, check, {"ACL_TABLE":{}}, {"ACL_TABLE":{}}, {}, [], True) # ACL_TABLE has YANG model + check({"ACL_TABLE":{"key1":"value1"}}, {"ACL_TABLE":{"key1":"value1"}}, {}, [], True) + self.assertRaises(ValueError, check, {"ACL_TABLE":{}, "NON_YANG":{}}, {"ACL_TABLE":{}}, {"NON_YANG":{}},[], True) + check(config={"ACL_TABLE":{"key1":"value1"}, "NON_YANG":{"key2":"value2"}, "VLAN":{"key31":"value31"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_yang={"ACL_TABLE":{"key1":"value1"}, "VLAN":{"key31":"value31"}}, + expected_non_yang={"NON_YANG":{"key2":"value2"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + ignore_tables_without_yang=True) + + # test ignore_paths_list + check({}, {}, {}, [""]) + self.assertRaises(ValueError, check, {"ACL_TABLE":{}}, {"ACL_TABLE":{}}, {}, ["/VLAN"]) # VLAN has YANG model + self.assertRaises(ValueError, check, {"ACL_TABLE":{}}, {}, {"ACL_TABLE":{}}, ["/ACL_TABLE"]) + check({"ACL_TABLE":{"key1":"value1"}}, {}, {"ACL_TABLE":{"key1":"value1"}}, ["/ACL_TABLE"]) + check({"ACL_TABLE":{"key1":"value1"}}, {}, {"ACL_TABLE":{"key1":"value1"}}, ["/ACL_TABLE/key1"]) + check(config={"NON_YANG":{"key1":"value1"},"ACL_TABLE":{"key2":"value2"}}, + expected_yang={"NON_YANG":{"key1":"value1"}}, + expected_non_yang={"ACL_TABLE":{"key2":"value2"}}, + ignore_paths_list= ["/ACL_TABLE"]) + check(config={"ACL_TABLE":{"key1":"value1"}, "VLAN":{"key31":"value31"}, "NON_YANG":{"key2":"value2"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_yang={"NON_YANG":{"key2":"value2"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_non_yang={"ACL_TABLE":{"key1":"value1"}, "VLAN":{"key31":"value31"}}, + ignore_paths_list=["/VLAN/key31", "/ACL_TABLE"]) + check(config={"ACL_TABLE":{"key1":"value1"}, "NON_YANG":{"key2":"value2"}, "VLAN":{"key31":"value31"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_yang={}, + expected_non_yang={"ACL_TABLE":{"key1":"value1"}, "VLAN":{"key31":"value31"}, "NON_YANG":{"key2":"value2"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + ignore_paths_list=["/VLAN/key31", "", "/ACL_TABLE"]) + + # test ignore_paths_list and ignore_tables_without_yang + check({}, {}, {}, [""]) + self.assertRaises(ValueError, check, {"ACL_TABLE":{}}, {"ACL_TABLE":{}}, {}, ["/VLAN"], True) # VLAN has YANG model + self.assertRaises(ValueError, check, {"ACL_TABLE":{}}, {}, {"ACL_TABLE":{}}, ["/ACL_TABLE"], True) + check({"ACL_TABLE":{"key1":"value1"}}, {}, {"ACL_TABLE":{"key1":"value1"}}, ["/ACL_TABLE"], True) + check({"ACL_TABLE":{"key1":"value1"}}, {}, {"ACL_TABLE":{"key1":"value1"}}, ["/ACL_TABLE/key1"], True) + check(config={"NON_YANG":{"key1":"value1"},"ACL_TABLE":{"key2":"value2"}}, + expected_yang={}, + expected_non_yang={"NON_YANG":{"key1":"value1"},"ACL_TABLE":{"key2":"value2"}}, + ignore_paths_list= ["/ACL_TABLE"], + ignore_tables_without_yang=True) + check(config={"ACL_TABLE":{"key1":"value1"}, "NON_YANG":{"key2":"value2"}, "VLAN":{"key31":"value31"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_yang={}, + expected_non_yang={"ACL_TABLE":{"key1":"value1"}, "VLAN":{"key31":"value31"}, "NON_YANG":{"key2":"value2"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + ignore_paths_list=["/VLAN/key31", "/ACL_TABLE"], + ignore_tables_without_yang=True) + check(config={"ACL_TABLE":{"key1":"value1"}, "NON_YANG":{"key2":"value2"}, "VLAN":{"key31":"value31"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + expected_yang={}, + expected_non_yang={"ACL_TABLE":{"key1":"value1"}, "VLAN":{"key31":"value31"}, "NON_YANG":{"key2":"value2"}, "ANOTHER_NON_YANG":{"key41":"value41"}}, + ignore_paths_list=["/VLAN/key31", "", "/ACL_TABLE"], + ignore_tables_without_yang=True) + + def test_merge_configs_with_distinct_field_path(self): + def check(config1, config2, expected=None): + splitter = ps.ConfigSplitter(ConfigWrapper(), []) + + # merging config1 and config2 + actual = splitter.merge_configs_with_distinct_field_path(config1, config2) + self.assertDictEqual(expected, actual) + + # merging config2 and config1 - should be the same result + actual = splitter.merge_configs_with_distinct_field_path(config2, config1) + self.assertDictEqual(expected, actual) + + check({}, {}, {}) + check({"TABLE1":{}}, {}, {"TABLE1":{}}) + check({"TABLE1":{}}, {"TABLE2": {}}, {"TABLE1":{}, "TABLE2":{}}) + check({"TABLE1":{"key1": "value1"}}, {}, {"TABLE1":{"key1": "value1"}}) + check({"TABLE1":{"key1": "value1"}}, {"TABLE1":{}}, {"TABLE1":{"key1": "value1"}}) + check({"TABLE1":{"key1": "value1"}}, + {"TABLE1":{"key2": "value2"}}, + {"TABLE1":{"key1": "value1", "key2": "value2"}}) + # keys the same + self.assertRaises(ValueError, check, {"TABLE1":{"key1": "value1"}}, {"TABLE1":{"key1": "value2"}}) + +class TestNonStrictPatchSorter(unittest.TestCase): + def test_sort__invalid_yang_covered_config__failure(self): + # Arrange + sorter = self.__create_patch_sorter(valid_yang_covered_config=False) + + # Act and assert + self.assertRaises(ValueError, sorter.sort, Files.MULTI_OPERATION_CONFIG_DB_PATCH) + + def test_sort__invalid_yang_covered_config_patch_updating_tables_without_yang__failure(self): + # Arrange + sorter = self.__create_patch_sorter(valid_patch_only_tables_with_yang_models=False) + + # Act and assert + self.assertRaises(ValueError, sorter.sort, Files.MULTI_OPERATION_CONFIG_DB_PATCH) + + def test_sort__no_errors_algorithm_specified__calls_inner_patch_sorter(self): + # Arrange + patch = Mock() + algorithm = Mock() + non_yang_changes = [Mock()] + yang_changes = [Mock(), Mock()] + expected = non_yang_changes + yang_changes + sorter = self.__create_patch_sorter(patch, algorithm, non_yang_changes, yang_changes) + + # Act + actual = sorter.sort(patch, algorithm) + + # Assert + self.assertListEqual(expected, actual) + + def test_sort__no_errors_algorithm_not_specified__calls_inner_patch_sorter(self): + # Arrange + patch = Mock() + non_yang_changes = [Mock()] + yang_changes = [Mock(), Mock()] + expected = non_yang_changes + yang_changes + sorter = self.__create_patch_sorter(patch, None, non_yang_changes, yang_changes) + + # Act + actual = sorter.sort(patch) + + # Assert + self.assertListEqual(expected, actual) + + def __create_patch_sorter(self, + patch=None, + any_algorithm=None, + any_adjusted_changes_non_yang=None, + any_adjusted_changes_yang=None, + valid_yang_covered_config=True, + valid_patch_only_tables_with_yang_models=True): + ignore_paths_list = Mock() + config_wrapper = Mock() + patch_wrapper = Mock() + inner_patch_sorter = Mock() + change_wrapper = Mock() + config_splitter = Mock() + + patch = patch if patch else Mock() + any_algorithm = any_algorithm if any_algorithm else ps.Algorithm.DFS + any_current_config = Mock() + any_target_config = Mock() + any_current_config_yang = Mock() + any_current_config_non_yang = Mock() + any_target_config_yang = Mock() + any_target_config_non_yang = Mock() + any_patch_non_yang = jsonpatch.JsonPatch([{"op":"add", "path":"/NON_YANG_TABLE", "value":{}}]) + any_patch_yang = Mock() + any_changes_yang = [Mock()] + any_changes_non_yang = [JsonChange(any_patch_non_yang)] + + config_wrapper.get_config_db_as_json.side_effect = \ + [any_current_config] + + patch_wrapper.simulate_patch.side_effect = \ + create_side_effect_dict( + {(str(patch), str(any_current_config)): + any_target_config}) + + config_splitter.split_yang_non_yang_distinct_field_path.side_effect = \ + create_side_effect_dict( + {(str(any_current_config),): (any_current_config_yang, any_current_config_non_yang), + (str(any_target_config),): (any_target_config_yang, any_target_config_non_yang)}) + + config_wrapper.validate_config_db_config.side_effect = \ + create_side_effect_dict({(str(any_target_config_yang),): valid_yang_covered_config}) + + patch_wrapper.generate_patch.side_effect = \ + create_side_effect_dict( + {(str(any_current_config_non_yang), str(any_target_config_non_yang)): any_patch_non_yang, + (str(any_current_config_yang), str(any_target_config_yang)): any_patch_yang}) + + patch_wrapper.validate_config_db_patch_has_yang_models.side_effect = \ + create_side_effect_dict( + {(str(any_patch_yang),): valid_patch_only_tables_with_yang_models}) + + inner_patch_sorter.sort.side_effect = \ + create_side_effect_dict( + {(str(any_patch_yang), str(any_algorithm), str(any_current_config_yang)): any_changes_yang}) + + change_wrapper.adjust_changes.side_effect = \ + create_side_effect_dict( + {(str(any_changes_non_yang), str(any_current_config_non_yang), str(any_current_config_yang)): any_adjusted_changes_non_yang, + (str(any_changes_yang), str(any_current_config_yang), str(any_target_config_non_yang)): any_adjusted_changes_yang}) + + return ps.NonStrictPatchSorter(config_wrapper, patch_wrapper, config_splitter, change_wrapper, inner_patch_sorter) + +class TestStrictPatchSorter(unittest.TestCase): + def test_sort__patch_updating_tables_without_yang__failure(self): + # Arrange + patch = Mock() + sorter = self.__create_patch_sorter(patch, valid_patch_only_tables_with_yang_models=False) + + # Act and assert + self.assertRaises(ValueError, sorter.sort, patch) + + def test_sort__target_config_not_valid_according_to_yang__failure(self): + # Arrange + patch = Mock() + sorter = self.__create_patch_sorter(patch, valid_config_db=False) + + # Act and assert + self.assertRaises(ValueError, sorter.sort, patch) + + def test_sort__no_errors_algorithm_specified__calls_inner_patch_sorter(self): + # Arrange + patch = Mock() + algorithm = Mock() + changes = [Mock(), Mock(), Mock()] + sorter = self.__create_patch_sorter(patch, algorithm, changes) + + # Act + actual = sorter.sort(patch, algorithm) + + # Assert + self.assertListEqual(changes, actual) + + def test_sort__no_errors_algorithm_not_specified__calls_inner_patch_sorter(self): + # Arrange + patch = Mock() + changes = [Mock(), Mock(), Mock()] + sorter = self.__create_patch_sorter(patch, None, changes) + + # Act + actual = sorter.sort(patch) + + # Assert + self.assertListEqual(changes, actual) + + def __create_patch_sorter(self, + patch=None, + algorithm=None, + changes=None, + valid_patch_only_tables_with_yang_models=True, + valid_config_db=True): + config_wrapper = Mock() + patch_wrapper = Mock() + inner_patch_sorter = Mock() + + any_current_config = Mock() + any_target_config = Mock() + patch = patch if patch else Mock() + algorithm = algorithm if algorithm else ps.Algorithm.DFS + + config_wrapper.get_config_db_as_json.side_effect = \ + [any_current_config, any_target_config] + + patch_wrapper.simulate_patch.side_effect = \ + create_side_effect_dict( + {(str(patch), str(any_current_config)): + any_target_config}) + + patch_wrapper.validate_config_db_patch_has_yang_models.side_effect = \ + create_side_effect_dict( + {(str(patch),): valid_patch_only_tables_with_yang_models}) + + config_wrapper.validate_config_db_config.side_effect = \ + create_side_effect_dict( + {(str(any_target_config),): valid_config_db}) + + + inner_patch_sorter.sort.side_effect = \ + create_side_effect_dict( + {(str(patch), str(algorithm)): changes}) + + return ps.StrictPatchSorter(config_wrapper, patch_wrapper, inner_patch_sorter)