Skip to content

device/telemetry: peer cache cleared before fallible work in refresh() #4128

Description

@elitegreg

Tracked by #4130.

Problem

ledgerPeerDiscovery.refresh clears the peer cache under the lock and then does work that can
fail, so any failure after that point leaves the agent with zero peers:

// controlplane/telemetry/internal/telemetry/peers.go:124
p.peersMu.Lock()
defer p.peersMu.Unlock()

p.peers = make([]*Peer, 0, len(p.peers))   // line 127 — cache destroyed here

devices := make(map[string]serviceability.Device)
...
// line 142
interfaces, err := p.config.LocalNet.Interfaces()
if err != nil {
	metrics.Errors.WithLabelValues(metrics.ErrorTypePeerDiscoveryGettingLocalInterfaces).Inc()
	return fmt.Errorf("failed to get local interfaces: %w", err)   // returns with peers empty
}

The GetProgramData call at line 118 is safe because it runs before the lock. LocalNet.Interfaces()
at line 142 is not. A transient failure there empties the peer list until a later refresh succeeds,
and Pinger.Tick then iterates an empty slice and produces nothing, silently.

The happy path already does the right thing: it builds into a local peers slice and assigns at
line 209. Line 127 is redundant with that.

Proposed change

Delete line 127 and move the lock to cover only the assignment.

func (p *ledgerPeerDiscovery) refresh(ctx context.Context) error {
	data, err := p.config.ProgramClient.GetProgramData(ctx)
	if err != nil { ... }

	// build devices, links, interfaces, peers into locals — no lock held

	p.peersMu.Lock()
	p.peers = peers
	p.peersMu.Unlock()
	return nil
}

This also shortens the critical section, which currently spans the whole build including
LocalNet.Interfaces().

Note on severity

This is not what caused the 2026-07-29 outage (the pinger's epoch dependency was, see the
companion issue), and it is bounded by the next successful refresh 10s later. Filing because it is
a real way to lose all probing with no log line, and the fix is two lines.

Acceptance

  • A test where LocalNet.Interfaces() returns an error asserts GetPeers() still returns the
    previously discovered peers.
  • No behavior change on the success path.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions