Skip to content

[action] [PR:26199] fix(ansible): serialize module resolution - #1355

Merged
mssonicbld merged 1 commit into
Azure:202608from
mssonicbld:cherry/msft-202608/26199
Aug 8, 2026
Merged

[action] [PR:26199] fix(ansible): serialize module resolution#1355
mssonicbld merged 1 commit into
Azure:202608from
mssonicbld:cherry/msft-202608/26199

Conversation

@mssonicbld

Copy link
Copy Markdown
Collaborator

Description of PR

Summary:

Serialize pytest-ansible module discovery to prevent concurrent SONiC neighbor initialization from temporarily reporting existing custom modules such as sonic_basic_facts as missing.

Fixes # (issue)

ADO: 38183684

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

Tracking issue/work item for backport/cherry-pick request: 38183684
Failure type: regression

Approach

What is the motivation for this PR?

PR KVM runs intermittently fail while constructing SONiC neighbor hosts in parallel. AnsibleHostBase.__getattr__ asks pytest-ansible whether sonic_basic_facts exists, but pytest-ansible resolves modules through mutable plugin-loader caches shared by all threads in the current Python process. A concurrent cache update can return a false unresolved result, causing an AttributeError before the test body runs. The issue is active on master and 202605, and most affected plans pass on retry after the loader and facts caches are warm.

How did you do it?

Added a process-local re-entrant lock around the two Ansible module-resolution operations in AnsibleHostBase: checking whether the dynamic module exists and resolving the executable module wrapper. Registered the lock with os.register_at_fork so a forked child reinitializes its copy instead of inheriting a locked guard from a vanished parent thread. Remote Ansible execution remains outside the lock, preserving parallel multi-host operations.

How did you verify/test it?

Reviewed a representative Elastictest failure where one of four SONiC neighbors reported sonic_basic_facts missing while the other three resolved it, followed by a successful module retry. The failing nbrhosts path uses SafeThreadPoolExecutor, which is backed by multiprocessing.pool.ThreadPool; xdist is not the concurrency boundary for the four neighbor constructors. Validated fork safety by holding the lock in a parent thread, forking a child, and confirming the child could acquire the reinitialized lock.

Any platform specific information?

The failure was observed on KVM VS t0-64-32 runs using SONiC neighbors. The fix is in the shared Ansible host wrapper and is not ASIC-specific.

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

Not a new test case.

Documentation

Not applicable.

Signed-off-by: Sonic Build Admin sonicbld@microsoft.com

### Description of PR

Summary:

Serialize pytest-ansible module discovery to prevent concurrent SONiC neighbor initialization from temporarily reporting existing custom modules such as `sonic_basic_facts` as missing.

Fixes # (issue)

ADO: 38183684

### Type of change

- [x] Bug fix
- [x] 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
- [x] 202605

Tracking issue/work item for backport/cherry-pick request: 38183684
Failure type: regression

### Approach
#### What is the motivation for this PR?

PR KVM runs intermittently fail while constructing SONiC neighbor hosts in parallel. `AnsibleHostBase.__getattr__` asks pytest-ansible whether `sonic_basic_facts` exists, but pytest-ansible resolves modules through mutable plugin-loader caches shared by all threads in the current Python process. A concurrent cache update can return a false unresolved result, causing an `AttributeError` before the test body runs. The issue is active on master and 202605, and most affected plans pass on retry after the loader and facts caches are warm.

#### How did you do it?

Added a process-local re-entrant lock around the two Ansible module-resolution operations in `AnsibleHostBase`: checking whether the dynamic module exists and resolving the executable module wrapper. Registered the lock with `os.register_at_fork` so a forked child reinitializes its copy instead of inheriting a locked guard from a vanished parent thread. Remote Ansible execution remains outside the lock, preserving parallel multi-host operations.

#### How did you verify/test it?

Reviewed a representative Elastictest failure where one of four SONiC neighbors reported `sonic_basic_facts` missing while the other three resolved it, followed by a successful module retry. The failing `nbrhosts` path uses `SafeThreadPoolExecutor`, which is backed by `multiprocessing.pool.ThreadPool`; xdist is not the concurrency boundary for the four neighbor constructors. Validated fork safety by holding the lock in a parent thread, forking a child, and confirming the child could acquire the reinitialized lock.

#### Any platform specific information?

The failure was observed on KVM VS `t0-64-32` runs using SONiC neighbors. The fix is in the shared Ansible host wrapper and is not ASIC-specific.

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

Not a new test case.

### Documentation

Not applicable.

Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator Author

Original PR: sonic-net/sonic-mgmt#26199

@mssonicbld

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.

@mssonicbld
mssonicbld merged commit 80d60e2 into Azure:202608 Aug 8, 2026
5 checks passed
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.

1 participant