Rocky 9 integration tcp/zeromq jobs fail most 3006.x PR runs: leaked minion keys from earlier test modules break netapi '*' targeting

[ggiesen]

What happened?

The "Rocky Linux 9 integration tcp" and "Rocky Linux 9 integration zeromq" jobs fail on most 3006.x PR runs with the same signature, regardless of what the PR touches. Fresh example: run 28719299995 (PR #69682, a logrotate parser fix), jobs 85277223700 (tcp) and 85277223702 (zeromq), both transports failing the same tests:

FAILED tests/pytests/integration/netapi/rest_tornado/test_root_handler.py::test_simple_local_post - salt.ext.tornado.ioloop.TimeoutError: Operation timed out after 30 seconds
FAILED tests/pytests/integration/netapi/rest_tornado/test_root_handler.py::test_simple_local_post_only_dictionary_request - salt.ext.tornado.ioloop.TimeoutError: Operation timed out after 30 seconds
FAILED tests/pytests/integration/netapi/rest_tornado/test_root_handler.py::test_simple_local_async_post
FAILED tests/pytests/integration/netapi/rest_tornado/test_multi_local_async_post (and siblings)
FAILED tests/pytests/integration/netapi/test_client.py::test_local_async

The async assertion shows the cause directly:

AssertionError: assert ['minion-T1psGj', 'minion-T1psGj-empty-string', 'minion-T1psGj-highstate',
                        'minion-T1psGj-sls', 'minion-T1psGj-top', 'non-root-minion-YWdURX',
                        'sub-minion-7ENuRT']
              == ['minion-T1psGj', 'sub-minion-7ENuRT']

Root cause: test-session isolation leak (accepted keys outlive their minions)

The netapi integration tests target * and assert exactly the two session minions. But two earlier test modules in the same test group start extra minions against the shared session master:

• tests/pytests/integration/minion/test_startup_states.py -- four minions named {salt_minion_id}-empty-string/-highstate/-sls/-top
• tests/pytests/integration/cli/test_salt_call_ownership.py -- non-root-minion-*

Both use with factory.started(): yield factory fixtures. That stops the minion process at teardown, but the master keeps the accepted key. Later netapi tests that target * do key-based matching, so:

• local_async returns all seven key-matched minions -> the list assertion fails;
• synchronous local waits for returns from five minions that no longer exist -> TimeoutError: Operation timed out after 30 seconds (which also blows through the shared 30 s and drags neighbouring tests down).

This only bites the PR pipeline's combined test run, where minion/, cli/ and netapi/ modules execute in the same pytest session against one master. The Nightly/scheduled runs split tests across multiple groups, which separates the leaking modules from the netapi assertions -- so nightly is green while nearly every PR run is red, which is the worst combination for triage (contributors learn to ignore the two red jobs).

In some earlier instances of these jobs we have also seen a secondary crash signature from the master's event handling: 'list' object has no attribute 'items' at salt/utils/event.py:941, where ret = load.get("return", {}) is assumed to be a dict before ret.items() -- a job return whose payload is a list triggers it. It did not appear in the runs quoted above, so it may be a separate lower-frequency issue, but it surfaces in the same jobs.

Possible fixes (maintainer input welcome before a PR)

1. Delete the extra minions' keys at fixture teardown in the two leaking modules (e.g. salt-key -d <id> -y via the master factory after factory.started() exits). Cheap, targeted, keeps the netapi assertions strict -- my preferred option, happy to send the PR.
2. And/or make the netapi tests robust against foreign keys (target the two known minion IDs with a list target instead of *, or prune unaccepted/stale keys in a fixture). This hardens against any future module leaking keys, at the cost of loosening what the tests prove about * targeting.
3. Independently, salt/utils/event.py:941 could guard ret.items() with an isinstance check so a list-shaped return does not crash the event tagger, though that treats a symptom.

Happy to implement whichever combination you prefer.

Type of salt install

Not applicable, CI (GitHub Actions PR pipeline, 3006.x).

Major version

3006.x

What supported OS are you seeing the problem on? Can select multiple. (If bug appears on an unsupported OS, please open a GitHub Discussion instead)

rockylinux-9

salt --versions-report output

Not applicable, CI integration test jobs.

[ggiesen]

Confirmation from the CI run on #69729 itself (run 28764048341, job 85297152499): the leaked-minion-key cluster is gone from that run - zero netapi failures, no inflated minion lists - so the fixture teardown fix addresses the first signature. The remaining failures in the job are the second, independent cluster: 12 occurrences of the event.py:941 crash ('list' object has no attribute 'items', triggered by failing state compilations returning a list of error strings), which fails the test_state_test assertions that scan the master log for 'Event iteration failed', plus consistent FactoryTimeout fallout in the orchestrate tests. That crash is now tracked as #69730 with a fix in #69731. There is also a third, smaller signature in the same job (salt-ssh test_renderer_file failing with TemplateNotFound: map.jinja and an 'ext_pillar interface ext_pillar_opts is unavailable' critical), which looks like separate cross-test config leakage and remains undiagnosed.