AI Tool Usage Notice
If you used an AI tool to help draft this issue,
please make sure you have reviewed and validated all content before submitting.
You are responsible for the accuracy and quality of everything in this report.
Low-quality or unreviewed AI-generated submissions may be closed without further investigation.
See our Generative AI Contribution Policy for details.
Describe the bug
#7698 fixed the DNS watcher clearing all known endpoints on a failed lookup, but the fix only covers the A-record path. The SRV path can still wipe every endpoint.
The new guard in lookup() relies on a nil return meaning "lookup failed" (pkg/util/grpcutil/dns_resolver.go):
func (w *dnsWatcher) lookup() []*Update {
newAddrs := w.lookupSRV()
if newAddrs == nil {
newAddrs = w.lookupHost()
}
if newAddrs == nil {
// Both the SRV and A record lookups failed. Keep the previously
// resolved addresses cached instead of removing them, so a transient
// DNS failure doesn't drop all currently known endpoints.
return nil
}
result := w.compileUpdate(newAddrs)
w.curAddrs = newAddrs
return result
}
lookupHost() honours that contract — it returns nil when the A lookup errors. lookupSRV() does not: it allocates newAddrs := make(map[string]*Update) up front, and when a per-target lookupHost(w.ctx, s.Target) call fails it logs a warning and continues. So if the SRV query itself succeeds but every target's A-record resolution fails transiently, lookupSRV() returns a non-nil empty map. lookup() then neither falls back to w.lookupHost() (the map is non-nil) nor takes the new early return, and proceeds to compileUpdate(empty), which emits Delete updates for every known address and sets w.curAddrs to empty — precisely the failure mode #7698 set out to prevent.
To Reproduce
Steps to reproduce the behavior:
- Configure a component with a
dnssrv:// or dnssrvnoa:// address (for example ruler or alertmanager peer discovery) so the watcher takes the SRV path.
- Induce a transient resolver failure that affects the SRV targets' A-record resolution but not the SRV query itself (for example the SRV response is served from cache while the target A lookups time out).
- Observe
failed SRV target DNS lookup warnings, and that every currently-known endpoint is deleted from the gRPC balancer rather than being retained. Traffic fails until a subsequent successful resolution.
The test added in #7698 covers only the A-record path, so this case is uncovered.
Expected behavior
The SRV path should follow the same nil-means-failure contract as lookupHost(): if SRV records were returned but every target resolution failed, return nil so the cached endpoints are kept. An empty non-nil map should be reserved for a genuine zero-record result.
Environment:
- Infrastructure: any deployment using
dnssrv:// / dnssrvnoa:// service discovery
- Deployment tool: any
Additional Context
Found while reviewing recent master commits (85a4553). Suggested fix: track whether any target lookup succeeded in lookupSRV() and return nil when SRV records existed but none of their targets resolved; add a regression test for the SRV path mirroring the A-record test from #7698.
AI Tool Usage Notice
If you used an AI tool to help draft this issue,
please make sure you have reviewed and validated all content before submitting.
You are responsible for the accuracy and quality of everything in this report.
Low-quality or unreviewed AI-generated submissions may be closed without further investigation.
See our Generative AI Contribution Policy for details.
Describe the bug
#7698 fixed the DNS watcher clearing all known endpoints on a failed lookup, but the fix only covers the A-record path. The SRV path can still wipe every endpoint.
The new guard in
lookup()relies on a nil return meaning "lookup failed" (pkg/util/grpcutil/dns_resolver.go):lookupHost()honours that contract — it returns nil when the A lookup errors.lookupSRV()does not: it allocatesnewAddrs := make(map[string]*Update)up front, and when a per-targetlookupHost(w.ctx, s.Target)call fails it logs a warning andcontinues. So if the SRV query itself succeeds but every target's A-record resolution fails transiently,lookupSRV()returns a non-nil empty map.lookup()then neither falls back tow.lookupHost()(the map is non-nil) nor takes the new early return, and proceeds tocompileUpdate(empty), which emitsDeleteupdates for every known address and setsw.curAddrsto empty — precisely the failure mode #7698 set out to prevent.To Reproduce
Steps to reproduce the behavior:
dnssrv://ordnssrvnoa://address (for example ruler or alertmanager peer discovery) so the watcher takes the SRV path.failed SRV target DNS lookupwarnings, and that every currently-known endpoint is deleted from the gRPC balancer rather than being retained. Traffic fails until a subsequent successful resolution.The test added in #7698 covers only the A-record path, so this case is uncovered.
Expected behavior
The SRV path should follow the same nil-means-failure contract as
lookupHost(): if SRV records were returned but every target resolution failed, return nil so the cached endpoints are kept. An empty non-nil map should be reserved for a genuine zero-record result.Environment:
dnssrv:///dnssrvnoa://service discoveryAdditional Context
Found while reviewing recent master commits (85a4553). Suggested fix: track whether any target lookup succeeded in
lookupSRV()and return nil when SRV records existed but none of their targets resolved; add a regression test for the SRV path mirroring the A-record test from #7698.