From 66cd3f6c0ab9c01951d103c0fe51ae1ebe048df4 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Sat, 8 Aug 2026 19:50:04 +0000 Subject: [PATCH] fix(ansible): serialize module resolution ### 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 --- tests/common/devices/base.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/common/devices/base.py b/tests/common/devices/base.py index 494e63c11..78b760395 100644 --- a/tests/common/devices/base.py +++ b/tests/common/devices/base.py @@ -2,6 +2,7 @@ import json import logging import collections +import os import signal import threading from contextlib import contextmanager @@ -42,6 +43,11 @@ def ansible_tqm_has_signal_registration(): _signal_patch_lock = threading.RLock() _signal_patch_ref_count = 0 _original_signal = None +# pytest-ansible resolves modules through mutable plugin-loader caches shared +# by threads in each Python process. Reset the process-local lock after fork so +# a child cannot inherit it while held by a vanished parent thread. +_ansible_module_resolution_lock = threading.RLock() +os.register_at_fork(after_in_child=_ansible_module_resolution_lock._at_fork_reinit) @contextmanager @@ -158,7 +164,9 @@ def __init__(self, ansible_adhoc, hostname, *args, **kwargs): self.hostname = hostname def __getattr__(self, module_name): - if self.host.has_module(module_name): + with _ansible_module_resolution_lock: + has_module = self.host.has_module(module_name) + if has_module: def _run_wrapper(*module_args, **kwargs): return self._run(module_name, *module_args, **kwargs) return _run_wrapper @@ -166,13 +174,17 @@ def _run_wrapper(*module_args, **kwargs): "'%s' object has no attribute '%s'" % (self.__class__, module_name) ) + def _get_ansible_module(self, module_name): + with _ansible_module_resolution_lock: + return getattr(self.host, module_name) + def _run(self, module_name, *module_args, **complex_args): previous_frame = inspect.currentframe().f_back filename, line_number, function_name, lines, index = inspect.getframeinfo(previous_frame) verbose = complex_args.pop('verbose', True) - module = getattr(self.host, module_name) + module = self._get_ansible_module(module_name) if verbose: logger.debug( "{}::{}#{}: [{}] AnsibleModule::{}, args={}, kwargs={}".format(