Skip to content

Fix flaky YANG validation: apply empty patch from file, not stdin pipe#25486

Open
sakshamkhurana21 wants to merge 1 commit into
sonic-net:masterfrom
sakshamkhurana21:sakkhurana/fix-yang-validation-empty-patch-stdin-race
Open

Fix flaky YANG validation: apply empty patch from file, not stdin pipe#25486
sakshamkhurana21 wants to merge 1 commit into
sonic-net:masterfrom
sakshamkhurana21:sakkhurana/fix-yang-validation-empty-patch-stdin-race

Conversation

@sakshamkhurana21

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Tests that run YANG validation (e.g. pc/test_lag_member_forwarding) intermittently fail/error with Failed to apply patch due to: Expecting value: line 1 column 1 (char 0). The empty-patch YANG probe pipes its payload via echo '[]' | sudo config apply-patch /dev/stdin. On the DUT, generic_config_updater/main.py does json.loads(open(path).read()) on /dev/stdin as its first operation, with no empty-input guard. Under load the pipe is read back empty (0 bytes) before the data lands, so json.loads('') raises the error above, before YANG validation even runs. This replaces the pipe with a durable file (config apply-patch <file>), matching the existing race-free convention in tests/common/gu_utils.py (apply_patch).

Fixes # (no linked issue; flaky-test fix)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605

Approach

What is the motivation for this PR?

Intermittent Expecting value: line 1 column 1 (char 0) failures in the YANG-validation step cause flaky PR failures. It surfaces both in the test body (config_reload -> MultiAsicSonicHost.yang_validate) and in teardown (conftest -> yang_utils.run_yang_validation). The root cause is reading an empty /dev/stdin pipe, not an actual config/YANG problem.

How did you do it?

Replaced echo '[]' | sudo config apply-patch /dev/stdin with a durable file at both probe sites (tests/common/devices/multi_asic.py::yang_validate and tests/common/helpers/yang_utils.py::run_yang_validation): copy [] to /tmp/yang_validation_empty_patch.json, run sudo config apply-patch <file>, then remove it. A file on disk always contains [], so the empty-read race is structurally impossible. This mirrors the gu_utils.apply_patch pattern already used throughout the repo.

How did you verify/test it?

  • Reproduced deterministically on a KVM t1-lag testbed (DUT vlab-03, SONiC.202605): forcing an empty stdin pipe (printf '' | sudo config apply-patch /dev/stdin) yields the exact error (rc=2, Expecting value: line 1 column 1 (char 0)); the file-based form returns rc=0 Patch applied successfully.
  • DUT-side instrumentation confirmed the patched probe always reads the file, never /dev/stdin.
  • Re-ran test_lag_member_forwarding_packets[vlab-03] with the patched framework: green; all probes read the file.
  • flake8 clean (120-char CI limit).

Any platform specific information?

None

Supported testbed topology if it's a new test case?

N/A

Documentation

No documentation changes required.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses intermittent flakiness in YANG validation probes by avoiding config apply-patch /dev/stdin (which can be read as empty under load) and instead applying an empty JSON patch from a file on disk.

Changes:

  • Updated YANG validation helpers to write an empty patch ([]) to /tmp and apply it via config apply-patch <file>.
  • Applied the same change in both the generic YANG validation helper and the MultiAsicSonicHost.yang_validate() path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/common/helpers/yang_utils.py Switches YANG validation probe from stdin pipe to file-based empty patch application.
tests/common/devices/multi_asic.py Switches Multi-ASIC YANG validation probe from stdin pipe to file-based empty patch application.

Comment thread tests/common/helpers/yang_utils.py Outdated
Comment on lines 5 to 9
YANG_EMPTY_PATCH_FILE = "/tmp/yang_validation_empty_patch.json"


def run_yang_validation(duthost, stage="validation"):
"""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread tests/common/devices/multi_asic.py Outdated
Signed-off-by: sakshamkhurana <sakkhurana@microsoft.com>
@sakshamkhurana21 sakshamkhurana21 force-pushed the sakkhurana/fix-yang-validation-empty-patch-stdin-race branch from 4f9c432 to acd8220 Compare June 19, 2026 07:44
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request for branch(es): 202605.
Added label(s) for branch(es) 202605.

---Powered by SONiC BuildBot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants