[master] Fix timing-fragile unit + stress tests exposed by container-image drift - #70230
Open
dwoz wants to merge 2 commits into
Open
[master] Fix timing-fragile unit + stress tests exposed by container-image drift#70230dwoz wants to merge 2 commits into
dwoz wants to merge 2 commits into
Conversation
…ection
The Minion.__init__() path exercised by this test synchronously executes
the engine-bootstrap Handle in salt/engines/__init__.py:16 (a
loop.call_soon that runs start_engines() inline). The 50-115 ms cost is
an inherent characteristic of the sync bootstrap path, not a handler
regression: on the 2026-08-31 salt-ci-containers image rebuild the
runtime tipped just past the fixture's 50 ms budget and the test began
ERROR'ing at teardown.
Mirrors the round-2 exemption pattern applied to
test_asynchronous.py::{test_helpers, test_double, test_double_sameloop}
(PR saltstack#70129) and test_master.py::test_register_resources_* (in-tree).
Refactoring start_engines() to yield mid-work is the systemic fix; the
tag is the minimum viable patch.
Two failures on master run 33438729348 traced back to the 2026-08-31 salt-ci-containers image rebuild widening zmq handshake / reply-flush windows past hardcoded per-op poll budgets: * mworker::test_requester_disconnect_midflight_leaves_worker_alive Round-2 (saltstack#70129) added a stale-reply drain step (single 2 s recv) before send_recv("after-reconnect"). On the new container the queued drop-me reply consistently arrives >2 s post-reconnect and slips past the bounded drain, so send_recv returns the stale reply and the assertion flips (drop-me != after-reconnect). Replace the bounded drain with a loop-until-matching-reply within a 15 s deadline: any stale queued reply is logged and discarded until we see the fresh "after-reconnect" one. * mworkerqueue::test_requester_churn_fd_bounded The very first churn cycle (warm0) races the worker's initial DEALER<->REP handshake to the queue subprocess. On the new container the handshake regularly exceeds the test's 2 s per-cycle poll, producing "queue stalled at warm0". Add an explicit warmup round-trip after mworkerqueue.worker() with a 10 s poll budget so the churn budget starts against an already-attached worker; also widen the per-cycle poll from 2 s to 5 s for headroom. MWorkerQueueHandle shape does not match MWorkerHandle (proxy vs subprocess-DEALER-bind); the round-2 DEALER context-swap pattern does not apply here. The mworkerqueue churn test opens fresh REQ sockets per iteration, so the actual failure mode is a startup race, not a rebind race -- addressed accordingly.
twangboy
approved these changes
Sep 3, 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
2 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.
Summary
Fixes two test-failure clusters on master exposed by the 2026-08-31 salt-ci-containers image rebuild (see
agents/reports/rootcause-pr-vs-push-divergencefor the digest evidence -- pre/post-rebuild image digests differ, same salt tree hash, same runner).tests/pytests/unit/test_minion.py::test_send_req_async_regression_62453-- teardown ERROR from the_asyncio_blocking_detectionautouse fixture:start_engines()runs sync at bootstrap and its 50-115 ms cost tips past the 50 ms budget on the new container. Added@pytest.mark.no_blocking(reason=...)-- mirrors the round-2 exemption pattern (PR Async MWorker: convert AESFuncs/ClearFuncs handlers to async dispatch #70129). Systemic follow-up (refactorstart_engines()to yield, or narrow the fixture's scope) noted but out of scope.tests/pytests/stress/master_subprocess/mworker/test_mworker_stress.py::test_requester_disconnect_midflight_leaves_worker_alive-- round-2's single-shot 2 s stale-reply drain misses the queued drop-me reply on the new container (arrives >2 s later); replaced with a loop-until-matching-reply within a 15 s deadline.tests/pytests/stress/master_subprocess/mworkerqueue/test_mworkerqueue_stress.py::test_requester_churn_fd_bounded-- the first churn cycle races the worker's initial DEALER<->REP handshake to the queue subprocess; added an explicit warmup round-trip with a 10 s poll budget before churn starts, and widened per-cycle poll from 2 s to 5 s.MWorkerQueueHandleis a proxy-shape handle, not a subprocess-DEALER-bind likeMWorkerHandle, so the round-2 DEALER-context-swap pattern doesn't apply -- the actual failure is a startup race, addressed accordingly.Fixes CI failures on master run 33438729348.
Test plan
venv314/bin/pyteston all three affected tests locally (3x back-to-back stability check on the stress pair, all green)test:fulllabel applied)Sibling 3008.x PR
Same tests are also red on 3008.x since 2026-08-31 (verified in the prior investigation report). A sibling PR against 3008.x should mechanically port these commits, otherwise the next 3008.x -> master merge-forward will re-conflict.