Skip to content

Bound DNS lookups in grains collection to fix salt-call --local hangs (#65324)#69840

Draft
charzl wants to merge 1 commit into
saltstack:3006.xfrom
charzl:fix/65324-ip-fqdn-dns-timeout-3006x
Draft

Bound DNS lookups in grains collection to fix salt-call --local hangs (#65324)#69840
charzl wants to merge 1 commit into
saltstack:3006.xfrom
charzl:fix/65324-ip-fqdn-dns-timeout-3006x

Conversation

@charzl

@charzl charzl commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes #65324salt-call --local (and any grains refresh) can hang for tens of seconds when the minion's hostname/FQDN does not resolve to an IPv4/IPv6 address via DNS, with no log output to explain why.

Root cause

Grains collection makes three separate, synchronous DNS lookups with no timeout, all of which the OS resolver will retry against /etc/resolv.conf's nameservers before giving up if the hostname has no usable DNS/hosts entry:

  1. salt.utils.network.get_fqhostname() (salt/utils/network.py) — calls socket.getfqdn() and then socket.getaddrinfo() to determine the minion's own FQDN. Invoked once per process from the hostname grain (salt/grains/core.py), which loads early — this is the actual point where the hang the original reporter saw (near the alphabetically-adjacent zfs grain) begins.
  2. ip_fqdn() (salt/grains/core.py) — calls socket.getaddrinfo(_fqdn, None, socket_type) once each for AF_INET and AF_INET6 to populate the fqdn_ip4/fqdn_ip6 grains.
  3. network.fqdns() (salt/modules/network.py, backing the fqdns grain) — reverse-resolves every local interface address via socket.gethostbyaddr(). This one is parallelized across a ThreadPoolExecutor, but the pool's implicit shutdown(wait=True) on context-manager exit still blocks until the slowest lookup finishes (or the pool is abandoned only at interpreter exit), so an unresolvable address is still effectively unbounded.

None of socket.getfqdn(), socket.getaddrinfo(), or socket.gethostbyaddr() accept a timeout — how long they block is entirely up to /etc/resolv.conf / glibc resolver retry settings, not anything Salt controls.

A second, compounding problem: the only diagnostic in ip_fqdn() was gated behind __opts__["__role"] == "master":

if timediff.seconds > 5 and __opts__["__role"] == "master":
    log.warning(...)

This condition is always false for salt-call --local (minion role, no master), so even a multi-second stall produced zero log output — the operation just appeared to hang with no explanation, which is exactly what the reporter observed. Git history shows this gate has an unusually contested past: added in 7855cd6ce6 ("Only display IPvX warning if role is master"), reverted minutes later in 88f49f9146, then re-added the same day in 6e1ab69710 ("Partial revert of #40934") with the rationale that the warning itself was "spurious" — specifically in the context of a master aggregating logs from many minions. That reasoning doesn't apply to a single-host salt-call --local run, where the warning is the only diagnostic available and there's no fan-out noise to worry about.

Reproduction

Reproduced on a fresh Ubuntu 22.04 VM (multipass) running Salt 3006.23. Because systemd-resolved synthesizes a local A/AAAA record for the machine's own hostname (bypassing the network entirely), reproducing the reported hang required both removing the hostname's IPv4 entry from /etc/hosts and pointing /etc/resolv.conf at an unreachable nameserver directly (bypassing the systemd-resolved stub).

Scenario salt-call --local state.test wall time
Baseline (working /etc/hosts entry) 0.29s
Unresolvable hostname, before this fix 40.7s, non-deterministic (bounded only by OS resolver retry policy)
Unresolvable hostname, after this fix (default grains_dns_lookup_timeout: 5) 20.6s, deterministic (4 lookups × 5s timeout)

strace -f -tt before the fix shows repeated ~5s gaps, each following a DNS query for the hostname, consistent with glibc's default single-query resolver timeout being hit multiple times across the three call sites above:

20:44:19.119454 sendmmsg(4, ... "salt-fd-repro" ...) = 2
20:44:24.197228 socket(AF_INET, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, IPPROTO_IP) = 4   # +5.08s
20:44:29.211403 sendmmsg(4, ... "salt-fd-repro" ...)
20:44:34.309598 socket(AF_NETLINK, ...)                                                  # +5.10s
20:44:39.439158 sendto(4, ... "salt-fd-repro" ...)
20:44:44.523289 socket(AF_INET, ...)                                                     # +5.08s
...

After the fix, strace shows exactly 4 gaps of ~5.0s each (the two get_fqhostname() calls plus the two ip_fqdn() calls, run sequentially since they occur in different grains), totaling 20.08s — matching grains_dns_lookup_timeout's default 5s bound times 4 calls, and no longer dependent on OS resolver retry behavior. Lowering grains_dns_lookup_timeout in the minion config proportionally lowers the worst-case wait.

The debug log in both the original report and this reproduction shows the stall beginning right around the zfs grains module — that module is not implicated; grains load alphabetically and hostname/ip_fqdn simply sit near it in load order.

Fix

  1. salt/utils/network.py: added _call_with_timeout(), a small helper that runs a blocking call (socket.getfqdn, socket.getaddrinfo, etc.) in a daemon thread and bounds how long the caller waits via a queue.Queue.get(timeout=...). A daemon thread is used deliberately instead of concurrent.futures.ThreadPoolExecutor: the executor's shutdown() (called implicitly on context-manager exit, and again by its atexit handler at interpreter shutdown) joins outstanding workers, which would silently re-introduce an unbounded wait the moment the pool is torn down or the process exits. An abandoned daemon-thread lookup is simply left to finish (or never finish) on its own without blocking anything.
    get_fqhostname() now accepts a timeout parameter (default 5s) and uses this helper for both of its underlying calls; on timeout it falls back to socket.gethostname() (no network) rather than propagating the error, matching what socket.getfqdn() would eventually have returned anyway.

  2. salt/grains/core.py:

    • ip_fqdn() now resolves each address family through the same bounded-wait pattern instead of calling socket.getaddrinfo() directly.
    • hostname() passes the configured timeout through to get_fqhostname().
    • The __role == "master" gate on the timeout warning is removed — the warning now fires any time a lookup takes a non-trivial fraction of the configured timeout (or hits it outright), regardless of role, so salt-call --local runs get a diagnostic instead of silence.
  3. salt/modules/network.py: fqdns()'s reverse-lookup loop is rewritten to launch one daemon thread per address (still resolved in parallel) with a single shared deadline, rather than a ThreadPoolExecutor whose exit/shutdown can still block on a straggler. N unresolvable addresses now cost at most grains_dns_lookup_timeout in total, not N × timeout.

  4. salt/config/__init__.py: new minion option grains_dns_lookup_timeout (default: 5 seconds), shared by all three call sites above.

Test plan

  • Unit tests for _call_with_timeout() covering the fast path, the timeout path, and exception propagation (tests/pytests/unit/utils/test_network.py)
  • Unit test for get_fqhostname() falling back to socket.gethostname() on timeout instead of hanging/raising
  • Unit tests for ip_fqdn() covering the timeout path and confirming the warning now logs regardless of __role (tests/pytests/unit/grains/test_core.py)
  • Unit tests for fqdns() covering a single hanging lookup and confirming N parallel hanging lookups still cost ~1 timeout, not N (tests/pytests/unit/modules/test_network.py)
  • Manual repro on a VM with an unresolvable hostname + unreachable DNS, before/after timing comparison (40.7s → 20.6s, deterministic)
  • Confirmed via strace -f -tt that post-fix wait time is exactly bounded by grains_dns_lookup_timeout × (number of lookups), not OS resolver retry policy
  • Traced through tests/support/pytest/loader.py's LoaderModuleMock to confirm __opts__ defaults to {} for modules under test that don't explicitly provide it, so the new __opts__.get("grains_dns_lookup_timeout", 5) calls don't break existing tests that predate this option
  • Not able to execute the full pytest suite against these changes in this environment (sandboxed, no PyPI access to install pytest-salt-factories and friends) — new and existing tests are syntax-checked and pattern-matched against the existing suite, but should be run in CI before merge

Fixes saltstack#65324. salt-call --local (and any grains refresh) could hang
for tens of seconds when the minion's hostname had no usable DNS/hosts
entry, because three call sites made synchronous, native-timeout-less
DNS lookups:

- salt.utils.network.get_fqhostname() (socket.getfqdn() +
  socket.getaddrinfo()), used by the hostname grain
- ip_fqdn()'s socket.getaddrinfo() calls for fqdn_ip4/fqdn_ip6
- network.fqdns()'s socket.gethostbyaddr() reverse lookups, which were
  already parallelized via ThreadPoolExecutor but still blocked on
  shutdown()/context-exit joining a straggler thread

All three now go through daemon-thread-based helpers bound by a new
grains_dns_lookup_timeout minion option (default 5s). Daemon threads
are used deliberately instead of ThreadPoolExecutor: the executor's
implicit shutdown(wait=True) on context exit (and its atexit handler)
would still block joining an abandoned lookup, silently reintroducing
the same unbounded wait.

Also removed the __opts__["__role"] == "master" gate on ip_fqdn()'s
timeout warning, added in 2017 to silence per-minion log noise on
masters aggregating many minions -- a reasonable call there, but one
that left a single-host salt-call --local run with zero diagnostic
output during the exact hang being reported.

Verified against a live repro (multipass VM, unresolvable hostname +
unreachable DNS): 40.7s non-deterministic hang -> 20.6s deterministic
wait, confirmed via strace -f -tt to be exactly bounded by
grains_dns_lookup_timeout x (number of lookups) rather than OS
resolver retry policy.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@welcome

welcome Bot commented Jul 21, 2026

Copy link
Copy Markdown

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject.pdl@broadcom.com. We're glad you've joined our community and look forward to doing awesome things with you!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant