Fix silent device-state desync after plugin restart#15
Conversation
A pod restart at a bad moment (udev event storm, disks still attaching, kubelet busy) could leave the plugin advertising an incomplete device set until the next restart. The state was built once from enumeration and then maintained purely from netlink events, but several paths lose events silently: - the netlink socket can overflow (ENOBUFS) and go-udev reports nothing - mux.Submit dropped events for all subscribers after a 1s timeout - scatter registered resources with the kubelet synchronously inside its event loop, back-pressuring the whole fan-out - the monitor reconnect path never re-enumerated - a failed kubelet registration permanently dropped the resource Fixes: - periodically reconcile discovery state against a full enumeration (default 60s, WithReconcileInterval), and resync after monitor reconnects, so any missed event heals within one interval - wrap every subscriber sink in an unbounded elastic queue so a slow consumer can never cause event loss upstream - enlarge the netlink receive buffer to 32MiB and install subsystem filters (block, net) to shrink the event stream - retry kubelet registration with exponential backoff instead of dropping the resource on first failure Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the udev-based device discovery and kubelet registration paths to prevent silent device-state drift after restarts and transient failures, ensuring the plugin self-heals when netlink events are missed and when kubelet is temporarily unavailable.
Changes:
- Add periodic reconciliation (re-enumeration) and reconnect resync to heal missed udev events.
- Introduce
mux.ElasticSinkand wrap discovery subscribers to prevent slow consumers from causing upstream event drops. - Add async kubelet registration with exponential backoff retry, plus e2e coverage for delayed kubelet availability.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/udev/udev.go | Adds reconcile loop, monitor buffer sizing + subsystem filters, reconnect resync, and elastic buffering of subscriber sinks. |
| internal/udev/reconcile_internal_test.go | Unit tests for enumeration reconciliation logic (applyEnumeration). |
| internal/plugin/registry.go | Changes kubelet registration to async + retry with exponential backoff. |
| internal/mux/elastic.go | Adds an unbounded elastic sink wrapper to decouple producers from slow consumers. |
| internal/mux/elastic_test.go | Tests ordering, non-blocking Submit, Close behavior, and post-close Submit rejection for ElasticSink. |
| cmd/udev-manager/fakekubelet_test.go | Adds controllable registration failures to simulate kubelet unavailability. |
| cmd/udev-manager/e2e_test.go | Adds e2e test validating registration eventually succeeds with retries/backoff. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a02b3a1 to
14ae170
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Cancel monitor and registration work when plugin instances stop, reconnect closed udev streams, preserve live sockets on duplicate resources, and keep replacement reconciliation ordering healthy.
Replace front-reslicing with a growable ring buffer that reuses slots, releases pathological peak capacity after draining, and performs zero-allocation steady-state FIFO operations.
Add a positive duration-valued reconcile_interval setting, preserve the one-minute default, reject ambiguous numeric values, and document the configuration.
| // back-pressure the fan-out: without it, one stalled subscriber causes | ||
| // the monitor's mux.Submit to time out and drop the event for every | ||
| // subscriber, permanently desynchronizing advertised resources. | ||
| elastic := mux.ElasticSink(sink, klogLogger{}) |
There was a problem hiding this comment.
The same problem exists on the other side as well. A request can be dropped when it is sent to the resource.broadcast, which delivers state to the ListAndWatch subscriber (i.e. the kubelet), because the resource.broadcast is implemented as a regular Mux with the default one-second timeout
udev-manager/internal/plugin/resource.go
Line 142 in 3439ebb
There was a problem hiding this comment.
Wouldn't it be better to entirely remove the possibility of dropping requests sent through Submit?
udev-manager/internal/mux/mux.go
Line 379 in 3439ebb
In my opinion, this would make the existing code simpler and more reliable, and entities such as elasticSink would no longer be needed.
Channels with a large but fixed capacity may be sufficient. For cases where we must not block while Kubernetes is draining a channel, we could use a dedicated channel with a small capacity and a single-element queue. If a new Instance update remains queued for some time, we could discard the previous pending updates and send only the latest one. But this still seems overcomplicated. Let's stick to the simpler solution (plain channels with fixed capacity)
| // other subsystems still show up via enumeration (State / reconcile), but | ||
| // their add/remove events are not delivered. | ||
| func (d *udevDiscovery) newMonitor() (<-chan *libudev.Device, <-chan error, error) { | ||
| mon := d.udev.NewMonitorFromNetlink("udev") |
There was a problem hiding this comment.
It is not entirely clear to me why we need to listen for Netlink events.
Wouldn't it be sufficient to keep only the reconciliation logic, which periodically iterates over the sysfs contents, compares them with the previous iteration, generates the corresponding health events, and sends them to the kubelet ListAndWatch channel?
There was a problem hiding this comment.
I mean, we just added new code that enumerates devices during reconciliation on top of the existing event stream from DeviceChan
udev-manager/internal/udev/udev.go
Line 387 in 3439ebb
So now we have two parallel event streams, and I'm not sure there isn't a race condition between them
| // longer present are deleted. It returns the devices that were added and | ||
| // removed. Callers must hold no lock; state is the caller-owned map guarded | ||
| // by d.mu in the discovery case. | ||
| func applyEnumeration(state map[Id]Device, found map[Id]Device) (added, removed []Device) { |
There was a problem hiding this comment.
It looks like this logic cannot detect state changes when a device is replaced or modified (for example, re-partitioned) while retaining the same syspath
A pod restart at a bad moment (udev event storm, disks still attaching, kubelet busy) could leave the plugin advertising an incomplete device set until the next restart. The state was built once from enumeration and then maintained purely from netlink events, but several paths lose events silently:
Fixes: