diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 885f6cb73..4aacfb262 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -175,3 +175,139 @@ stages: - publish: '$(System.DefaultWorkingDirectory)/dist/' artifact: wheels displayName: "Publish Python wheels" + + - script: | + set -e + pushd generic_config_updater + python3 -m build -n + displayName: 'Build Python 3 wheel for GCU' + + - publish: '$(System.DefaultWorkingDirectory)/generic_config_updater/dist/' + artifact: gcu_wheels + displayName: "Publish Python wheels for GCU" + +- stage: BuildData + + jobs: + - job: + displayName: "Build sonic-utilities-data" + pool: + vmImage: ubuntu-24.04 + + container: + image: sonicdev-microsoft.azurecr.io:443/sonic-slave-bookworm:$(BUILD_BRANCH) + + steps: + - script: | + set -ex + sudo apt-get update + sudo apt-get install -y python3-pip + sudo apt-get install -y python3-protobuf + displayName: "Install dependencies" + + - script: | + sourceBranch=$(Build.SourceBranchName) + if [[ "$(Build.Reason)" == "PullRequest" ]];then + sourceBranch=$(System.PullRequest.TargetBranch) + fi + echo "Download artifact branch: $sourceBranch" + echo "##vso[task.setvariable variable=sourceBranch]$sourceBranch" + displayName: "Get correct artifact downloading branch" + + - task: DownloadPipelineArtifact@2 + inputs: + source: specific + project: build + pipeline: 142 + artifact: sonic-buildimage.vs + runVersion: 'latestFromBranch' + runBranch: 'refs/heads/$(sourceBranch)' + patterns: | + **/*.deb + **/*.whl + displayName: "Download artifacts from latest sonic-buildimage build" + + - script: | + set -xe + sudo apt-get -y purge libnl-3-dev libnl-route-3-dev || true + sudo dpkg -i libnl-3-200_*.deb + sudo dpkg -i libnl-genl-3-200_*.deb + sudo dpkg -i libnl-route-3-200_*.deb + sudo dpkg -i libnl-nf-3-200_*.deb + sudo dpkg -i libyang_1.0.73_amd64.deb + sudo dpkg -i libyang-cpp_1.0.73_amd64.deb + sudo dpkg -i python3-yang_1.0.73_amd64.deb + workingDirectory: $(Pipeline.Workspace)/target/debs/bookworm/ + displayName: 'Install Debian dependencies' + + - task: DownloadPipelineArtifact@2 + inputs: + source: specific + project: build + pipeline: 9 + artifact: sonic-swss-common-bookworm + runVersion: 'latestFromBranch' + runBranch: 'refs/heads/$(sourceBranch)' + displayName: "Download sonic swss common deb packages" + + - script: | + set -xe + sudo dpkg -i libswsscommon_1.0.0_amd64.deb + sudo dpkg -i python3-swsscommon_1.0.0_amd64.deb + workingDirectory: $(Pipeline.Workspace)/ + displayName: 'Install swss-common dependencies' + + - task: DownloadPipelineArtifact@2 + inputs: + source: specific + project: build + pipeline: sonic-net.sonic-dash-api + artifact: sonic-dash-api + runVersion: 'latestFromBranch' + runBranch: 'refs/heads/$(BUILD_BRANCH)' + path: $(Build.ArtifactStagingDirectory)/download + patterns: | + libdashapi*.deb + displayName: "Download dash api" + + - script: | + set -xe + sudo apt-get update + sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/libdashapi_*.deb + workingDirectory: $(Pipeline.Workspace)/ + displayName: 'Install libdashapi libraries' + + - script: | + set -xe + sudo pip3 install swsssdk-2.0.1-py3-none-any.whl + sudo pip3 install sonic_py_common-1.0-py3-none-any.whl + sudo pip3 install sonic_yang_mgmt-1.0-py3-none-any.whl + sudo pip3 install sonic_yang_models-1.0-py3-none-any.whl + sudo pip3 install sonic_config_engine-1.0-py3-none-any.whl + sudo pip3 install sonic_platform_common-1.0-py3-none-any.whl + workingDirectory: $(Pipeline.Workspace)/target/python-wheels/bookworm/ + displayName: 'Install Python dependencies' + + - task: DownloadPipelineArtifact@2 + inputs: + artifact: wheels + path: $(Build.ArtifactStagingDirectory)/download + displayName: "Download pre-stage built sonic-utilities" + + - script: | + sudo pip3 install sonic_utilities-1.2-py3-none-any.whl + workingDirectory: $(Build.ArtifactStagingDirectory)/download + displayName: 'Install sonic-utilities wheel' + + - script: | + set -xe + cd sonic-utilities-data + dpkg-buildpackage -b + cd .. + mkdir -p dist + mv sonic-utilities-data_*.deb dist/ + displayName: 'Build sonic-utilities-data package' + + - publish: '$(System.DefaultWorkingDirectory)/dist/' + artifact: sonic_utilities_data + displayName: "Publish sonic-utilities-data package" diff --git a/config/main.py b/config/main.py index 6420e4ae4..0806ac26d 100644 --- a/config/main.py +++ b/config/main.py @@ -1,12 +1,9 @@ #!/usr/sbin/env python -import threading import click -import concurrent.futures import datetime import ipaddress import json -import jsonpatch import netaddr import netifaces import os @@ -22,8 +19,11 @@ from jsonpatch import JsonPatchConflict from jsonpointer import JsonPointerException from collections import OrderedDict -from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat, extract_scope -from generic_config_updater.gu_common import HOST_NAMESPACE, GenericConfigUpdaterError +from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat +from generic_config_updater.gu_common import HOST_NAMESPACE +from generic_config_updater.main import ( + apply_patch_from_file as _gcu_apply_patch_from_file +) from minigraph import parse_device_desc_xml, minigraph_encoder from natsort import natsorted from portconfig import get_child_ports @@ -31,7 +31,6 @@ from sonic_py_common import device_info, multi_asic from sonic_py_common.general import getstatusoutput_noshell from sonic_py_common.interface import get_interface_table_name, get_port_table_name, get_intf_longname -from sonic_yang_cfg_generator import SonicYangCfgDbGenerator from utilities_common import util_base from swsscommon import swsscommon from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, ConfigDBPipeConnector, \ @@ -141,6 +140,10 @@ # Helper functions # + +# get_all_running_config is imported from generic_config_updater.main + + # Sort nested dict def sort_dict(data): """ Sort of 1st level and 2nd level dict of data naturally by its key @@ -1236,64 +1239,6 @@ def multiasic_save_to_singlefile(db, filename): json.dump(all_current_config, file, indent=4) -def apply_patch_wrapper(args): - return apply_patch_for_scope(*args) - - -# Function to apply patch for a single ASIC. -def apply_patch_for_scope(scope_changes, results, config_format, verbose, dry_run, ignore_non_yang_tables, ignore_path): - scope, changes = scope_changes - # Replace localhost to DEFAULT_NAMESPACE which is db definition of Host - if scope.lower() == HOST_NAMESPACE or scope == "": - scope = multi_asic.DEFAULT_NAMESPACE - - scope_for_log = scope if scope else HOST_NAMESPACE - thread_id = threading.get_ident() - log.log_notice(f"apply_patch_for_scope started for {scope_for_log} by {changes} in thread:{thread_id}") - - try: - # Call apply_patch with the ASIC-specific changes and predefined parameters - GenericUpdater(scope=scope).apply_patch(jsonpatch.JsonPatch(changes), - config_format, - verbose, - dry_run, - ignore_non_yang_tables, - ignore_path) - results[scope_for_log] = {"success": True, "message": "Success"} - log.log_notice(f"'apply-patch' executed successfully for {scope_for_log} by {changes} in thread:{thread_id}") - except Exception as e: - results[scope_for_log] = {"success": False, "message": str(e)} - log.log_error(f"'apply-patch' executed failed for {scope_for_log} by {changes} due to {str(e)}") - - -def validate_patch(patch): - try: - command = ["show", "runningconfiguration", "all"] - proc = subprocess.Popen(command, text=True, stdout=subprocess.PIPE) - all_running_config, returncode = proc.communicate() - if returncode: - log.log_notice(f"Fetch all runningconfiguration failed as output:{all_running_config}") - return False - - # Structure validation and simulate apply patch. - all_target_config = patch.apply(json.loads(all_running_config)) - - # Verify target config by YANG models - target_config = all_target_config.pop(HOST_NAMESPACE) if multi_asic.is_multi_asic() else all_target_config - target_config.pop("bgpraw", None) - if not SonicYangCfgDbGenerator().validate_config_db_json(target_config): - return False - - if multi_asic.is_multi_asic(): - for asic in multi_asic.get_namespace_list(): - target_config = all_target_config.pop(asic) - target_config.pop("bgpraw", None) - if not SonicYangCfgDbGenerator().validate_config_db_json(target_config): - return False - - return True - except Exception as e: - raise GenericConfigUpdaterError(f"Validate json patch: {patch} failed due to:{e}") def multiasic_validate_single_file(filename): @@ -1603,6 +1548,7 @@ def print_dry_run_message(dry_run): if dry_run: click.secho("** DRY RUN EXECUTION **", fg="yellow", underline=True) + @config.command('apply-patch') @click.argument('patch-file-path', type=str, required=True) @click.option('-f', '--format', type=click.Choice([e.name for e in ConfigFormat]), @@ -1622,74 +1568,21 @@ def apply_patch(ctx, patch_file_path, format, dry_run, parallel, ignore_non_yang format or SonicYang format. : Path to the patch file on the file-system.""" - try: - print_dry_run_message(dry_run) - with open(patch_file_path, 'r') as fh: - text = fh.read() - patch_as_json = json.loads(text) - patch = jsonpatch.JsonPatch(patch_as_json) + print_dry_run_message(dry_run) - if not validate_patch(patch): - raise GenericConfigUpdaterError(f"Failed validating patch:{patch}") - - results = {} - config_format = ConfigFormat[format.upper()] - # Initialize a dictionary to hold changes categorized by scope - changes_by_scope = {} - - # Iterate over each change in the JSON Patch - for change in patch: - scope, modified_path = extract_scope(change["path"]) - - # Modify the 'path' in the change to remove the scope - change["path"] = modified_path - - # Check if the scope is already in our dictionary, if not, initialize it - if scope not in changes_by_scope: - changes_by_scope[scope] = [] - - # Add the modified change to the appropriate list based on scope - changes_by_scope[scope].append(change) + try: + _gcu_apply_patch_from_file( + patch_file_path, + config_format_name=format, + verbose=verbose, + dry_run=dry_run, + parallel=parallel, + ignore_non_yang_tables=ignore_non_yang_tables, + ignore_path=ignore_path, + ) - # Empty case to force validate YANG model. - if not changes_by_scope: - asic_list = [multi_asic.DEFAULT_NAMESPACE] - if multi_asic.is_multi_asic(): - asic_list.extend(multi_asic.get_namespace_list()) - for asic in asic_list: - changes_by_scope[asic] = [] - - # Apply changes for each scope - if parallel: - with concurrent.futures.ThreadPoolExecutor() as executor: - # Prepare the argument tuples - arguments = [(scope_changes, results, config_format, - verbose, dry_run, ignore_non_yang_tables, ignore_path) - for scope_changes in changes_by_scope.items()] - - # Submit all tasks and wait for them to complete - futures = [executor.submit(apply_patch_wrapper, args) for args in arguments] - - # Wait for all tasks to complete - concurrent.futures.wait(futures) - else: - for scope_changes in changes_by_scope.items(): - apply_patch_for_scope(scope_changes, - results, - config_format, - verbose, dry_run, - ignore_non_yang_tables, - ignore_path) - - # Check if any updates failed - failures = [scope for scope, result in results.items() if not result['success']] - - if failures: - failure_messages = '\n'.join([f"- {failed_scope}: {results[failed_scope]['message']}" for failed_scope in failures]) - raise GenericConfigUpdaterError(f"Failed to apply patch on the following scopes:\n{failure_messages}") - - log.log_notice(f"Patch applied successfully for {patch}.") + log.log_notice("Patch applied successfully.") click.secho("Patch applied successfully.", fg="cyan", underline=True) except Exception as ex: click.secho("Failed to apply patch due to: {}".format(ex), fg="red", underline=True, err=True) diff --git a/generic_config_updater/.coveragerc b/generic_config_updater/.coveragerc new file mode 100644 index 000000000..d947f08ae --- /dev/null +++ b/generic_config_updater/.coveragerc @@ -0,0 +1,6 @@ +[run] +branch = True +source = generic_config_updater +omit = + .eggs/* + tests/* diff --git a/generic_config_updater/field_operation_validators.py b/generic_config_updater/field_operation_validators.py index ebc1cd6ad..4f97aede5 100644 --- a/generic_config_updater/field_operation_validators.py +++ b/generic_config_updater/field_operation_validators.py @@ -6,7 +6,12 @@ from sonic_py_common import device_info from .gu_common import GenericConfigUpdaterError from swsscommon import swsscommon -from utilities_common.constants import DEFAULT_SUPPORTED_FECS_LIST + +# Default FEC modes when STATE_DB does not advertise supported_fecs for a port. +# Kept local to avoid pulling utilities_common into the GCU wheel. +# NOTE: A duplicate of this list exists in utilities_common/constants.py. +# If you update this list, update that copy too. +DEFAULT_SUPPORTED_FECS_LIST = ['rs', 'fc', 'none', 'auto'] SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) GCU_TABLE_MOD_CONF_FILE = f"{SCRIPT_DIR}/gcu_field_operation_validators.conf.json" diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 0c209a563..cc6bb7ed4 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -9,6 +9,7 @@ import copy import re import os +import hashlib from sonic_py_common import logger, multi_asic from enum import Enum from functools import cmp_to_key @@ -81,6 +82,8 @@ def __init__(self, yang_dir=YANG_DIR, scope=multi_asic.DEFAULT_NAMESPACE): self.scope = scope self.yang_dir = YANG_DIR self.sonic_yang_with_loaded_models = None + self._validate_config_cache = {} + self._currently_loaded_hash = None def get_config_db_as_json(self): return get_config_db_as_json(self.scope) @@ -138,6 +141,16 @@ def validate_sonic_yang_config(self, sonic_yang_as_json): return False, ex def validate_config_db_config(self, config_db_as_json): + # Cache validation results by config content hash. + # validate_config_db_config is a pure function: same config always produces + # the same result. Caching avoids redundant loadData() calls when the DFS + # revisits the same config state during backtracking. + _cache_key = hashlib.md5( + json.dumps(config_db_as_json, sort_keys=True).encode() + ).hexdigest() + if _cache_key in self._validate_config_cache: + return self._validate_config_cache[_cache_key] + sy = self.create_sonic_yang_with_loaded_models() # TODO: Move these validators to YANG models @@ -147,14 +160,22 @@ def validate_config_db_config(self, config_db_as_json): try: # Loading data automatically does full validation sy.loadData(config_db_as_json) + self._currently_loaded_hash = _cache_key for supplemental_yang_validator in supplemental_yang_validators: success, error = supplemental_yang_validator(config_db_as_json) if not success: - return success, error + result = (success, error) + self._validate_config_cache[_cache_key] = result + return result except sonic_yang.SonicYangException as ex: - return False, ex + self._currently_loaded_hash = None + result = (False, str(ex)) + self._validate_config_cache[_cache_key] = result + return result - return True, None + result = (True, None) + self._validate_config_cache[_cache_key] = result + return result def validate_field_operation(self, old_config, target_config): """ @@ -525,7 +546,17 @@ def find_ref_paths(self, paths, config, reload_config: bool = True): sy = self._create_sonic_yang_with_loaded_models() if reload_config: - sy.loadData(config) + _config_hash = hashlib.md5( + json.dumps(config, sort_keys=True).encode() + ).hexdigest() + already_loaded = ( + self.config_wrapper is not None and + self.config_wrapper._currently_loaded_hash == _config_hash + ) + if not already_loaded: + sy.loadData(config) + if self.config_wrapper is not None: + self.config_wrapper._currently_loaded_hash = _config_hash # Force to be a list if not isinstance(paths, list): diff --git a/generic_config_updater/main.py b/generic_config_updater/main.py new file mode 100644 index 000000000..ce0c1a99f --- /dev/null +++ b/generic_config_updater/main.py @@ -0,0 +1,793 @@ +""" +GCU (Generic Config Updater) — apply-patch orchestration +========================================================= + +This module is the **single source of truth** for all apply-patch logic. +It is consumed by: + +* ``config apply-patch`` (config/main.py — thin entry-point / standalone redirect) +* ``gcu-standalone`` (console_scripts entry point installed by the GCU wheel + into the GCU container virtual-env at + /opt/sonic/gcu/current/bin/gcu-standalone) + +No caller should re-implement scope extraction, parallel execution, +pre-processing, or per-scope dispatch — they should call the helpers +exposed here instead. +""" + +import copy +import json +import logging +import os +import sys +import argparse +import subprocess +import threading +import concurrent.futures + +import jsonpatch +import jsonpointer + +from generic_config_updater.generic_updater import ( + GenericUpdater, + ConfigFormat, + extract_scope, +) +from generic_config_updater.gu_common import ( + HOST_NAMESPACE, + GenericConfigUpdaterError, +) +from sonic_py_common import multi_asic +from utilities_common.general import load_db_config + +logger = logging.getLogger(__name__) + +# Constants +DEFAULT_CONFIG_DB_FILE = '/etc/sonic/config_db.json' + + +# --------------------------------------------------------------------------- +# Lightweight JSON-Patch format validation (RFC 6902) +# --------------------------------------------------------------------------- + +def validate_patch_format(patch): + """Return *True* if *patch* is a structurally valid JSON Patch list.""" + try: + if not isinstance(patch, list): + return False + for change in patch: + if not isinstance(change, dict): + return False + if 'op' not in change or 'path' not in change: + return False + if change['op'] not in ( + 'add', 'remove', 'replace', 'move', 'copy', 'test', + ): + return False + return True + except Exception: + return False + + +# --------------------------------------------------------------------------- +# Running-config retrieval +# --------------------------------------------------------------------------- + +def get_all_running_config(): + """Fetch all running configuration as a JSON string via ``show``.""" + command = ["show", "runningconfiguration", "all"] + proc = subprocess.Popen(command, text=True, stdout=subprocess.PIPE) + all_running_config, _ = proc.communicate() + if proc.returncode: + raise GenericConfigUpdaterError( + f"Fetch all runningconfiguration failed with rc={proc.returncode}" + ) + return all_running_config + + +# --------------------------------------------------------------------------- +# Patch pre-processing helpers +# --------------------------------------------------------------------------- + +def filter_duplicate_patch_operations(patch_ops, all_running_config): + """Remove leaf-list ``add`` ops that would create duplicate entries.""" + if not any(op.get("path", "").endswith("/-") for op in patch_ops): + return patch_ops + + config = ( + json.loads(all_running_config) + if isinstance(all_running_config, str) + else all_running_config + ) + + patch_copy = jsonpatch.JsonPatch([copy.deepcopy(op) for op in patch_ops]) + all_target_config = patch_copy.apply(config) + + def _find_duplicate_entries(cfg): + duplicates = {} + + def _check(obj, path=""): + if isinstance(obj, dict): + for k, v in obj.items(): + _check(v, f"{path}/{k}" if path else f"/{k}") + elif isinstance(obj, list): + seen, dups = set(), set() + for item in obj: + (dups if item in seen else seen).add(item) + if dups: + duplicates[path] = list(dups) + for idx, item in enumerate(obj): + _check(item, f"{path}[{idx}]") + + _check(cfg) + return duplicates + + dups = _find_duplicate_entries(all_target_config) + if not dups: + return patch_ops + + ops_to_remove = set() + for list_path, dup_values in dups.items(): + for op_idx, op in enumerate(patch_ops): + if ( + op.get("op") == "add" + and op.get("path", "").endswith("/-") + and op.get("path").startswith(list_path) + and op.get("value") in dup_values + ): + ops_to_remove.add(op_idx) + + return [op for idx, op in enumerate(patch_ops) if idx not in ops_to_remove] + + +def append_emptytables_if_required(patch_ops, all_running_config): + """Insert ``add`` ops for missing top-level tables before the first + reference to each table so that subsequent ops don't fail.""" + config = ( + json.loads(all_running_config) + if isinstance(all_running_config, str) + else all_running_config + ) + missing_tables = set() + patch_ops_copy = [copy.deepcopy(op) for op in patch_ops] + + for operation in patch_ops_copy: + if 'path' not in operation: + continue + path_parts = operation['path'].strip('/').split('/') + if not path_parts: + continue + + if path_parts[0].startswith('asic') or path_parts[0] == HOST_NAMESPACE: + if len(path_parts) < 2: + continue + table_path = f"/{path_parts[0]}/{path_parts[1]}" + else: + table_path = f"/{path_parts[0]}" + + try: + jsonpointer.resolve_pointer(config, table_path) + except jsonpointer.JsonPointerException: + missing_tables.add(table_path) + + if not missing_tables: + return patch_ops_copy + + for table in missing_tables: + insert_idx = None + for idx, op in enumerate(patch_ops_copy): + if 'path' in op and op['path'].startswith(table): + insert_idx = idx + break + empty_table_patch = {"op": "add", "path": table, "value": {}} + if insert_idx is not None: + patch_ops_copy.insert(insert_idx, empty_table_patch) + else: + patch_ops_copy.append(empty_table_patch) + + return patch_ops_copy + + +# --------------------------------------------------------------------------- +# Full YANG validation of a patch against running config +# --------------------------------------------------------------------------- + +def validate_patch(patch_ops, all_running_config): + """Simulate applying *patch_ops* to *all_running_config* and validate + the result against YANG models. Returns ``True`` on success. + + Raises ``GenericConfigUpdaterError`` on unexpected failures. + """ + try: + from sonic_yang_cfg_generator import SonicYangCfgDbGenerator + except ImportError: + # In environments without sonic_yang_cfg_generator (e.g. minimal + # standalone venv), skip YANG validation. + logger.warning( + "sonic_yang_cfg_generator not available; skipping YANG validation" + ) + return True + + try: + config = ( + json.loads(all_running_config) + if isinstance(all_running_config, str) + else all_running_config + ) + patch_copy = jsonpatch.JsonPatch( + [copy.deepcopy(op) for op in patch_ops] + ) + all_target_config = patch_copy.apply(config) + + target_config = ( + all_target_config.pop(HOST_NAMESPACE) + if multi_asic.is_multi_asic() + else all_target_config + ) + target_config.pop("bgpraw", None) + if not SonicYangCfgDbGenerator().validate_config_db_json( + target_config + ): + return False + + if multi_asic.is_multi_asic(): + for asic in multi_asic.get_namespace_list(): + target_config = all_target_config.pop(asic) + target_config.pop("bgpraw", None) + if not SonicYangCfgDbGenerator().validate_config_db_json( + target_config + ): + return False + + return True + except Exception as e: + raise GenericConfigUpdaterError( + f"Validate json patch: {patch_ops} failed due to: {e}" + ) + + +# --------------------------------------------------------------------------- +# Per-scope dispatch +# --------------------------------------------------------------------------- + +def apply_patch_for_scope(scope_changes, results, config_format, + verbose, dry_run, + ignore_non_yang_tables, ignore_path): + """Apply a patch for a single ASIC scope and record the outcome in + *results* (a shared dict).""" + scope, changes = scope_changes + if scope.lower() == HOST_NAMESPACE or scope == "": + scope = multi_asic.DEFAULT_NAMESPACE + + scope_for_log = scope if scope else HOST_NAMESPACE + thread_id = threading.get_ident() + logger.info( + "apply_patch_for_scope started for %s with %d changes in thread %s", + scope_for_log, len(changes), thread_id, + ) + + try: + GenericUpdater(scope=scope).apply_patch( + jsonpatch.JsonPatch(changes), + config_format, + verbose, + dry_run, + ignore_non_yang_tables, + ignore_path, + ) + results[scope_for_log] = {"success": True, "message": "Success"} + logger.info("apply-patch succeeded for %s", scope_for_log) + except Exception as e: + results[scope_for_log] = {"success": False, "message": str(e)} + logger.error("apply-patch failed for %s: %s", scope_for_log, e) + + +def _apply_patch_wrapper(args): + """Thin wrapper so ``ThreadPoolExecutor.submit`` can unpack a tuple.""" + return apply_patch_for_scope(*args) + + +# --------------------------------------------------------------------------- +# Top-level apply-patch orchestrator +# --------------------------------------------------------------------------- + +def apply_patch_from_file(patch_file_path, config_format_name, verbose, + dry_run, parallel, ignore_non_yang_tables, + ignore_path, preprocess=True): + """Read a JSON-Patch file and apply it — the single implementation + used by all entry points. + + Parameters + ---------- + patch_file_path : str + Path to the JSON-Patch file. + config_format_name : str + ``"CONFIGDB"`` or ``"SONICYANG"``. + verbose : bool + dry_run : bool + parallel : bool + If *True*, apply per-ASIC changes in parallel threads. + ignore_non_yang_tables : bool + ignore_path : tuple/list of str + preprocess : bool + When *True* (default), fetch running config and run + ``append_emptytables_if_required``, ``filter_duplicate_patch_operations`` + and ``validate_patch``. Callers that already performed these steps + (or intentionally want to skip them) can pass *False*. + + Raises + ------ + GenericConfigUpdaterError + On validation failure or any per-scope failure. + """ + # 1. Read & validate patch file + with open(patch_file_path, 'r') as fh: + patch_json = json.loads(fh.read()) + + if not validate_patch_format(patch_json): + raise GenericConfigUpdaterError( + f"Invalid patch format in file: {patch_file_path}" + ) + + patch_ops = patch_json + config_format = ConfigFormat[config_format_name.upper()] + + # 2. Optional pre-processing (running-config fetch + YANG validation) + if preprocess: + all_running_config = get_all_running_config() + patch_ops = append_emptytables_if_required( + patch_ops, all_running_config + ) + patch_ops = filter_duplicate_patch_operations( + patch_ops, all_running_config + ) + if not validate_patch(patch_ops, all_running_config): + raise GenericConfigUpdaterError( + f"Failed validating patch: {patch_ops}" + ) + + # 3. Build a JsonPatch and split by scope + patch = jsonpatch.JsonPatch(patch_ops) + changes_by_scope = {} + + for change in patch: + scope, modified_path = extract_scope(change["path"]) + change["path"] = modified_path + changes_by_scope.setdefault(scope, []).append(change) + + # Empty case — still force YANG validation per scope + if not changes_by_scope: + asic_list = [multi_asic.DEFAULT_NAMESPACE] + if multi_asic.is_multi_asic(): + asic_list.extend(multi_asic.get_namespace_list()) + for asic in asic_list: + changes_by_scope[asic] = [] + + # 4. Dispatch + results = {} + if parallel: + with concurrent.futures.ThreadPoolExecutor() as executor: + arguments = [ + (sc, results, config_format, verbose, dry_run, + ignore_non_yang_tables, ignore_path) + for sc in changes_by_scope.items() + ] + futures = [ + executor.submit(_apply_patch_wrapper, arg) + for arg in arguments + ] + concurrent.futures.wait(futures) + else: + for scope_changes in changes_by_scope.items(): + apply_patch_for_scope( + scope_changes, results, config_format, + verbose, dry_run, ignore_non_yang_tables, ignore_path, + ) + + # 5. Aggregate results + failures = [s for s, r in results.items() if not r['success']] + if failures: + msgs = '\n'.join( + f"- {s}: {results[s]['message']}" for s in failures + ) + raise GenericConfigUpdaterError( + f"Failed to apply patch on the following scopes:\n{msgs}" + ) + + +# --------------------------------------------------------------------------- +# Helper utilities (used by the gcu-standalone entry point) +# --------------------------------------------------------------------------- + +def multiasic_save_to_singlefile(filename): + """Save all ASIC configurations to a single file in multi-asic mode.""" + all_configs = {} + + # Get host configuration + cmd = ["sonic-cfggen", "-d", "--print-data"] + result = subprocess.run(cmd, capture_output=True, text=True, check=True) + host_config = json.loads(result.stdout) + all_configs['localhost'] = host_config + + # Get each ASIC configuration + for namespace in multi_asic.get_namespace_list(): + cmd = ["sonic-cfggen", "-d", "--print-data", "-n", namespace] + result = subprocess.run( + cmd, capture_output=True, text=True, check=True + ) + asic_config = json.loads(result.stdout) + all_configs[namespace] = asic_config + + # Save to file + with open(filename, 'w') as f: + json.dump(all_configs, f, indent=2) + + +def print_error(message): + """Print error message to stderr.""" + print(f"Error: {message}", file=sys.stderr) + + +def print_success(message): + """Print success message.""" + print(message) + + +# --------------------------------------------------------------------------- +# Sub-command implementations (used by gcu-standalone) +# --------------------------------------------------------------------------- + +def create_checkpoint(args): + """Create a checkpoint of the current configuration.""" + try: + if args.verbose: + print(f"Creating checkpoint: {args.checkpoint_name}") + + updater = GenericUpdater() + updater.checkpoint(args.checkpoint_name, args.verbose) + + print_success( + f"Checkpoint '{args.checkpoint_name}' created successfully." + ) + except Exception as ex: + print_error( + f"Failed to create checkpoint '{args.checkpoint_name}': {ex}" + ) + sys.exit(1) + + +def delete_checkpoint(args): + """Delete a checkpoint.""" + try: + if args.verbose: + print(f"Deleting checkpoint: {args.checkpoint_name}") + + updater = GenericUpdater() + updater.delete_checkpoint(args.checkpoint_name, args.verbose) + + print_success( + f"Checkpoint '{args.checkpoint_name}' deleted successfully." + ) + except Exception as ex: + print_error( + f"Failed to delete checkpoint '{args.checkpoint_name}': {ex}" + ) + sys.exit(1) + + +def list_checkpoints(args): + """List all available checkpoints.""" + try: + updater = GenericUpdater() + checkpoints = updater.list_checkpoints(args.verbose) + + if not checkpoints: + print("No checkpoints found.") + return + + print("Available checkpoints:") + for checkpoint in checkpoints: + print(f" - {checkpoint}") + except Exception as ex: + print_error(f"Failed to list checkpoints: {ex}") + sys.exit(1) + + +def apply_patch(args): + """Apply a configuration patch — delegates to apply_patch_from_file.""" + try: + if args.verbose: + print(f"Applying patch from: {args.patch_file}") + print(f"Format: {args.format}") + if args.dry_run: + print("** DRY RUN EXECUTION **") + + apply_patch_from_file( + patch_file_path=args.patch_file, + config_format_name=args.format, + verbose=args.verbose, + dry_run=args.dry_run, + parallel=args.parallel, + ignore_non_yang_tables=args.ignore_non_yang_tables, + ignore_path=args.ignore_path, + preprocess=True, + ) + + print_success("Patch applied successfully.") + except Exception as ex: + print_error(f"Failed to apply patch: {ex}") + sys.exit(1) + + +def replace_config(args): + """Replace the entire configuration with a new configuration.""" + try: + if args.verbose: + print(f"Replacing configuration from: {args.config_file}") + print(f"Format: {args.format}") + + with open(args.config_file, 'r') as f: + target_config = json.loads(f.read()) + + config_format = ConfigFormat[args.format.upper()] + updater = GenericUpdater() + updater.replace( + target_config, config_format, args.verbose, False, + args.ignore_non_yang_tables, args.ignore_path, + ) + + print_success("Configuration replaced successfully.") + except Exception as ex: + print_error(f"Failed to replace configuration: {ex}") + sys.exit(1) + + +def save_config(args): + """Save the current configuration to a file.""" + try: + filename = args.filename if args.filename else DEFAULT_CONFIG_DB_FILE + + if args.verbose: + print(f"Saving configuration to: {filename}") + + if multi_asic.is_multi_asic(): + multiasic_save_to_singlefile(filename) + else: + cmd = ["sonic-cfggen", "-d", "--print-data"] + result = subprocess.run( + cmd, capture_output=True, text=True, check=True + ) + config_to_save = json.loads(result.stdout) + with open(filename, 'w') as f: + json.dump(config_to_save, f, indent=2) + + print_success(f"Configuration saved successfully to '{filename}'.") + except subprocess.CalledProcessError as e: + print_error(f"Failed to get current configuration: {e}") + sys.exit(1) + except Exception as ex: + print_error(f"Failed to save configuration: {ex}") + sys.exit(1) + + +def rollback_config(args): + """Rollback configuration to a checkpoint.""" + try: + if args.verbose: + print(f"Rolling back to checkpoint: {args.checkpoint_name}") + + updater = GenericUpdater() + updater.rollback( + args.checkpoint_name, args.verbose, False, + args.ignore_non_yang_tables, args.ignore_path, + ) + + print_success( + f"Configuration rolled back to " + f"'{args.checkpoint_name}' successfully." + ) + except Exception as ex: + print_error( + f"Failed to rollback to checkpoint " + f"'{args.checkpoint_name}': {ex}" + ) + sys.exit(1) + + +# --------------------------------------------------------------------------- +# Argument parser (shared by gcu-standalone entry point) +# --------------------------------------------------------------------------- + +def build_parser(): + """Build and return the argument parser.""" + parser = argparse.ArgumentParser( + description=( + 'GCU - Generic Config Updater for SONiC configuration management' + ), + formatter_class=argparse.RawDescriptionHelpFormatter, + epilog=""" +Examples: + %(prog)s create-checkpoint my-checkpoint + %(prog)s apply-patch patch.json + %(prog)s apply-patch patch.json --dry-run + %(prog)s replace config.json + %(prog)s save backup.json + """, + ) + + subparsers = parser.add_subparsers( + dest='command', help='Available commands' + ) + + # ---- create-checkpoint ---- + p = subparsers.add_parser( + 'create-checkpoint', + help='Create a checkpoint of the current configuration', + ) + p.add_argument('checkpoint_name', help='Name for the checkpoint') + p.add_argument( + '-v', '--verbose', action='store_true', + help='Print additional details', + ) + + # ---- delete-checkpoint ---- + p = subparsers.add_parser( + 'delete-checkpoint', help='Delete a checkpoint', + ) + p.add_argument( + 'checkpoint_name', help='Name of the checkpoint to delete', + ) + p.add_argument( + '-v', '--verbose', action='store_true', + help='Print additional details', + ) + + # ---- list-checkpoints ---- + p = subparsers.add_parser( + 'list-checkpoints', help='List all available checkpoints', + ) + p.add_argument( + '-v', '--verbose', action='store_true', + help='Print additional details', + ) + + # ---- apply-patch ---- + p = subparsers.add_parser( + 'apply-patch', help='Apply a configuration patch', + ) + p.add_argument('patch_file', help='Path to the JSON patch file') + p.add_argument( + '-f', '--format', choices=['CONFIGDB', 'SONICYANG'], + default='CONFIGDB', + help='Format of the patch file (default: CONFIGDB)', + ) + p.add_argument( + '-v', '--verbose', action='store_true', + help='Print additional details', + ) + p.add_argument( + '-d', '--dry-run', action='store_true', default=False, + help='Test out the command without affecting config state', + ) + p.add_argument( + '-p', '--parallel', action='store_true', + help='Apply changes to all ASICs in parallel (multi-asic only)', + ) + p.add_argument( + '-n', '--ignore-non-yang-tables', action='store_true', + help='Ignore validation for tables without YANG models', + ) + p.add_argument( + '-i', '--ignore-path', action='append', default=[], + help='Ignore validation for config specified by given path ' + '(JsonPointer)', + ) + + # ---- replace ---- + p = subparsers.add_parser( + 'replace', help='Replace the entire configuration', + ) + p.add_argument('config_file', help='Path to the configuration file') + p.add_argument( + '-f', '--format', choices=['CONFIGDB', 'SONICYANG'], + default='CONFIGDB', + help='Format of the configuration file (default: CONFIGDB)', + ) + p.add_argument( + '-v', '--verbose', action='store_true', + help='Print additional details', + ) + p.add_argument( + '-n', '--ignore-non-yang-tables', action='store_true', + help='Ignore validation for tables without YANG models', + ) + p.add_argument( + '-i', '--ignore-path', action='append', default=[], + help='Ignore validation for config specified by given path ' + '(JsonPointer)', + ) + + # ---- save ---- + p = subparsers.add_parser( + 'save', help='Save the current configuration to a file', + ) + p.add_argument( + 'filename', nargs='?', + help=f'Output filename (default: {DEFAULT_CONFIG_DB_FILE})', + ) + p.add_argument( + '-v', '--verbose', action='store_true', + help='Print additional details', + ) + + # ---- rollback ---- + p = subparsers.add_parser( + 'rollback', help='Rollback configuration to a checkpoint', + ) + p.add_argument( + 'checkpoint_name', + help='Name of the checkpoint to rollback to', + ) + p.add_argument( + '-v', '--verbose', action='store_true', + help='Print additional details', + ) + p.add_argument( + '-n', '--ignore-non-yang-tables', action='store_true', + help='Ignore validation for tables without YANG models', + ) + p.add_argument( + '-i', '--ignore-path', action='append', default=[], + help='Ignore validation for config specified by given path ' + '(JsonPointer)', + ) + + return parser + + +# --------------------------------------------------------------------------- +# Main entry point (used by gcu-standalone console_scripts) +# --------------------------------------------------------------------------- + +def main(): + """Main entry point for the gcu-standalone console script.""" + load_db_config() + + parser = build_parser() + args = parser.parse_args() + + if not args.command: + parser.print_help() + return + + # Validate file paths if provided + if hasattr(args, 'patch_file') and args.patch_file: + if not os.path.exists(args.patch_file): + print_error(f"Patch file not found: {args.patch_file}") + sys.exit(1) + + if hasattr(args, 'config_file') and args.config_file: + if not os.path.exists(args.config_file): + print_error(f"Config file not found: {args.config_file}") + sys.exit(1) + + command_functions = { + 'create-checkpoint': create_checkpoint, + 'delete-checkpoint': delete_checkpoint, + 'list-checkpoints': list_checkpoints, + 'apply-patch': apply_patch, + 'replace': replace_config, + 'save': save_config, + 'rollback': rollback_config, + } + + if args.command in command_functions: + command_functions[args.command](args) + else: + print_error(f"Unknown command: {args.command}") + parser.print_help() + sys.exit(1) + + +if __name__ == '__main__': + main() diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index 2895c1982..75f20849c 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -1019,15 +1019,15 @@ def _validate_replace(self, move, diff, simulated_config): """ deleted_paths, added_paths = self._get_paths(diff.current_config, simulated_config, []) - # Note: on replace operations we are loading both current and simulated configs so we have to - # load twice :( - - # For deleted paths, we check the current config has no dependencies between nodes under the removed path - if not self._validate_paths_config(deleted_paths, diff.current_config, reload_config=True): + # Validate added_paths against simulated_config first: FullConfigMoveValidator has + # already loaded simulated_config into the sy singleton (via validate_config_db_config), + # so _currently_loaded_hash will match and find_ref_paths skips loadData. + # Then validate deleted_paths against current_config (requires a fresh loadData). + # This ordering gives 2 loadData calls instead of 3 for REPLACE operations. + if not self._validate_paths_config(added_paths, simulated_config, reload_config=True): return False - # For added paths, we check the simulated config has no dependencies between nodes under the added path - if not self._validate_paths_config(added_paths, simulated_config, reload_config=True): + if not self._validate_paths_config(deleted_paths, diff.current_config, reload_config=True): return False return True @@ -1266,6 +1266,61 @@ def _get_non_existing_keys_tokens(self, config1, config2, reverse): yield [table, key] +class BulkLeafListMoveGenerator: + """ + A non-extendable generator that produces a single REPLACE move for each + leaf-list field whose items differ between current and target configs. + + Instead of generating N individual REMOVE/ADD moves (one per list item), + this emits one REPLACE of the whole list. The DFS tries non-extendable + generators first, so if the bulk replace validates, we skip N-1 moves + and their associated loadData() calls. + + This is conservative: + - Only handles leaf-lists (lists of scalars, not lists of dicts) + - Only replaces lists that already exist in both current and target + - If this move fails validation, DFS continues to other generators + which produce per-item moves (no explicit fallback mechanism) + """ + def __init__(self, path_addressing): + self.path_addressing = path_addressing + + def generate(self, diff): + for move in self._traverse(diff, diff.current_config, diff.target_config, []): + yield move + + def _traverse(self, diff, current_ptr, target_ptr, tokens): + if not isinstance(current_ptr, dict) or not isinstance(target_ptr, dict): + return + + for key in current_ptr: + if key not in target_ptr: + continue + + tokens.append(key) + current_val = current_ptr[key] + target_val = target_ptr[key] + + if isinstance(current_val, list) and isinstance(target_val, list): + # Only handle leaf-lists (lists of scalars) + if (current_val != target_val and + self._is_leaf_list(current_val) and + self._is_leaf_list(target_val)): + yield JsonMoveGroup( + JsonMove(diff, OperationType.REPLACE, list(tokens), list(tokens)), + ) + elif isinstance(current_val, dict) and isinstance(target_val, dict): + for move in self._traverse(diff, current_val, target_val, tokens): + yield move + + tokens.pop() + + @staticmethod + def _is_leaf_list(lst): + """Return True if lst contains only scalars (str, int, float, bool).""" + return all(isinstance(item, (str, int, float, bool)) for item in lst) + + class BulkKeyLevelMoveGenerator: """ Same concept as KeyLevelMoveGenerator, but groups additions and removals of sibling keys. @@ -2114,7 +2169,9 @@ def create(self, algorithm=Algorithm.DFS): move_generators = [RemoveCreateOnlyDependencyMoveGenerator(self.path_addressing), LowLevelMoveGenerator(self.path_addressing)] # TODO: Enable TableLevelMoveGenerator once it is confirmed whole table can be updated at the same time - move_non_extendable_generators = [BulkKeyLevelMoveGenerator(self.path_addressing), + move_non_extendable_generators = [RemoveCreateOnlyDependencyMoveGenerator(self.path_addressing), + BulkLeafListMoveGenerator(self.path_addressing), + BulkKeyLevelMoveGenerator(self.path_addressing), KeyLevelMoveGenerator(self.path_addressing), BulkKeyGroupLowLevelMoveGenerator(self.path_addressing), BulkLowLevelMoveGenerator(self.path_addressing)] diff --git a/generic_config_updater/pytest.ini b/generic_config_updater/pytest.ini new file mode 100644 index 000000000..bf901aa5c --- /dev/null +++ b/generic_config_updater/pytest.ini @@ -0,0 +1,5 @@ +[pytest] +addopts = --cov-config=.coveragerc --cov --cov-report html --cov-report term --cov-report xml --junitxml=test-results.xml -vv +testpaths = ../tests/generic_config_updater + + diff --git a/generic_config_updater/setup.py b/generic_config_updater/setup.py new file mode 100644 index 000000000..2f0b0aa6d --- /dev/null +++ b/generic_config_updater/setup.py @@ -0,0 +1,54 @@ +# generic_config_updater/ — Standalone GCU wheel build context +# +# This setup.py builds the 'sonic-gcu' Python wheel, published independently +# from the main 'sonic-utilities' wheel. The gcu-standalone binary installed +# from this wheel is placed at /opt/sonic/gcu/current/bin/gcu-standalone and +# allows the GCU container to deliver fixes to generic_config_updater without +# touching the host sonic-utilities package. +# +# pytest.ini and .coveragerc in this directory configure test runs scoped to +# GCU only (see azure-pipelines.yml 'Build Python 3 wheel for GCU' step). +# +# Dependencies are intentionally absent from install_requires: the GCU venv is +# created with --system-site-packages and the wheel is installed with --no-deps, +# so all runtime dependencies (SONiC packages and third-party libs) are +# inherited from the host SONiC environment. +from setuptools import setup + + +setup( + name='sonic-gcu', + version='1.0.0', + description='GCU package for SONiC', + license='Apache 2.0', + author='SONiC Team', + author_email='linuxnetdev@microsoft.com', + url='https://github.com/Azure/sonic-utilities/generic_config_updater', + maintainer='Xincun Li', + maintainer_email='xincun.li@microsoft.com', + package_dir={'generic_config_updater': '.'}, + packages=[ + 'generic_config_updater', + ], + package_data={ + 'generic_config_updater': ['gcu_services_validator.conf.json', 'gcu_field_operation_validators.conf.json'] + }, + entry_points={ + 'console_scripts': [ + 'gcu-standalone=generic_config_updater.main:main', + ] + }, + classifiers=[ + 'Development Status :: 3 - Alpha', + 'Environment :: Console', + 'Intended Audience :: Developers', + 'Intended Audience :: Information Technology', + 'Intended Audience :: System Administrators', + 'License :: OSI Approved :: Apache Software License', + 'Natural Language :: English', + 'Operating System :: POSIX :: Linux', + 'Programming Language :: Python :: 3.7', + 'Topic :: Utilities', + ], + keywords='SONiC GCU package' +) diff --git a/tests/config_test.py b/tests/config_test.py index 7ba574b06..323bf8310 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -1817,8 +1817,9 @@ def setUp(self): self.any_checkpoint_name = "any_checkpoint_name" self.any_checkpoints_list = ["checkpoint1", "checkpoint2", "checkpoint3"] self.any_checkpoints_list_as_text = json.dumps(self.any_checkpoints_list, indent=4) + self.any_checkpoints_list_with_time_as_text = json.dumps(self.any_checkpoints_list_with_time, indent=4) - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch__no_params__get_required_params_error_msg(self): # Arrange unexpected_exit_code = 0 @@ -1831,7 +1832,7 @@ def test_apply_patch__no_params__get_required_params_error_msg(self): self.assertNotEqual(unexpected_exit_code, result.exit_code) self.assertTrue(expected_output in result.output) - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch__help__gets_help_msg(self): # Arrange expected_exit_code = 0 @@ -1844,14 +1845,18 @@ def test_apply_patch__help__gets_help_msg(self): self.assertEqual(expected_exit_code, result.exit_code) self.assertTrue(expected_output in result.output) - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch__only_required_params__default_values_used_for_optional_params(self): # Arrange expected_exit_code = 0 expected_output = "Patch applied successfully" 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('generic_config_updater.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_patch_as_text)): # Act @@ -1863,7 +1868,11 @@ def test_apply_patch__only_required_params__default_values_used_for_optional_par mock_generic_updater.apply_patch.assert_called_once() mock_generic_updater.apply_patch.assert_has_calls([expected_call_with_default_values]) - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch__all_optional_params_non_default__non_default_values_used(self): # Arrange expected_exit_code = 0 @@ -1872,7 +1881,7 @@ def test_apply_patch__all_optional_params_non_default__non_default_values_used(s 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('generic_config_updater.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_patch_as_text)): # Act @@ -1893,14 +1902,18 @@ def test_apply_patch__all_optional_params_non_default__non_default_values_used(s mock_generic_updater.apply_patch.assert_called_once() mock_generic_updater.apply_patch.assert_has_calls([expected_call_with_non_default_values]) - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch__exception_thrown__error_displayed_error_code_returned(self): # Arrange unexpected_exit_code = 0 any_error_message = "any_error_message" mock_generic_updater = mock.Mock() mock_generic_updater.apply_patch.side_effect = Exception(any_error_message) - with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_patch_as_text)): # Act @@ -1929,13 +1942,17 @@ def test_apply_patch__optional_parameters_passed_correctly(self): ["--ignore-path", "/ANY_TABLE"], mock.call(self.any_patch, ConfigFormat.CONFIGDB, False, False, False, ("/ANY_TABLE",))) - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def validate_apply_patch_optional_parameter(self, param_args, expected_call): # Arrange expected_exit_code = 0 expected_output = "Patch applied successfully" mock_generic_updater = mock.Mock() - with mock.patch('config.main.GenericUpdater', return_value=mock_generic_updater): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_generic_updater): with mock.patch('builtins.open', mock.mock_open(read_data=self.any_patch_as_text)): # Act @@ -1949,6 +1966,150 @@ def validate_apply_patch_optional_parameter(self, param_args, expected_call): mock_generic_updater.apply_patch.assert_called_once() mock_generic_updater.apply_patch.assert_has_calls([expected_call]) + def test_filter_duplicate_patch_operations_basic(self): + from generic_config_updater.main import filter_duplicate_patch_operations + # Patch tries to add duplicate port to ACL_TABLE leaf-list + patch_ops = [ + {"op": "add", "path": "/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet1"}, + {"op": "add", "path": "/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet2"}, + {"op": "add", "path": "/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet3"} + ] + config = { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet1", "Ethernet2"] + } + } + } + filtered_patch_ops = filter_duplicate_patch_operations(patch_ops, json.dumps(config)) + # Only the non-duplicate add ops should remain + self.assertEqual(len(filtered_patch_ops), 1, "Only Ethernet3 add op should remain") + self.assertEqual(filtered_patch_ops[0]['value'], "Ethernet3", "Only Ethernet3 add op should remain") + + def test_filter_duplicate_patch_operations_no_duplicates(self): + from generic_config_updater.main import filter_duplicate_patch_operations + # Patch does not contain any duplicate ops + patch_ops = [ + {"op": "add", "path": "/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet3"}, + {"op": "remove", "path": "/ACL_TABLE/MY_ACL_TABLE/ports/0"}, + {"op": "replace", "path": "/ACL_TABLE/MY_ACL_TABLE/description", "value": "New description"} + ] + config = { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet1", "Ethernet2"], + "description": "Old description" + } + } + } + filtered_patch_ops = filter_duplicate_patch_operations(patch_ops, json.dumps(config)) + # All ops should remain as there are no duplicates + self.assertEqual(len(filtered_patch_ops), len(patch_ops), "All patch should remain as no duplicates") + self.assertEqual(filtered_patch_ops, patch_ops, "Filtered ops should match original ops") + + def test_filter_duplicate_patch_operations_non_list_field(self): + from generic_config_updater.main import filter_duplicate_patch_operations + # Patch tries to add duplicate entries to a non-list field + patch_ops = [ + {"op": "add", "path": "/PORT/Ethernet0/description", "value": "Desc1"}, + {"op": "add", "path": "/PORT/Ethernet0/description", "value": "Desc2"} + ] + config = { + "PORT": { + "Ethernet0": { + "description": "Existing description" + } + } + } + filtered_patch_ops = filter_duplicate_patch_operations(patch_ops, json.dumps(config)) + # Both ops should remain as description is not a list field + self.assertEqual(len(filtered_patch_ops), len(patch_ops), "Both add ops should remain for non-list field") + self.assertEqual(filtered_patch_ops, patch_ops, "Filtered ops should match original ops") + + def test_filter_duplicate_patch_operations_empty_config(self): + from generic_config_updater.main import filter_duplicate_patch_operations + patch_ops = [ + {"op": "add", "path": "/PORT/Ethernet0/allowed_vlans/-", "value": "100"}, + {"op": "add", "path": "/PORT/Ethernet0/allowed_vlans/-", "value": "200"} + ] + config = { + "PORT": { + "Ethernet0": { + "allowed_vlans": [] + } + } + } + filtered_patch_ops = filter_duplicate_patch_operations(patch_ops, json.dumps(config)) + # All ops should remain as config is empty and has no existing entries + self.assertEqual(len(filtered_patch_ops), len(patch_ops), "All add ops should remain for empty list") + self.assertEqual(filtered_patch_ops, patch_ops, "Filtered ops should match original ops") + + def test_append_emptytables_if_required_basic_config(self): + from generic_config_updater.main import append_emptytables_if_required + + patch_ops = [ + {"op": "add", "path": "/ACL_TABLE2/ports", "value": ["Ethernet1", "Ethernet2"]} + ] + config = { + "ACL_TABLE": { + "ports": ["Ethernet3"] + } + } + updated_patch_ops = append_emptytables_if_required(patch_ops, json.dumps(config)) + assert len(updated_patch_ops) == 2, "Patch should have 2 operations after appending empty tables" + assert updated_patch_ops[0]['path'] == "/ACL_TABLE2", "First op should create ACL_TABLE2" + assert updated_patch_ops[0]['op'] == "add", "First op should be an add operation" + assert updated_patch_ops[1] == patch_ops[0], "Second op should be the original add operation" + + def test_append_emptytables_if_required_no_action_needed(self): + from generic_config_updater.main import append_emptytables_if_required + patch_ops = [ + {"op": "add", "path": "/ACL_TABLE/ports", "value": ["Ethernet1", "Ethernet2"]} + ] + config = { + "ACL_TABLE": { + "ports": ["Ethernet3"] + } + } + updated_patch_ops = append_emptytables_if_required(patch_ops, json.dumps(config)) + assert len(updated_patch_ops) == 1, "Patch should remain unchanged with 1 operation" + assert updated_patch_ops[0] == patch_ops[0], "Patch operation should remain unchanged" + + def test_append_emptytables_if_required_multiple_tables(self): + from generic_config_updater.main import append_emptytables_if_required + + patch_ops = [ + {"op": "add", "path": "/TABLE1/field", "value": "value1"}, + {"op": "add", "path": "/TABLE2/field", "value": "value2"} + ] + config = {} + updated_patch_ops = append_emptytables_if_required(patch_ops, json.dumps(config)) + assert len(updated_patch_ops) == 4, "Patch should have 4 operations after appending empty tables" + assert updated_patch_ops[0]['path'] == "/TABLE1", "First op should create TABLE1" + assert updated_patch_ops[1] == patch_ops[0], "Second op should be the original TABLE1 add operation" + assert updated_patch_ops[2]['path'] == "/TABLE2", "Third op should create TABLE2" + assert updated_patch_ops[3] == patch_ops[1], "Fourth op should be the original TABLE2 add operation" + + # ------------------------------------------------------------------ + # print_dry_run_message + # ------------------------------------------------------------------ + + def test_print_dry_run_message_dry_run_true_prints_banner(self): + """print_dry_run_message with dry_run=True should echo a banner.""" + from config.main import print_dry_run_message + result = self.runner.invoke( + click.command()(lambda: print_dry_run_message(True)) + ) + self.assertIn("DRY RUN", result.output) + + def test_print_dry_run_message_dry_run_false_no_output(self): + """print_dry_run_message with dry_run=False should produce no output.""" + from config.main import print_dry_run_message + result = self.runner.invoke( + click.command()(lambda: print_dry_run_message(False)) + ) + self.assertEqual(result.output.strip(), "") + def test_replace__no_params__get_required_params_error_msg(self): # Arrange unexpected_exit_code = 0 @@ -3497,12 +3658,16 @@ def setUp(self): self.all_config["asic1"] = data self.all_config["asic1"]["bgpraw"] = "" - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch_multiasic(self): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() print("Multi ASIC: {}".format(multi_asic.is_multi_asic())) @@ -3517,12 +3682,16 @@ def test_apply_patch_multiasic(self): # Verify mocked_open was called as expected mocked_open.assert_called_with(self.patch_file_path, 'r') - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch_dryrun_multiasic(self): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() # Mock ConfigDBConnector to ensure it's not called during dry-run @@ -3552,13 +3721,17 @@ def test_apply_patch_dryrun_multiasic(self): # Ensure ConfigDBConnector was never instantiated or called mock_config_db_connector.assert_not_called() - @patch('config.main.validate_patch', mock.Mock(return_value=True)) - @patch('config.main.concurrent.futures.wait', autospec=True) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.concurrent.futures.wait', autospec=True) def test_apply_patch_dryrun_parallel_multiasic(self, MockThreadPoolWait): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() # Mock ConfigDBConnector to ensure it's not called during dry-run @@ -3592,13 +3765,17 @@ def test_apply_patch_dryrun_parallel_multiasic(self, MockThreadPoolWait): # Ensure ConfigDBConnector was never instantiated or called mock_config_db_connector.assert_not_called() - @patch('config.main.validate_patch', mock.Mock(return_value=True)) - @patch('config.main.concurrent.futures.wait', autospec=True) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.concurrent.futures.wait', autospec=True) def test_apply_patch_check_running_in_parallel_multiasic(self, MockThreadPoolWait): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() # Mock ConfigDBConnector to ensure it's not called during dry-run @@ -3631,13 +3808,17 @@ def test_apply_patch_check_running_in_parallel_multiasic(self, MockThreadPoolWai # Ensure ConfigDBConnector was never instantiated or called mock_config_db_connector.assert_not_called() - @patch('config.main.validate_patch', mock.Mock(return_value=True)) - @patch('config.main.apply_patch_wrapper') + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) + @patch('generic_config_updater.main._apply_patch_wrapper') def test_apply_patch_check_apply_call_parallel_multiasic(self, mock_apply_patch): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() # Mock ConfigDBConnector to ensure it's not called during dry-run @@ -3672,13 +3853,17 @@ def test_apply_patch_check_apply_call_parallel_multiasic(self, mock_apply_patch) # Ensure ConfigDBConnector was never instantiated or called mock_config_db_connector.assert_not_called() - @patch('config.main.validate_patch', mock.Mock(return_value=True)) - @patch('config.main.concurrent.futures.wait', autospec=True) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.concurrent.futures.wait', autospec=True) def test_apply_patch_check_running_in_not_parallel_multiasic(self, MockThreadPoolWait): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() # Mock ConfigDBConnector to ensure it's not called during dry-run @@ -3710,12 +3895,16 @@ def test_apply_patch_check_running_in_not_parallel_multiasic(self, MockThreadPoo # Ensure ConfigDBConnector was never instantiated or called mock_config_db_connector.assert_not_called() - @patch('config.main.validate_patch', mock.Mock(return_value=True)) + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def test_apply_patch_parallel_with_error_multiasic(self): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() # Mock ConfigDBConnector to ensure it's not called during dry-run @@ -3746,8 +3935,261 @@ def test_apply_patch_parallel_with_error_multiasic(self): # Ensure ConfigDBConnector was never instantiated or called mock_config_db_connector.assert_not_called() - @patch('config.main.subprocess.Popen') - @patch('config.main.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) + def test_filter_duplicate_patch_operations_basic_multiasic(self): + from generic_config_updater.main import filter_duplicate_patch_operations + import jsonpatch + # Multi-ASIC config: each ASIC has its own ACL_TABLE + config = { + "localhost": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet1", "Ethernet2"] + } + } + }, + "asic0": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet3", "Ethernet4"] + } + } + }, + "asic1": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet5", "Ethernet6"] + } + } + } + } + # Patch tries to add duplicate ports to each ASIC's ACL_TABLE leaf-list + patch_ops = [ + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet1"}, + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet2"}, + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet3"}, + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet4"}, + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet5"}, + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet6"} + ] + patch = jsonpatch.JsonPatch(patch_ops) + filtered_patch = filter_duplicate_patch_operations(patch, config) + filtered_ops = list(filtered_patch) + self.assertEqual(len(filtered_ops), 0, "All adds are duplicates, should be filtered out in multi-asic config") + + def test_filter_duplicate_patch_operations_no_duplicates_multiasic(self): + from generic_config_updater.main import filter_duplicate_patch_operations + import jsonpatch + # Multi-ASIC config: each ASIC has its own ACL_TABLE + config = { + "localhost": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet1", "Ethernet2"] + } + } + }, + "asic0": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet3", "Ethernet4"] + } + } + }, + "asic1": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet5", "Ethernet6"] + } + } + } + } + # Patch tries to add new ports to each ASIC's ACL_TABLE leaf-list + patch_ops = [ + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet7"}, + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet8"}, + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet9"}, + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet10"}, + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet11"}, + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet12"} + ] + patch = jsonpatch.JsonPatch(patch_ops) + filtered_patch = filter_duplicate_patch_operations(patch, config) + filtered_ops = list(filtered_patch) + self.assertEqual( + len(filtered_ops), + len(patch_ops), + "No adds are duplicates, none should be filtered out in multi-asic config" + ) + + def test_filter_duplicate_patch_operations_mixed_multiasic(self): + from generic_config_updater.main import filter_duplicate_patch_operations + import jsonpatch + # Multi-ASIC config: each ASIC has its own ACL_TABLE + config = { + "localhost": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet1", "Ethernet2"] + } + } + }, + "asic0": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet3", "Ethernet4"] + } + } + }, + "asic1": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": ["Ethernet5", "Ethernet6"] + } + } + } + } + # Patch tries to add some duplicate and some new ports to each ASIC's ACL_TABLE leaf-list + patch_ops = [ + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet1"}, # duplicate + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet8"}, # new + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet3"}, # duplicate + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet10"}, # new + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet5"}, # duplicate + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet12"} # new + ] + patch = jsonpatch.JsonPatch(patch_ops) + filtered_patch = filter_duplicate_patch_operations(patch, config) + filtered_ops = list(filtered_patch) + self.assertEqual( + len(filtered_ops), + 3, + "Three adds are duplicates, three are new and should remain in multi-asic config" + ) + + def test_filter_duplicate_patch_operations_empty_config_multiasic(self): + from generic_config_updater.main import filter_duplicate_patch_operations + import jsonpatch + config = { + "localhost": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": [] + } + } + }, + "asic0": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": [] + } + } + }, + "asic1": { + "ACL_TABLE": { + "MY_ACL_TABLE": { + "ports": [] + } + } + } + } + # Patch tries to add ports to each ASIC's ACL_TABLE leaf-list + patch_ops = [ + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet1"}, + {"op": "add", "path": "/localhost/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet2"}, + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet3"}, + {"op": "add", "path": "/asic0/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet4"}, + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet5"}, + {"op": "add", "path": "/asic1/ACL_TABLE/MY_ACL_TABLE/ports/-", "value": "Ethernet6"} + ] + patch = jsonpatch.JsonPatch(patch_ops) + filtered_patch = filter_duplicate_patch_operations(patch, config) + filtered_ops = list(filtered_patch) + self.assertEqual( + len(filtered_ops), + len(patch_ops), + "No adds are duplicates in empty list, " + "none should be filtered out in multi-asic config" + ) + + def test_test_append_emptytables_if_required_basic_config_multiasic(self): + from generic_config_updater.main import append_emptytables_if_required + # Multi-ASIC config: each ASIC has its own PORT table + config = { + "localhost": { + "PORT": { + "Ethernet0": { + "mtu": "9100" + } + } + }, + "asic0": { + "PORT": { + "Ethernet1": { + "mtu": "9100" + } + } + }, + "asic1": { + "PORT": { + "Ethernet2": { + "mtu": "9100" + } + } + } + } + # Patch does not include PORT table for asic1 + patch_ops = [ + {"op": "add", "path": "/localhost/BGP_NEIGHBOR/ARISTA01T1", "value": "10.0.0.1"}, + {"op": "add", "path": "/asic0/BGP_NEIGHBOR/ARISTA02T1", "value": "10.0.0.2"}, + {"op": "add", "path": "/asic1/BGP_NEIGHBOR/ARISTA02T1", "value": "10.0.0.3"} + ] + updated_patch = append_emptytables_if_required(patch_ops, config) + updated_patch_list = list(updated_patch) + assert len(updated_patch_list) == 6, "BGP_NEIGHBOR table for each namespace should be added to the patch" + assert updated_patch_list[0] == {"op": "add", "path": "/localhost/BGP_NEIGHBOR", "value": {}} + assert updated_patch_list[2] == {"op": "add", "path": "/asic0/BGP_NEIGHBOR", "value": {}} + assert updated_patch_list[4] == {"op": "add", "path": "/asic1/BGP_NEIGHBOR", "value": {}} + assert updated_patch_list[1] == patch_ops[0] + assert updated_patch_list[3] == patch_ops[1] + assert updated_patch_list[5] == patch_ops[2] + + def test_test_append_emptytables_if_required_no_additional_tables_multiasic(self): + from generic_config_updater.main import append_emptytables_if_required + # Multi-ASIC config: each ASIC has its own PORT table + config = { + "localhost": { + "PORT": { + "Ethernet0": { + "mtu": "9100" + } + } + }, + "asic0": { + "PORT": { + "Ethernet1": { + "mtu": "9100" + } + } + }, + "asic1": { + "PORT": { + "Ethernet2": { + "mtu": "9100" + } + } + } + } + # Patch already includes PORT table for each namespace + patch_ops = [ + {"op": "add", "path": "/localhost/PORT/Ethernet0/mtu", "value": "9200"}, + {"op": "add", "path": "/asic0/PORT/Ethernet1/mtu", "value": "9200"}, + {"op": "add", "path": "/asic1/PORT/Ethernet2/mtu", "value": "9200"} + ] + updated_patch = append_emptytables_if_required(patch_ops, config) + assert len(updated_patch) == len(patch_ops), "No additional tables should be added to the patch" + + @patch('generic_config_updater.main.subprocess.Popen') + @patch('sonic_yang_cfg_generator.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) def test_apply_patch_validate_patch_multiasic(self, mock_subprocess_popen): mock_instance = MagicMock() mock_instance.communicate.return_value = (json.dumps(self.all_config), 0) @@ -3756,7 +4198,7 @@ def test_apply_patch_validate_patch_multiasic(self, mock_subprocess_popen): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() print("Multi ASIC: {}".format(multi_asic.is_multi_asic())) @@ -3773,8 +4215,8 @@ def test_apply_patch_validate_patch_multiasic(self, mock_subprocess_popen): # Verify mocked_open was called as expected mocked_open.assert_called_with(self.patch_file_path, 'r') - @patch('config.main.subprocess.Popen') - @patch('config.main.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.subprocess.Popen') + @patch('sonic_yang_cfg_generator.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) def test_apply_patch_validate_patch_with_badpath_multiasic(self, mock_subprocess_popen): mock_instance = MagicMock() mock_instance.communicate.return_value = (json.dumps(self.all_config), 0) @@ -3793,7 +4235,7 @@ def test_apply_patch_validate_patch_with_badpath_multiasic(self, mock_subprocess # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(bad_patch)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() print("Multi ASIC: {}".format(multi_asic.is_multi_asic())) @@ -3810,8 +4252,8 @@ def test_apply_patch_validate_patch_with_badpath_multiasic(self, mock_subprocess # Verify mocked_open was called as expected mocked_open.assert_called_with(self.patch_file_path, 'r') - @patch('config.main.subprocess.Popen') - @patch('config.main.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.subprocess.Popen') + @patch('sonic_yang_cfg_generator.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) def test_apply_patch_parallel_badpath_multiasic(self, mock_subprocess_popen): mock_instance = MagicMock() mock_instance.communicate.return_value = (json.dumps(self.all_config), 0) @@ -3830,7 +4272,7 @@ def test_apply_patch_parallel_badpath_multiasic(self, mock_subprocess_popen): # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(bad_patch)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() print("Multi ASIC: {}".format(multi_asic.is_multi_asic())) @@ -3848,8 +4290,8 @@ def test_apply_patch_parallel_badpath_multiasic(self, mock_subprocess_popen): # Verify mocked_open was called as expected mocked_open.assert_called_with(self.patch_file_path, 'r') - @patch('config.main.subprocess.Popen') - @patch('config.main.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) + @patch('generic_config_updater.main.subprocess.Popen') + @patch('sonic_yang_cfg_generator.SonicYangCfgDbGenerator.validate_config_db_json', mock.Mock(return_value=True)) def test_apply_patch_validate_patch_with_wrong_fetch_config(self, mock_subprocess_popen): mock_instance = MagicMock() mock_instance.communicate.return_value = (json.dumps(self.all_config), 2) @@ -3858,7 +4300,7 @@ def test_apply_patch_validate_patch_with_wrong_fetch_config(self, mock_subproces # Mock open to simulate file reading with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: # Mock GenericUpdater to avoid actual patch application - with patch('config.main.GenericUpdater') as mock_generic_updater: + with patch('generic_config_updater.main.GenericUpdater') as mock_generic_updater: mock_generic_updater.return_value.apply_patch = MagicMock() print("Multi ASIC: {}".format(multi_asic.is_multi_asic())) @@ -4009,7 +4451,11 @@ def test_delete_checkpoint_multiasic(self): self.assertEqual(result.exit_code, 0, "Command should succeed") self.assertIn("Checkpoint deleted successfully.", result.output) - @classmethod + @patch('subprocess.Popen', mock.Mock(return_value=mock.Mock( + communicate=mock.Mock(return_value=('{"some": "config"}', None)), + returncode=0 + ))) + @patch('generic_config_updater.main.validate_patch', mock.Mock(return_value=True)) def teardown_class(cls): print("TEARDOWN") os.environ['UTILITIES_UNIT_TESTING'] = "0" diff --git a/tests/generic_config_updater/files/patch_sorter_test_success.json b/tests/generic_config_updater/files/patch_sorter_test_success.json index 4d8cb8a5a..896cdbe05 100644 --- a/tests/generic_config_updater/files/patch_sorter_test_success.json +++ b/tests/generic_config_updater/files/patch_sorter_test_success.json @@ -537,9 +537,13 @@ "expected_changes": [ [ { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/ports/0", - "value": "Ethernet0" + "op": "replace", + "path": "/ACL_TABLE/EVERFLOWV6/ports", + "value": [ + "Ethernet0", + "Ethernet4", + "Ethernet8" + ] } ] ] @@ -867,23 +871,14 @@ ], [ { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1", - "value": "Ethernet1" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/2", - "value": "Ethernet2" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/3", - "value": "Ethernet3" + "op": "replace", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0", + "Ethernet1", + "Ethernet2", + "Ethernet3" + ] } ] ] @@ -1538,15 +1533,12 @@ "expected_changes": [ [ { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOWV6/ports/0" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/ports/0", - "value": "Ethernet0" + "op": "replace", + "path": "/ACL_TABLE/EVERFLOWV6/ports", + "value": [ + "Ethernet0", + "Ethernet8" + ] } ] ] @@ -1753,8 +1745,13 @@ "expected_changes": [ [ { - "op": "remove", - "path": "/VLAN/Vlan1000/dhcp_servers/0" + "op": "replace", + "path": "/VLAN/Vlan1000/dhcp_servers", + "value": [ + "192.0.0.2", + "192.0.0.3", + "192.0.0.4" + ] } ] ] @@ -2383,8 +2380,8 @@ "value": { "Ethernet0": { "alias": "Eth1", - "description": "Ethernet0 100G link", "lanes": "67", + "description": "Ethernet0 100G link", "speed": "100000" } } @@ -3879,6 +3876,28 @@ } ], "expected_changes": [ + [ + { + "op": "replace", + "path": "/ACL_TABLE/EVERFLOW/ports", + "value": [ + "Ethernet64", + "Ethernet68", + "Ethernet72" + ] + } + ], + [ + { + "op": "replace", + "path": "/ACL_TABLE/EVERFLOWV6/ports", + "value": [ + "Ethernet64", + "Ethernet68", + "Ethernet72" + ] + } + ], [ { "op": "add", @@ -4222,20 +4241,6 @@ "value": "up" } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOW/ports/0", - "value": "Ethernet64" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/ports/0", - "value": "Ethernet64" - } - ], [ { "op": "add", @@ -5412,6 +5417,26 @@ } ], "expected_changes": [ + [ + { + "op": "replace", + "path": "/ACL_TABLE/EVERFLOW/ports", + "value": [ + "Ethernet68", + "Ethernet72" + ] + } + ], + [ + { + "op": "replace", + "path": "/ACL_TABLE/EVERFLOWV6/ports", + "value": [ + "Ethernet68", + "Ethernet72" + ] + } + ], [ { "op": "remove", @@ -5630,18 +5655,6 @@ "path": "/BGP_NEIGHBOR/fc00::a/admin_status" } ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOW/ports/0" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOWV6/ports/0" - } - ], [ { "op": "remove", diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index a2df2a148..fb26c5261 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -221,6 +221,92 @@ def test_validate_config_db_config__invalid_config__returns_false(self): self.assertEqual(expected, actual) self.assertIsNotNone(error) + def test_validate_config_db_config__same_config_called_twice__loadData_called_once(self): + """Cache hit: second call with same config must not call loadData again.""" + config_wrapper = gu_common.ConfigWrapper() + mock_sy = MagicMock() + mock_sy.loadData = MagicMock() + mock_sy.loadYangModel = MagicMock() + config_wrapper.sonic_yang_with_loaded_models = mock_sy + + config = {"ACL_TABLE": {}} + + config_wrapper.validate_config_db_config(config) + config_wrapper.validate_config_db_config(config) + + # loadData should only be called once — second call hits the result cache + mock_sy.loadData.assert_called_once() + + def test_validate_config_db_config__different_configs__loadData_called_each_time(self): + """Cache miss: different configs must each call loadData.""" + config_wrapper = gu_common.ConfigWrapper() + mock_sy = MagicMock() + mock_sy.loadData = MagicMock() + config_wrapper.sonic_yang_with_loaded_models = mock_sy + + config_a = {"ACL_TABLE": {"rule1": {}}} + config_b = {"ACL_TABLE": {"rule2": {}}} + + config_wrapper.validate_config_db_config(config_a) + config_wrapper.validate_config_db_config(config_b) + + self.assertEqual(2, mock_sy.loadData.call_count) + + def test_find_ref_paths__after_validate_same_config__loadData_skipped(self): + """ + Core optimization: if validate_config_db_config already loaded config into sy, + find_ref_paths with the same config must skip loadData entirely. + """ + config_wrapper = gu_common.ConfigWrapper() + mock_sy = MagicMock() + mock_sy.loadData = MagicMock() + mock_sy.root = MagicMock() + mock_sy.root.find_path = MagicMock(return_value=MagicMock(data=MagicMock(return_value=[]))) + config_wrapper.sonic_yang_with_loaded_models = mock_sy + + path_addressing = gu_common.PathAddressing(config_wrapper) + config = {"ACL_TABLE": {}} + + # First: validate loads the config into sy and sets _currently_loaded_hash + config_wrapper.validate_config_db_config(config) + self.assertIsNotNone(config_wrapper._currently_loaded_hash, + "validate_config_db_config should have set _currently_loaded_hash") + load_count_after_validate = mock_sy.loadData.call_count + + # Second: find_ref_paths with same config should NOT call loadData again + path_addressing.find_ref_paths("/ACL_TABLE", config, reload_config=True) + load_count_after_find = mock_sy.loadData.call_count + + self.assertEqual(load_count_after_validate, load_count_after_find, + "find_ref_paths should skip loadData when validate already loaded same config") + + def test_find_ref_paths__after_validate_different_config__loadData_called(self): + """ + Cache miss in find_ref_paths: if validate loaded config_A, calling find_ref_paths + with config_B must still call loadData. + """ + config_wrapper = gu_common.ConfigWrapper() + mock_sy = MagicMock() + mock_sy.loadData = MagicMock() + mock_sy.root = MagicMock() + mock_sy.root.find_path = MagicMock(return_value=MagicMock(data=MagicMock(return_value=[]))) + config_wrapper.sonic_yang_with_loaded_models = mock_sy + + path_addressing = gu_common.PathAddressing(config_wrapper) + config_a = {"ACL_TABLE": {"rule1": {}}} + config_b = {"ACL_TABLE": {"rule2": {}}} + + config_wrapper.validate_config_db_config(config_a) + self.assertIsNotNone(config_wrapper._currently_loaded_hash, + "validate_config_db_config should have set _currently_loaded_hash") + load_count_after_validate = mock_sy.loadData.call_count + + path_addressing.find_ref_paths("/ACL_TABLE", config_b, reload_config=True) + load_count_after_find = mock_sy.loadData.call_count + + self.assertEqual(load_count_after_find, load_count_after_validate + 1, + "find_ref_paths should call loadData exactly once when config differs") + def test_validate_bgp_peer_group__valid_non_intersecting_ip_ranges__returns_true(self): # Arrange config_wrapper = gu_common.ConfigWrapper() diff --git a/tests/generic_config_updater/main_test.py b/tests/generic_config_updater/main_test.py new file mode 100644 index 000000000..3a61e2504 --- /dev/null +++ b/tests/generic_config_updater/main_test.py @@ -0,0 +1,965 @@ +import io +import json +import os +import subprocess +import sys +import unittest +from argparse import Namespace +from unittest import mock + +# Make sure the repo root is on the path +_TEST_DIR = os.path.dirname(os.path.abspath(__file__)) +_ROOT_DIR = os.path.dirname(os.path.dirname(_TEST_DIR)) +sys.path.insert(0, _ROOT_DIR) + +from generic_config_updater.generic_updater import ConfigFormat # noqa: E402 +from generic_config_updater.gu_common import GenericConfigUpdaterError # noqa: E402 +import generic_config_updater.main as gcu_main # noqa: E402 + + +# --------------------------------------------------------------------------- +# validate_patch_format +# --------------------------------------------------------------------------- + +class TestValidatePatchFormat(unittest.TestCase): + + def test_valid_patch_returns_true(self): + patch = [{"op": "add", "path": "/TABLE/key", "value": "v"}] + self.assertTrue(gcu_main.validate_patch_format(patch)) + + def test_valid_all_ops(self): + for op in ("add", "remove", "replace", "move", "copy", "test"): + patch = [{"op": op, "path": "/X"}] + self.assertTrue(gcu_main.validate_patch_format(patch)) + + def test_empty_list_is_valid(self): + self.assertTrue(gcu_main.validate_patch_format([])) + + def test_not_a_list_returns_false(self): + self.assertFalse(gcu_main.validate_patch_format({"op": "add", "path": "/X"})) + + def test_item_not_dict_returns_false(self): + self.assertFalse(gcu_main.validate_patch_format(["not_a_dict"])) + + def test_missing_op_returns_false(self): + self.assertFalse(gcu_main.validate_patch_format([{"path": "/X"}])) + + def test_missing_path_returns_false(self): + self.assertFalse(gcu_main.validate_patch_format([{"op": "add"}])) + + def test_invalid_op_returns_false(self): + self.assertFalse( + gcu_main.validate_patch_format([{"op": "invalid_op", "path": "/X"}]) + ) + + def test_none_input_returns_false(self): + self.assertFalse(gcu_main.validate_patch_format(None)) + + def test_multiple_valid_changes(self): + patch = [ + {"op": "add", "path": "/A", "value": {}}, + {"op": "remove", "path": "/B"}, + {"op": "replace", "path": "/C", "value": 1}, + ] + self.assertTrue(gcu_main.validate_patch_format(patch)) + + def test_one_invalid_in_list_returns_false(self): + patch = [ + {"op": "add", "path": "/A", "value": {}}, + {"op": "BAD", "path": "/B"}, + ] + self.assertFalse(gcu_main.validate_patch_format(patch)) + + +# --------------------------------------------------------------------------- +# get_all_running_config +# --------------------------------------------------------------------------- + +class TestGetAllRunningConfig(unittest.TestCase): + + def _make_popen(self, stdout, returncode): + proc = mock.Mock() + proc.communicate.return_value = (stdout, None) + proc.returncode = returncode + return proc + + def test_success_returns_config_string(self): + cfg = '{"PORT": {}}' + with mock.patch('subprocess.Popen', return_value=self._make_popen(cfg, 0)): + result = gcu_main.get_all_running_config() + self.assertEqual(result, cfg) + + def test_nonzero_returncode_raises(self): + with mock.patch('subprocess.Popen', + return_value=self._make_popen('', 1)): + with self.assertRaises(GenericConfigUpdaterError): + gcu_main.get_all_running_config() + + +# --------------------------------------------------------------------------- +# filter_duplicate_patch_operations +# --------------------------------------------------------------------------- + +class TestFilterDuplicatePatchOperations(unittest.TestCase): + + def test_no_leaf_list_ops_returned_unchanged(self): + patch_ops = [{"op": "add", "path": "/TABLE/key", "value": "v"}] + config = {} + result = gcu_main.filter_duplicate_patch_operations(patch_ops, json.dumps(config)) + self.assertEqual(result, patch_ops) + + def test_removes_duplicate_leaf_list_add(self): + config = {"ACL_TABLE": {"MY_ACL": {"ports": ["Eth0", "Eth1"]}}} + patch_ops = [ + {"op": "add", "path": "/ACL_TABLE/MY_ACL/ports/-", "value": "Eth0"}, + {"op": "add", "path": "/ACL_TABLE/MY_ACL/ports/-", "value": "Eth2"}, + ] + result = gcu_main.filter_duplicate_patch_operations(patch_ops, json.dumps(config)) + paths_values = [(op["path"], op["value"]) for op in result] + self.assertNotIn(("/ACL_TABLE/MY_ACL/ports/-", "Eth0"), paths_values) + self.assertIn(("/ACL_TABLE/MY_ACL/ports/-", "Eth2"), paths_values) + + def test_no_duplicates_nothing_removed(self): + config = {"ACL_TABLE": {"MY_ACL": {"ports": []}}} + patch_ops = [ + {"op": "add", "path": "/ACL_TABLE/MY_ACL/ports/-", "value": "Eth0"}, + {"op": "add", "path": "/ACL_TABLE/MY_ACL/ports/-", "value": "Eth1"}, + ] + result = gcu_main.filter_duplicate_patch_operations(patch_ops, json.dumps(config)) + self.assertEqual(len(result), 2) + + def test_accepts_dict_config(self): + config = {"ACL_TABLE": {"MY_ACL": {"ports": ["Eth0"]}}} + patch_ops = [ + {"op": "add", "path": "/ACL_TABLE/MY_ACL/ports/-", "value": "Eth0"}, + ] + result = gcu_main.filter_duplicate_patch_operations(patch_ops, config) + self.assertEqual(len(result), 0) + + +# --------------------------------------------------------------------------- +# append_emptytables_if_required +# --------------------------------------------------------------------------- + +class TestAppendEmptyTablesIfRequired(unittest.TestCase): + + def test_no_missing_tables_returned_unchanged(self): + config = {"TABLE1": {}} + patch_ops = [{"op": "add", "path": "/TABLE1/key", "value": "v"}] + result = gcu_main.append_emptytables_if_required(patch_ops, json.dumps(config)) + self.assertEqual(result, patch_ops) + + def test_missing_table_prepended(self): + config = {} + patch_ops = [{"op": "add", "path": "/TABLE1/key", "value": "v"}] + result = gcu_main.append_emptytables_if_required(patch_ops, json.dumps(config)) + self.assertEqual(result[0], {"op": "add", "path": "/TABLE1", "value": {}}) + self.assertEqual(result[1], patch_ops[0]) + + def test_multiple_missing_tables(self): + config = {} + patch_ops = [ + {"op": "add", "path": "/TABLE1/field", "value": "v1"}, + {"op": "add", "path": "/TABLE2/field", "value": "v2"}, + ] + result = gcu_main.append_emptytables_if_required(patch_ops, json.dumps(config)) + created_paths = [op["path"] for op in result if op.get("value") == {}] + self.assertIn("/TABLE1", created_paths) + self.assertIn("/TABLE2", created_paths) + + def test_accepts_dict_config(self): + config = {} + patch_ops = [{"op": "add", "path": "/TABLE1/key", "value": "v"}] + result = gcu_main.append_emptytables_if_required(patch_ops, config) + self.assertEqual(result[0]["path"], "/TABLE1") + + def test_op_without_path_skipped(self): + config = {} + patch_ops = [{"op": "add", "value": "v"}] + result = gcu_main.append_emptytables_if_required(patch_ops, config) + # Should not crash, just return the ops with no empty table inserted + self.assertEqual(len(result), 1) + + def test_empty_path_parts_skipped(self): + config = {} + patch_ops = [{"op": "add", "path": "/", "value": "v"}] + # Should not raise + result = gcu_main.append_emptytables_if_required(patch_ops, config) + self.assertIsInstance(result, list) + + def test_asic_scoped_table_path(self): + """Paths starting with asic0/TABLE should resolve two-level pointer.""" + config = {"asic0": {}} + patch_ops = [{"op": "add", "path": "/asic0/NEW_TABLE/key", "value": "v"}] + result = gcu_main.append_emptytables_if_required(patch_ops, json.dumps(config)) + created_paths = [op["path"] for op in result if op.get("value") == {}] + self.assertIn("/asic0/NEW_TABLE", created_paths) + + +# --------------------------------------------------------------------------- +# validate_patch +# --------------------------------------------------------------------------- + +class TestValidatePatch(unittest.TestCase): + + def _simple_ops(self): + return [{"op": "add", "path": "/TABLE/key", "value": "v"}] + + def test_returns_true_when_yang_not_available(self): + """When sonic_yang_cfg_generator is not importable, validation is skipped.""" + with mock.patch.dict('sys.modules', {'sonic_yang_cfg_generator': None}): + result = gcu_main.validate_patch([], json.dumps({})) + self.assertTrue(result) + + def test_returns_true_on_valid_config(self): + mock_generator = mock.Mock() + mock_generator.validate_config_db_json.return_value = True + mock_module = mock.Mock() + mock_module.SonicYangCfgDbGenerator.return_value = mock_generator + + with mock.patch.dict('sys.modules', {'sonic_yang_cfg_generator': mock_module}): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + result = gcu_main.validate_patch([], json.dumps({})) + self.assertTrue(result) + + def test_returns_false_when_validation_fails(self): + mock_generator = mock.Mock() + mock_generator.validate_config_db_json.return_value = False + mock_module = mock.Mock() + mock_module.SonicYangCfgDbGenerator.return_value = mock_generator + + with mock.patch.dict('sys.modules', {'sonic_yang_cfg_generator': mock_module}): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + result = gcu_main.validate_patch([], json.dumps({})) + self.assertFalse(result) + + def test_raises_on_unexpected_exception(self): + mock_module = mock.Mock() + mock_module.SonicYangCfgDbGenerator.side_effect = RuntimeError("boom") + + with mock.patch.dict('sys.modules', {'sonic_yang_cfg_generator': mock_module}): + with self.assertRaises(GenericConfigUpdaterError): + gcu_main.validate_patch([], json.dumps({})) + + def test_multiasic_validates_all_asics(self): + mock_generator = mock.Mock() + mock_generator.validate_config_db_json.return_value = True + mock_module = mock.Mock() + mock_module.SonicYangCfgDbGenerator.return_value = mock_generator + + config = {"localhost": {}, "asic0": {}, "asic1": {}} + + with mock.patch.dict('sys.modules', {'sonic_yang_cfg_generator': mock_module}): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=True): + with mock.patch('sonic_py_common.multi_asic.get_namespace_list', + return_value=['asic0', 'asic1']): + result = gcu_main.validate_patch([], json.dumps(config)) + self.assertTrue(result) + # Called once per host + once per asic + self.assertEqual(mock_generator.validate_config_db_json.call_count, 3) + + def test_multiasic_returns_false_when_asic_fails(self): + call_count = [0] + + def side_effect(_cfg): + call_count[0] += 1 + # Fail on the second call (first asic) + return call_count[0] != 2 + + mock_generator = mock.Mock() + mock_generator.validate_config_db_json.side_effect = side_effect + mock_module = mock.Mock() + mock_module.SonicYangCfgDbGenerator.return_value = mock_generator + + config = {"localhost": {}, "asic0": {}} + + with mock.patch.dict('sys.modules', {'sonic_yang_cfg_generator': mock_module}): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=True): + with mock.patch('sonic_py_common.multi_asic.get_namespace_list', + return_value=['asic0']): + result = gcu_main.validate_patch([], json.dumps(config)) + self.assertFalse(result) + + +# --------------------------------------------------------------------------- +# apply_patch_for_scope +# --------------------------------------------------------------------------- + +class TestApplyPatchForScope(unittest.TestCase): + + def test_success_records_success(self): + mock_updater = mock.Mock() + results = {} + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + gcu_main.apply_patch_for_scope( + ("", [{"op": "add", "path": "/T/k", "value": "v"}]), + results, ConfigFormat.CONFIGDB, False, False, False, () + ) + self.assertTrue(results[gcu_main.HOST_NAMESPACE]["success"]) + + def test_exception_records_failure(self): + mock_updater = mock.Mock() + mock_updater.apply_patch.side_effect = Exception("scope error") + results = {} + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + gcu_main.apply_patch_for_scope( + ("", [{"op": "add", "path": "/T/k", "value": "v"}]), + results, ConfigFormat.CONFIGDB, False, False, False, () + ) + key = list(results.keys())[0] + self.assertFalse(results[key]["success"]) + self.assertIn("scope error", results[key]["message"]) + + def test_host_namespace_scope_mapping(self): + mock_updater = mock.Mock() + results = {} + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + gcu_main.apply_patch_for_scope( + (gcu_main.HOST_NAMESPACE, []), + results, ConfigFormat.CONFIGDB, False, False, False, () + ) + # HOST_NAMESPACE scope should map correctly + self.assertIn(gcu_main.HOST_NAMESPACE, results) + + +# --------------------------------------------------------------------------- +# apply_patch_from_file +# --------------------------------------------------------------------------- + +class TestApplyPatchFromFile(unittest.TestCase): + + def _make_patch_file(self, patch_ops): + return mock.mock_open(read_data=json.dumps(patch_ops)) + + def test_invalid_format_raises(self): + bad_patch = {"op": "add"} # not a list + with mock.patch('builtins.open', mock.mock_open(read_data=json.dumps(bad_patch))): + with self.assertRaises(GenericConfigUpdaterError): + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, False, False, () + ) + + def test_success_no_preprocess(self): + patch_ops = [{"op": "add", "path": "/TABLE/key", "value": "v"}] + mock_updater = mock.Mock() + with mock.patch('builtins.open', self._make_patch_file(patch_ops)): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('generic_config_updater.main.extract_scope', + return_value=('', '/TABLE/key')): + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, False, False, (), preprocess=False + ) + mock_updater.apply_patch.assert_called_once() + + def test_preprocess_path_runs_helpers(self): + patch_ops = [{"op": "add", "path": "/TABLE/key", "value": "v"}] + running_cfg = json.dumps({"TABLE": {}}) + mock_updater = mock.Mock() + + with mock.patch('builtins.open', self._make_patch_file(patch_ops)): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + with mock.patch('generic_config_updater.main.get_all_running_config', + return_value=running_cfg): + with mock.patch('generic_config_updater.main.append_emptytables_if_required', + return_value=patch_ops) as mock_append: + with mock.patch('generic_config_updater.main.filter_duplicate_patch_operations', + return_value=patch_ops) as mock_filter: + with mock.patch('generic_config_updater.main.validate_patch', + return_value=True) as mock_validate: + with mock.patch('generic_config_updater.main.GenericUpdater', + return_value=mock_updater): + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, False, False, (), preprocess=True + ) + mock_append.assert_called_once() + mock_filter.assert_called_once() + mock_validate.assert_called_once() + + def test_preprocess_validation_failure_raises(self): + patch_ops = [{"op": "add", "path": "/TABLE/key", "value": "v"}] + running_cfg = json.dumps({}) + + with mock.patch('builtins.open', self._make_patch_file(patch_ops)): + with mock.patch('generic_config_updater.main.get_all_running_config', + return_value=running_cfg): + with mock.patch('generic_config_updater.main.append_emptytables_if_required', + return_value=patch_ops): + with mock.patch('generic_config_updater.main.filter_duplicate_patch_operations', + return_value=patch_ops): + with mock.patch('generic_config_updater.main.validate_patch', + return_value=False): + with self.assertRaises(GenericConfigUpdaterError): + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, False, False, (), preprocess=True + ) + + def test_parallel_dispatches_with_threadpool(self): + patch_ops = [{"op": "add", "path": "/TABLE/key", "value": "v"}] + mock_updater = mock.Mock() + + with mock.patch('builtins.open', self._make_patch_file(patch_ops)): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('generic_config_updater.main.extract_scope', + return_value=('', '/TABLE/key')): + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, True, False, (), preprocess=False + ) + mock_updater.apply_patch.assert_called_once() + + def test_scope_failure_raises(self): + patch_ops = [{"op": "add", "path": "/TABLE/key", "value": "v"}] + mock_updater = mock.Mock() + mock_updater.apply_patch.side_effect = Exception("boom") + + with mock.patch('builtins.open', self._make_patch_file(patch_ops)): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('generic_config_updater.main.extract_scope', + return_value=('', '/TABLE/key')): + with self.assertRaises(GenericConfigUpdaterError) as ctx: + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, False, False, (), preprocess=False + ) + self.assertIn("Failed to apply patch", str(ctx.exception)) + + def test_empty_patch_still_validates(self): + """Empty patch ops triggers per-asic validation loop.""" + patch_ops = [] + mock_updater = mock.Mock() + + with mock.patch('builtins.open', self._make_patch_file(patch_ops)): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, False, False, (), preprocess=False + ) + mock_updater.apply_patch.assert_called_once() + + def test_empty_patch_multiasic(self): + """Empty patch in multiasic triggers all asic namespaces.""" + patch_ops = [] + mock_updater = mock.Mock() + + with mock.patch('builtins.open', self._make_patch_file(patch_ops)): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=True): + with mock.patch('sonic_py_common.multi_asic.get_namespace_list', + return_value=['asic0', 'asic1']): + gcu_main.apply_patch_from_file( + '/fake/patch.json', 'CONFIGDB', + False, False, False, False, (), preprocess=False + ) + self.assertEqual(mock_updater.apply_patch.call_count, 3) # default + asic0 + asic1 + + +# --------------------------------------------------------------------------- +# print_error / print_success +# --------------------------------------------------------------------------- + +class TestPrintHelpers(unittest.TestCase): + + def test_print_error_writes_to_stderr(self): + captured = io.StringIO() + with mock.patch('sys.stderr', captured): + gcu_main.print_error("something went wrong") + self.assertIn("something went wrong", captured.getvalue()) + + def test_print_success_writes_to_stdout(self): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + gcu_main.print_success("all good") + self.assertIn("all good", captured.getvalue()) + + +# --------------------------------------------------------------------------- +# multiasic_save_to_singlefile +# --------------------------------------------------------------------------- + +class TestMultiasicSaveToSinglefile(unittest.TestCase): + + def test_saves_host_and_asic_configs(self): + host_config = {"PORT": {}} + asic_config = {"VLAN": {}} + + def fake_run(cmd, **kwargs): + result = mock.Mock() + if "-n" in cmd: + result.stdout = json.dumps(asic_config) + else: + result.stdout = json.dumps(host_config) + return result + + mock_open_obj = mock.mock_open() + with mock.patch('subprocess.run', side_effect=fake_run): + with mock.patch('sonic_py_common.multi_asic.get_namespace_list', + return_value=['asic0']): + with mock.patch('builtins.open', mock_open_obj): + gcu_main.multiasic_save_to_singlefile('/tmp/all_config.json') + + written = ''.join( + call.args[0] + for call in mock_open_obj().write.call_args_list + ) + saved = json.loads(written) + self.assertIn('localhost', saved) + self.assertIn('asic0', saved) + + +# --------------------------------------------------------------------------- +# Sub-command functions +# --------------------------------------------------------------------------- + +class TestCreateCheckpoint(unittest.TestCase): + + def _make_args(self, name='cp1', verbose=False): + return Namespace(checkpoint_name=name, verbose=verbose) + + def test_success(self): + mock_updater = mock.Mock() + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('generic_config_updater.main.print_success') as mock_ps: + gcu_main.create_checkpoint(self._make_args()) + mock_updater.checkpoint.assert_called_once_with('cp1', False) + mock_ps.assert_called_once() + + def test_success_verbose(self): + mock_updater = mock.Mock() + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + with mock.patch('generic_config_updater.main.print_success'): + gcu_main.create_checkpoint(self._make_args(verbose=True)) + self.assertIn('cp1', captured.getvalue()) + + def test_failure_calls_sys_exit(self): + mock_updater = mock.Mock() + mock_updater.checkpoint.side_effect = Exception("fail") + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with self.assertRaises(SystemExit): + gcu_main.create_checkpoint(self._make_args()) + + +class TestDeleteCheckpoint(unittest.TestCase): + + def _make_args(self, name='cp1', verbose=False): + return Namespace(checkpoint_name=name, verbose=verbose) + + def test_success(self): + mock_updater = mock.Mock() + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('generic_config_updater.main.print_success') as mock_ps: + gcu_main.delete_checkpoint(self._make_args()) + mock_updater.delete_checkpoint.assert_called_once_with('cp1', False) + mock_ps.assert_called_once() + + def test_success_verbose(self): + mock_updater = mock.Mock() + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + with mock.patch('generic_config_updater.main.print_success'): + gcu_main.delete_checkpoint(self._make_args(verbose=True)) + self.assertIn('cp1', captured.getvalue()) + + def test_failure_calls_sys_exit(self): + mock_updater = mock.Mock() + mock_updater.delete_checkpoint.side_effect = Exception("fail") + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with self.assertRaises(SystemExit): + gcu_main.delete_checkpoint(self._make_args()) + + +class TestListCheckpoints(unittest.TestCase): + + def _make_args(self, time=False, verbose=False): + return Namespace(time=time, verbose=verbose) + + def test_no_checkpoints(self): + mock_updater = mock.Mock() + mock_updater.list_checkpoints.return_value = [] + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + gcu_main.list_checkpoints(self._make_args()) + self.assertIn('No checkpoints', captured.getvalue()) + + def test_list_without_time(self): + mock_updater = mock.Mock() + mock_updater.list_checkpoints.return_value = ['cp1', 'cp2'] + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + gcu_main.list_checkpoints(self._make_args()) + output = captured.getvalue() + self.assertIn('cp1', output) + self.assertIn('cp2', output) + + def test_list_with_time(self): + mock_updater = mock.Mock() + mock_updater.list_checkpoints.return_value = [ + {'name': 'cp1', 'time': '2025-01-01T00:00:00Z'}, + ] + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + gcu_main.list_checkpoints(self._make_args(time=True)) + output = captured.getvalue() + self.assertIn('cp1', output) + self.assertIn('2025-01-01', output) + + def test_failure_calls_sys_exit(self): + mock_updater = mock.Mock() + mock_updater.list_checkpoints.side_effect = Exception("fail") + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with self.assertRaises(SystemExit): + gcu_main.list_checkpoints(self._make_args()) + + +class TestApplyPatchSubcommand(unittest.TestCase): + + def _make_args(self, patch_file='/fake/p.json', fmt='CONFIGDB', + verbose=False, dry_run=False, parallel=False, + ignore_non_yang_tables=False, ignore_path=None, + path_trace=None): + return Namespace( + patch_file=patch_file, + format=fmt, + verbose=verbose, + dry_run=dry_run, + parallel=parallel, + ignore_non_yang_tables=ignore_non_yang_tables, + ignore_path=ignore_path or [], + path_trace=path_trace, + ) + + def test_success(self): + with mock.patch('generic_config_updater.main.apply_patch_from_file') as mock_apf: + with mock.patch('generic_config_updater.main.print_success') as mock_ps: + gcu_main.apply_patch(self._make_args()) + mock_apf.assert_called_once() + mock_ps.assert_called_once() + + def test_verbose_prints_details(self): + with mock.patch('generic_config_updater.main.apply_patch_from_file'): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + with mock.patch('generic_config_updater.main.print_success'): + gcu_main.apply_patch(self._make_args(verbose=True, dry_run=True)) + output = captured.getvalue() + self.assertIn('/fake/p.json', output) + self.assertIn('DRY RUN', output) + + def test_failure_calls_sys_exit(self): + with mock.patch('generic_config_updater.main.apply_patch_from_file', + side_effect=Exception("oops")): + with self.assertRaises(SystemExit): + gcu_main.apply_patch(self._make_args()) + + +class TestReplaceConfigSubcommand(unittest.TestCase): + + def _make_args(self, config_file='/fake/cfg.json', fmt='CONFIGDB', + verbose=False, ignore_non_yang_tables=False, + ignore_path=None): + return Namespace( + config_file=config_file, + format=fmt, + verbose=verbose, + ignore_non_yang_tables=ignore_non_yang_tables, + ignore_path=ignore_path or [], + ) + + def test_success(self): + mock_updater = mock.Mock() + cfg = json.dumps({"PORT": {}}) + with mock.patch('builtins.open', mock.mock_open(read_data=cfg)): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('generic_config_updater.main.print_success') as mock_ps: + gcu_main.replace_config(self._make_args()) + mock_updater.replace.assert_called_once() + mock_ps.assert_called_once() + + def test_verbose_prints_details(self): + mock_updater = mock.Mock() + cfg = json.dumps({}) + with mock.patch('builtins.open', mock.mock_open(read_data=cfg)): + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + with mock.patch('generic_config_updater.main.print_success'): + gcu_main.replace_config(self._make_args(verbose=True)) + self.assertIn('/fake/cfg.json', captured.getvalue()) + + def test_failure_calls_sys_exit(self): + with mock.patch('builtins.open', side_effect=Exception("no file")): + with self.assertRaises(SystemExit): + gcu_main.replace_config(self._make_args()) + + +class TestSaveConfigSubcommand(unittest.TestCase): + + def _make_args(self, filename=None, verbose=False): + return Namespace(filename=filename, verbose=verbose) + + def test_save_defaults_to_config_db_file(self): + fake_cfg = json.dumps({"PORT": {}}) + + def fake_run(cmd, **kwargs): + r = mock.Mock() + r.stdout = fake_cfg + return r + + mock_open_obj = mock.mock_open() + with mock.patch('subprocess.run', side_effect=fake_run): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + with mock.patch('builtins.open', mock_open_obj): + with mock.patch('generic_config_updater.main.print_success') as mock_ps: + gcu_main.save_config(self._make_args()) + mock_ps.assert_called_once() + + def test_save_with_explicit_filename_verbose(self): + fake_cfg = json.dumps({}) + + def fake_run(cmd, **kwargs): + r = mock.Mock() + r.stdout = fake_cfg + return r + + mock_open_obj = mock.mock_open() + with mock.patch('subprocess.run', side_effect=fake_run): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + with mock.patch('builtins.open', mock_open_obj): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + with mock.patch('generic_config_updater.main.print_success'): + gcu_main.save_config(self._make_args(filename='/tmp/out.json', verbose=True)) + self.assertIn('/tmp/out.json', captured.getvalue()) + + def test_save_multiasic(self): + with mock.patch('generic_config_updater.main.multiasic_save_to_singlefile') as mock_save: + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=True): + with mock.patch('generic_config_updater.main.print_success'): + gcu_main.save_config(self._make_args(filename='/tmp/out.json')) + mock_save.assert_called_once_with('/tmp/out.json') + + def test_subprocess_error_exits(self): + with mock.patch('subprocess.run', + side_effect=subprocess.CalledProcessError(1, 'cmd')): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + with self.assertRaises(SystemExit): + gcu_main.save_config(self._make_args()) + + def test_generic_exception_exits(self): + with mock.patch('subprocess.run', side_effect=RuntimeError("oops")): + with mock.patch('sonic_py_common.multi_asic.is_multi_asic', return_value=False): + with self.assertRaises(SystemExit): + gcu_main.save_config(self._make_args()) + + +class TestRollbackConfigSubcommand(unittest.TestCase): + + def _make_args(self, name='cp1', verbose=False, + ignore_non_yang_tables=False, ignore_path=None): + return Namespace( + checkpoint_name=name, + verbose=verbose, + ignore_non_yang_tables=ignore_non_yang_tables, + ignore_path=ignore_path or [], + ) + + def test_success(self): + mock_updater = mock.Mock() + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with mock.patch('generic_config_updater.main.print_success') as mock_ps: + gcu_main.rollback_config(self._make_args()) + mock_updater.rollback.assert_called_once() + mock_ps.assert_called_once() + + def test_verbose_prints_details(self): + mock_updater = mock.Mock() + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + captured = io.StringIO() + with mock.patch('sys.stdout', captured): + with mock.patch('generic_config_updater.main.print_success'): + gcu_main.rollback_config(self._make_args(verbose=True)) + self.assertIn('cp1', captured.getvalue()) + + def test_failure_calls_sys_exit(self): + mock_updater = mock.Mock() + mock_updater.rollback.side_effect = Exception("fail") + with mock.patch('generic_config_updater.main.GenericUpdater', return_value=mock_updater): + with self.assertRaises(SystemExit): + gcu_main.rollback_config(self._make_args()) + + +# --------------------------------------------------------------------------- +# build_parser +# --------------------------------------------------------------------------- + +class TestBuildParser(unittest.TestCase): + + def setUp(self): + self.parser = gcu_main.build_parser() + + def test_parser_returns_argparse_parser(self): + import argparse + self.assertIsNotNone(self.parser) + self.assertIsInstance(self.parser, argparse.ArgumentParser) + + def test_create_checkpoint_command(self): + args = self.parser.parse_args(['create-checkpoint', 'mycp']) + self.assertEqual(args.command, 'create-checkpoint') + self.assertEqual(args.checkpoint_name, 'mycp') + self.assertFalse(args.verbose) + + def test_create_checkpoint_verbose(self): + args = self.parser.parse_args(['create-checkpoint', 'mycp', '--verbose']) + self.assertTrue(args.verbose) + + def test_delete_checkpoint_command(self): + args = self.parser.parse_args(['delete-checkpoint', 'mycp']) + self.assertEqual(args.command, 'delete-checkpoint') + self.assertEqual(args.checkpoint_name, 'mycp') + + def test_list_checkpoints_command(self): + args = self.parser.parse_args(['list-checkpoints']) + self.assertEqual(args.command, 'list-checkpoints') + self.assertFalse(args.time) + + def test_list_checkpoints_with_time(self): + args = self.parser.parse_args(['list-checkpoints', '--time']) + self.assertTrue(args.time) + + def test_apply_patch_command_defaults(self): + args = self.parser.parse_args(['apply-patch', 'my.json']) + self.assertEqual(args.command, 'apply-patch') + self.assertEqual(args.patch_file, 'my.json') + self.assertEqual(args.format, 'CONFIGDB') + self.assertFalse(args.dry_run) + self.assertFalse(args.parallel) + self.assertFalse(args.ignore_non_yang_tables) + self.assertEqual(args.ignore_path, []) + + def test_apply_patch_all_flags(self): + args = self.parser.parse_args([ + 'apply-patch', 'my.json', + '--format', 'SONICYANG', + '--dry-run', + '--parallel', + '--ignore-non-yang-tables', + '--ignore-path', '/T1', + '--ignore-path', '/T2', + '--verbose', + ]) + self.assertEqual(args.format, 'SONICYANG') + self.assertTrue(args.dry_run) + self.assertTrue(args.parallel) + self.assertTrue(args.ignore_non_yang_tables) + self.assertEqual(args.ignore_path, ['/T1', '/T2']) + self.assertTrue(args.verbose) + + def test_replace_command_defaults(self): + args = self.parser.parse_args(['replace', 'cfg.json']) + self.assertEqual(args.command, 'replace') + self.assertEqual(args.config_file, 'cfg.json') + self.assertEqual(args.format, 'CONFIGDB') + + def test_save_command_default_filename(self): + args = self.parser.parse_args(['save']) + self.assertEqual(args.command, 'save') + self.assertIsNone(args.filename) + + def test_save_command_explicit_filename(self): + args = self.parser.parse_args(['save', '/tmp/out.json']) + self.assertEqual(args.filename, '/tmp/out.json') + + def test_rollback_command(self): + args = self.parser.parse_args(['rollback', 'cp1']) + self.assertEqual(args.command, 'rollback') + self.assertEqual(args.checkpoint_name, 'cp1') + + +# --------------------------------------------------------------------------- +# main() +# --------------------------------------------------------------------------- + +class TestMain(unittest.TestCase): + + def _run_main(self, argv): + with mock.patch('sys.argv', ['gcu-standalone'] + argv): + try: + gcu_main.main() + except SystemExit: + pass + + def test_no_command_prints_help(self): + captured = io.StringIO() + with mock.patch('sys.argv', ['gcu-standalone']): + with mock.patch('sys.stdout', captured): + gcu_main.main() + # argparse prints usage/help on no subcommand + # main() calls parser.print_help() and returns + + def test_create_checkpoint_dispatched(self): + with mock.patch('generic_config_updater.main.create_checkpoint') as mock_fn: + with mock.patch('sys.argv', ['gcu', 'create-checkpoint', 'mycp']): + gcu_main.main() + mock_fn.assert_called_once() + + def test_delete_checkpoint_dispatched(self): + with mock.patch('generic_config_updater.main.delete_checkpoint') as mock_fn: + with mock.patch('sys.argv', ['gcu', 'delete-checkpoint', 'mycp']): + gcu_main.main() + mock_fn.assert_called_once() + + def test_list_checkpoints_dispatched(self): + with mock.patch('generic_config_updater.main.list_checkpoints') as mock_fn: + with mock.patch('sys.argv', ['gcu', 'list-checkpoints']): + gcu_main.main() + mock_fn.assert_called_once() + + def test_save_dispatched(self): + with mock.patch('generic_config_updater.main.save_config') as mock_fn: + with mock.patch('sys.argv', ['gcu', 'save']): + gcu_main.main() + mock_fn.assert_called_once() + + def test_rollback_dispatched(self): + with mock.patch('generic_config_updater.main.rollback_config') as mock_fn: + with mock.patch('sys.argv', ['gcu', 'rollback', 'cp1']): + gcu_main.main() + mock_fn.assert_called_once() + + def test_apply_patch_missing_file_exits(self): + with mock.patch('sys.argv', ['gcu', 'apply-patch', '/nonexistent/file.json']): + with self.assertRaises(SystemExit): + gcu_main.main() + + def test_replace_missing_file_exits(self): + with mock.patch('sys.argv', ['gcu', 'replace', '/nonexistent/cfg.json']): + with self.assertRaises(SystemExit): + gcu_main.main() + + def test_apply_patch_existing_file_dispatched(self, ): + with mock.patch('generic_config_updater.main.apply_patch') as mock_fn: + with mock.patch('os.path.exists', return_value=True): + with mock.patch('sys.argv', ['gcu', 'apply-patch', '/fake/patch.json']): + gcu_main.main() + mock_fn.assert_called_once() + + def test_replace_existing_file_dispatched(self): + with mock.patch('generic_config_updater.main.replace_config') as mock_fn: + with mock.patch('os.path.exists', return_value=True): + with mock.patch('sys.argv', ['gcu', 'replace', '/fake/cfg.json']): + gcu_main.main() + mock_fn.assert_called_once() + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index d33cf5644..bb9e61ba7 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -1365,6 +1365,54 @@ def test_validate__replace_list_item_different_location_than_target_and_no_deps_ # Act and assert self.assertTrue(self.validator.validate(move, diff, move.apply(diff.current_config))) + def test_validate_replace__calls_find_ref_paths_simulated_config_before_current_config(self): + """ + _validate_replace must check added_paths/simulated_config FIRST, then deleted_paths/current_config. + This ordering lets find_ref_paths reuse the sy singleton already loaded with simulated_config + by FullConfigMoveValidator (via _currently_loaded_hash), saving one loadData call per REPLACE. + """ + config_wrapper = ConfigWrapper() + mock_pa = MagicMock(spec=PathAddressing) + + # Track which config is passed to find_ref_paths in call order + configs_seen = [] + + def track_find_ref_paths(paths, config, reload_config=True): + configs_seen.append(config) + return [] # no refs → validation passes + mock_pa.find_ref_paths = MagicMock(side_effect=track_find_ref_paths) + mock_pa.create_path = PathAddressing.create_path + + validator = ps.NoDependencyMoveValidator(mock_pa, config_wrapper) + + # Patch _get_paths on the validator instance (not on mock_pa — _get_paths is a method + # on NoDependencyMoveValidator, not on PathAddressing) + deleted_paths = ["/PORT/Ethernet0"] + added_paths = ["/PORT/Ethernet4"] + validator._get_paths = MagicMock(return_value=(deleted_paths, added_paths)) + + current_config = {"PORT": {"Ethernet0": {"lanes": "0"}}} + simulated_config = {"PORT": {"Ethernet4": {"lanes": "4"}}} + diff = ps.Diff(current_config, simulated_config) + + # Create a move mock with the right attributes, wrapped in a group mock + # that is iterable (validate() does `for move in group:`) + inner_move = MagicMock() + inner_move.op_type = OperationType.REPLACE + inner_move.path = "" + group = MagicMock() + group.__iter__ = MagicMock(return_value=iter([inner_move])) + + validator.validate(group, diff, simulated_config) + + # Assert: simulated_config must be checked first (for added_paths), + # then current_config (for deleted_paths) + self.assertEqual(len(configs_seen), 2, "find_ref_paths should be called exactly twice") + self.assertIs(configs_seen[0], simulated_config, + "First find_ref_paths call must use simulated_config (for added_paths)") + self.assertIs(configs_seen[1], current_config, + "Second find_ref_paths call must use current_config (for deleted_paths)") + def prepare_config(self, config, patch): return patch.apply(config) @@ -2195,6 +2243,91 @@ def verify_moves(self, ops, moves): moves_ops.extend(move.get_jsonpatch()) self.assertCountEqual(ops, moves_ops) + +class TestBulkLeafListMoveGenerator(unittest.TestCase): + def setUp(self): + path_addressing = PathAddressing() + self.generator = ps.BulkLeafListMoveGenerator(path_addressing) + + def test_generate__leaf_list_items_removed__single_replace_move(self): + """Removing items from a leaf-list should produce one REPLACE move.""" + self.verify( + current={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0", "Ethernet4", "Ethernet8"], "type": "MIRROR"}}}, + target={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet8"], "type": "MIRROR"}}}, + ex_ops=[{"op": "replace", "path": "/ACL_TABLE/EVERFLOW/ports", + "value": ["Ethernet8"]}]) + + def test_generate__leaf_list_items_added__single_replace_move(self): + """Adding items to a leaf-list should produce one REPLACE move.""" + self.verify( + current={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0"], "type": "MIRROR"}}}, + target={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0", "Ethernet4", "Ethernet8"], "type": "MIRROR"}}}, + ex_ops=[{"op": "replace", "path": "/ACL_TABLE/EVERFLOW/ports", + "value": ["Ethernet0", "Ethernet4", "Ethernet8"]}]) + + def test_generate__leaf_list_unchanged__no_moves(self): + """Identical leaf-lists should produce no moves.""" + self.verify( + current={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0", "Ethernet4"], "type": "MIRROR"}}}, + target={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0", "Ethernet4"], "type": "MIRROR"}}}, + ex_ops=[]) + + def test_generate__non_list_fields_differ__no_moves(self): + """Non-list field changes should not produce moves from this generator.""" + self.verify( + current={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0"], "type": "MIRROR"}}}, + target={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0"], "type": "L3"}}}, + ex_ops=[]) + + def test_generate__list_of_dicts__no_moves(self): + """Lists of dicts (not leaf-lists) should be skipped.""" + self.verify( + current={"TABLE": {"KEY": {"items": [{"a": 1}, {"b": 2}]}}}, + target={"TABLE": {"KEY": {"items": [{"a": 1}]}}}, + ex_ops=[]) + + def test_generate__list_only_in_current__no_moves(self): + """List exists in current but not target — not a REPLACE, skip.""" + self.verify( + current={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0"], "type": "MIRROR"}}}, + target={"ACL_TABLE": {"EVERFLOW": {"type": "MIRROR"}}}, + ex_ops=[]) + + def test_generate__list_only_in_target__no_moves(self): + """List exists in target but not current — handled by other generators, skip.""" + self.verify( + current={"ACL_TABLE": {"EVERFLOW": {"type": "MIRROR"}}}, + target={"ACL_TABLE": {"EVERFLOW": {"type": "MIRROR", "ports": ["Ethernet0"]}}}, + ex_ops=[]) + + def test_generate__leaf_list_all_items_removed__single_replace_move(self): + """Removing all items from a leaf-list should produce one REPLACE with empty list.""" + self.verify( + current={"ACL_TABLE": {"EVERFLOW": {"ports": ["Ethernet0", "Ethernet4"], "type": "MIRROR"}}}, + target={"ACL_TABLE": {"EVERFLOW": {"ports": [], "type": "MIRROR"}}}, + ex_ops=[{"op": "replace", "path": "/ACL_TABLE/EVERFLOW/ports", "value": []}]) + + def test_generate__multiple_tables_with_leaf_lists__multiple_moves(self): + """Multiple differing leaf-lists should each get a REPLACE move.""" + self.verify( + current={"ACL_TABLE": { + "T1": {"ports": ["Ethernet0", "Ethernet4"], "type": "L3"}, + "T2": {"ports": ["Ethernet8", "Ethernet12"], "type": "MIRROR"}}}, + target={"ACL_TABLE": { + "T1": {"ports": ["Ethernet0"], "type": "L3"}, + "T2": {"ports": ["Ethernet12"], "type": "MIRROR"}}}, + ex_ops=[{"op": "replace", "path": "/ACL_TABLE/T1/ports", "value": ["Ethernet0"]}, + {"op": "replace", "path": "/ACL_TABLE/T2/ports", "value": ["Ethernet12"]}]) + + def verify(self, current, target, ex_ops): + diff = ps.Diff(current, target) + moves = list(self.generator.generate(diff)) + moves_ops = [] + for move in moves: + moves_ops.extend(move.get_jsonpatch()) + self.assertCountEqual(ex_ops, moves_ops) + + class TestLowLevelMoveGenerator(unittest.TestCase): def setUp(self): path_addressing = PathAddressing() @@ -3271,7 +3404,8 @@ def verify(self, algo, algo_class): expected_non_extendable_generators = [ps.BulkKeyLevelMoveGenerator, ps.KeyLevelMoveGenerator, ps.BulkKeyGroupLowLevelMoveGenerator, - ps.BulkLowLevelMoveGenerator] + ps.BulkLowLevelMoveGenerator, + ps.BulkLeafListMoveGenerator] expected_extenders = [ps.RequiredValueMoveExtender, ps.UpperLevelMoveExtender, ps.DeleteInsteadOfReplaceMoveExtender, diff --git a/utilities_common/constants.py b/utilities_common/constants.py index 25858b785..a0ef0a391 100644 --- a/utilities_common/constants.py +++ b/utilities_common/constants.py @@ -1,6 +1,10 @@ #All the constant used in sonic-utilities DEFAULT_NAMESPACE = '' + +# NOTE: A duplicate of this list exists in generic_config_updater/field_operation_validators.py +# (kept separate to avoid a utilities_common dependency in the GCU wheel). +# If you update this list, update that copy too. DEFAULT_SUPPORTED_FECS_LIST = [ 'rs', 'fc', 'none', 'auto'] DISPLAY_ALL = 'all' DISPLAY_EXTERNAL = 'frontend'