Skip to content

DNS watcher still clears all endpoints on the SRV path when target A-record lookups fail (incomplete #7698 fix) #7730

Description

@sandy2008

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:

  1. Configure a component with a dnssrv:// or dnssrvnoa:// address (for example ruler or alertmanager peer discovery) so the watcher takes the SRV path.
  2. 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).
  3. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions