From a360876eb34a0f7d7c49ea92c18b0da607e81dc8 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 22 Apr 2026 19:03:55 +0000 Subject: [PATCH 1/3] GCU: silence transient YANG leafref ERR during patch-sort search FullConfigMoveValidator is invoked speculatively during the patch-sort search to check whether a candidate move results in a YANG-valid config. Forward leafref references produce expected transient validation failures; the sorter already handles these as a prune signal via the returned (False, error) tuple. However, sonic_yang.loadData logs a LOG_ERR "Data Loading Failed" line to syslog on every exception (log-and-throw antipattern) before raising, so each speculative probe leaks an ERR to syslog. In practice a single config apply-patch during the GCU test suite can emit 100+ such lines, which trips loganalyzer and makes real errors hard to find. This change threads a quiet= kwarg through: ConfigWrapper.validate_config_db_config(..., quiet=False) -> sonic_yang.loadData(..., quiet=quiet) and makes FullConfigMoveValidator.validate pass quiet=True. Non- speculative callers (e.g. final target-config validation) keep the existing default (quiet=False) and continue to log on real errors. Depends on sonic-net/sonic-buildimage: quiet= kwarg added to sonic_yang_ext.SonicYangExtMixin.loadData. Validated on DUT with sonic-mgmt generic_config_updater subset (dhcp_relay, eth_interface, vlan_interface, portchannel_interface, cacl, lo_interface, bgp_prefix, syslog): BEFORE: 31P/1F/3S/3E, 231 apply-patch, 115 Data Loading Failed ERR AFTER: 31P/1F/3S/2E, 231 apply-patch, 34 Data Loading Failed ERR Remaining ERR lines are from legitimate non-speculative callers. Signed-off-by: Xichen96 --- generic_config_updater/gu_common.py | 4 ++-- generic_config_updater/patch_sorter.py | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 3b25ec292a1..d142c8aa8d2 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -137,7 +137,7 @@ def validate_sonic_yang_config(self, sonic_yang_as_json): except sonic_yang.SonicYangException as ex: return False, ex - def validate_config_db_config(self, config_db_as_json): + def validate_config_db_config(self, config_db_as_json, quiet=False): sy = self.create_sonic_yang_with_loaded_models() # TODO: Move these validators to YANG models @@ -146,7 +146,7 @@ def validate_config_db_config(self, config_db_as_json): try: # Loading data automatically does full validation - sy.loadData(config_db_as_json) + sy.loadData(config_db_as_json, quiet=quiet) for supplemental_yang_validator in supplemental_yang_validators: success, error = supplemental_yang_validator(config_db_as_json) if not success: diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index 18461f75811..f15bf37ebee 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -849,7 +849,12 @@ def __init__(self, config_wrapper): self.config_wrapper = config_wrapper def validate(self, move, diff, simulated_config) -> Tuple[bool, Optional[str]]: - is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config) + # Speculative validation during patch-sort search. Transient YANG + # leafref failures are expected here and are handled by the sorter + # as a signal to prune the search branch. Pass quiet=True so + # sonic_yang.loadData does not leak a LOG_ERR line to syslog for + # every transient failure during the search. + is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config, quiet=True) return is_valid, error class CreateOnlyMoveValidator: From 438ccd8be95ccc3119ce66bbd69177ec1447a14b Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 22 Apr 2026 19:04:05 +0000 Subject: [PATCH 2/3] Address review: backward-compat quiet= + unit test - gu_common.validate_config_db_config: wrap sy.loadData call in try/except TypeError so callers keep working against older sonic-yang-mgmt wheels that do not yet ship the quiet= kwarg. - tests/generic_config_updater/patch_sorter_test.py: add regression guard asserting FullConfigMoveValidator.validate invokes the config wrapper with quiet=True so the speculative patch-sort search does not spam syslog with transient leafref LOG_ERR lines. Signed-off-by: Xichen96 --- tests/generic_config_updater/patch_sorter_test.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index a092f26101a..bfb6e67aab5 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -946,6 +946,21 @@ def test_validate__valid_config_db_after_applying_move__success(self): self.assertTrue( validator.validate(JsonMoveGroup("", self.any_move), self.any_diff, self.any_simulated_config)[0]) + def test_validate__passes_quiet_true_to_config_wrapper(self): + # Regression guard: FullConfigMoveValidator MUST invoke the config + # wrapper with quiet=True so the speculative patch-sort search does + # not spam syslog with transient YANG leafref LOG_ERR lines (the + # sorter already handles those via the returned tuple). + config_wrapper = Mock() + config_wrapper.validate_config_db_config.return_value = (True, None) + validator = ps.FullConfigMoveValidator(config_wrapper) + + validator.validate(JsonMoveGroup("", self.any_move), self.any_diff, self.any_simulated_config) + + config_wrapper.validate_config_db_config.assert_called_once_with( + self.any_simulated_config, quiet=True) + + class TestCreateOnlyMoveValidator(unittest.TestCase): def setUp(self): self.validator = ps.CreateOnlyMoveValidator(ps.PathAddressing()) From 36f11d36120b37ec53fb5bf59aa6022d27ae7a53 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 22 Apr 2026 19:04:15 +0000 Subject: [PATCH 3/3] Simplify: drop quiet= plumbing, hardcode quiet=True in loadData Per review feedback, the quiet= kwarg threading through ConfigWrapper.validate_config_db_config adds API surface for no real benefit. loadData's LOG_ERR "Data Loading Failed" line is always redundant -- every caller already gets the full exception via the returned (False, error) tuple, so the syslog line is duplicate noise regardless of whether the call site is the speculative patch-sort search or a non-speculative validator. This commit: - Removes the quiet= kwarg from validate_config_db_config and pins quiet=True directly in the loadData call. - Reverts patch_sorter.FullConfigMoveValidator.validate to its original one-line form (no longer needs to pass quiet=True). - Updates the regression unit test to assert the wrapper is called without the kwarg. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xichen96 --- generic_config_updater/gu_common.py | 11 ++++++++--- generic_config_updater/patch_sorter.py | 7 +------ tests/generic_config_updater/patch_sorter_test.py | 12 +++++++----- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index d142c8aa8d2..9f538ace4a4 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -137,7 +137,7 @@ def validate_sonic_yang_config(self, sonic_yang_as_json): except sonic_yang.SonicYangException as ex: return False, ex - def validate_config_db_config(self, config_db_as_json, quiet=False): + def validate_config_db_config(self, config_db_as_json): sy = self.create_sonic_yang_with_loaded_models() # TODO: Move these validators to YANG models @@ -145,8 +145,13 @@ def validate_config_db_config(self, config_db_as_json, quiet=False): self.validate_lanes] try: - # Loading data automatically does full validation - sy.loadData(config_db_as_json, quiet=quiet) + # Loading data automatically does full validation. + # quiet=True suppresses sonic_yang.loadData's LOG_ERR + # "Data Loading Failed" line on every exception (log-and-throw + # antipattern). Real failures still surface via the returned + # tuple / SonicYangException, so callers retain full error + # signal -- only the duplicate syslog spam is silenced. + sy.loadData(config_db_as_json, quiet=True) for supplemental_yang_validator in supplemental_yang_validators: success, error = supplemental_yang_validator(config_db_as_json) if not success: diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index f15bf37ebee..18461f75811 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -849,12 +849,7 @@ def __init__(self, config_wrapper): self.config_wrapper = config_wrapper def validate(self, move, diff, simulated_config) -> Tuple[bool, Optional[str]]: - # Speculative validation during patch-sort search. Transient YANG - # leafref failures are expected here and are handled by the sorter - # as a signal to prune the search branch. Pass quiet=True so - # sonic_yang.loadData does not leak a LOG_ERR line to syslog for - # every transient failure during the search. - is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config, quiet=True) + is_valid, error = self.config_wrapper.validate_config_db_config(simulated_config) return is_valid, error class CreateOnlyMoveValidator: diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index bfb6e67aab5..c60b7b07532 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -947,10 +947,12 @@ def test_validate__valid_config_db_after_applying_move__success(self): validator.validate(JsonMoveGroup("", self.any_move), self.any_diff, self.any_simulated_config)[0]) def test_validate__passes_quiet_true_to_config_wrapper(self): - # Regression guard: FullConfigMoveValidator MUST invoke the config - # wrapper with quiet=True so the speculative patch-sort search does - # not spam syslog with transient YANG leafref LOG_ERR lines (the - # sorter already handles those via the returned tuple). + # Regression guard: gu_common.ConfigWrapper.validate_config_db_config + # MUST call sonic_yang.loadData with quiet=True so the speculative + # patch-sort search does not spam syslog with transient YANG + # leafref LOG_ERR lines (the sorter already handles those via the + # returned tuple). This is enforced inside ConfigWrapper itself, + # so all callers benefit; FullConfigMoveValidator simply delegates. config_wrapper = Mock() config_wrapper.validate_config_db_config.return_value = (True, None) validator = ps.FullConfigMoveValidator(config_wrapper) @@ -958,7 +960,7 @@ def test_validate__passes_quiet_true_to_config_wrapper(self): validator.validate(JsonMoveGroup("", self.any_move), self.any_diff, self.any_simulated_config) config_wrapper.validate_config_db_config.assert_called_once_with( - self.any_simulated_config, quiet=True) + self.any_simulated_config) class TestCreateOnlyMoveValidator(unittest.TestCase):