From 15c1d7d1a51b50582453b650122cd3a945a548a5 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Thu, 23 Apr 2026 13:02:27 +1000 Subject: [PATCH] GCU: silence transient YANG leafref ERR during patch-sort search (#4482) * 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 * 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 * 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 --------- Signed-off-by: Xichen96 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit e04fdbbf24dd4bfbdbbd23ca110b0b3ffbdce06c) --- generic_config_updater/gu_common.py | 8 +++++++- .../generic_config_updater/patch_sorter_test.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 1ef2e42005..b543a1a1b3 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -147,7 +147,13 @@ def validate_config_db_config(self, config_db_as_json): try: tmp_config_db_as_json = copy.deepcopy(config_db_as_json) - sy.loadData(tmp_config_db_as_json) + # 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(tmp_config_db_as_json, quiet=True) sy.validate_data_tree() diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index baeab6fddb..45bfbbcfc6 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -933,6 +933,23 @@ def test_validate__valid_config_db_after_applying_move__success(self): # Act and assert self.assertTrue(validator.validate(self.any_move, self.any_diff)) + def test_validate__passes_quiet_true_to_config_wrapper(self): + # 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) + + validator.validate(self.any_move, self.any_diff) + + config_wrapper.validate_config_db_config.assert_called_once_with( + self.any_simulated_config) + + class TestCreateOnlyMoveValidator(unittest.TestCase): def setUp(self): self.validator = ps.CreateOnlyMoveValidator(ps.PathAddressing())