GCU: silence transient YANG leafref ERR during patch-sort search (#4482)#4484
Merged
vmittal-msft merged 1 commit intosonic-net:202511from Apr 28, 2026
Merged
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…ic-net#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 <lukelin0907@gmail.com> * 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 <lukelin0907@gmail.com> * 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 <lukelin0907@gmail.com> --------- Signed-off-by: Xichen96 <lukelin0907@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit e04fdbb)
Xichen96
force-pushed
the
cherry-pick/4482-202511
branch
from
976ce22 to
15c1d7d
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azpw run |
Collaborator
|
Retrying failed(or canceled) jobs... |
Collaborator
|
Retrying failed(or canceled) stages in build 1097405: ✅Stage Build:
|
vmittal-msft
approved these changes
Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of #4482 to 202511.
Original PR: #4482 Original commit: e04fdbb
What I did
Silenced the Data Loading Failed LOG_ERR spam emitted by sonic_yang.loadData during GCU's speculative patch-sort search. FullConfigMoveValidator already handles validation failures via the returned (False, error) tuple, so the syslog line is duplicate noise that trips loganalyzer.
Conflict resolution
generic_config_updater/gu_common.py: 202511 still carries the original tmp_config_db_as_json = copy.deepcopy(config_db_as_json) step in validate_config_db_config (removed on master by Brad House in fc9ae89 / PR #3831 "optimization: prevent unneeded deep copies that show up in profiling", not cherry-picked to 202511). Resolution: keep the deepcopy, apply
quiet=True to the sy.loadData(tmp_config_db_as_json, ...) call.
tests/generic_config_updater/patch_sorter_test.py: clean auto-merge.
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)