ROX-32949: reactive DNF/RPM scanning in roxagent (Phase 1)#21545
Draft
vikin91 wants to merge 10 commits into
Draft
ROX-32949: reactive DNF/RPM scanning in roxagent (Phase 1)#21545vikin91 wants to merge 10 commits into
vikin91 wants to merge 10 commits into
Conversation
Detects package-change events on candidate RPM database directories with trivial buffered-channel coalescing (no debounce yet — Phase 2). Part of ROX-32949 reactive DNF scanning; see docs/superpowers/specs/2026-07-03-reactive-dnf-scanning-design.md. Co-authored-by: Cursor <cursoragent@cursor.com>
Wires the new watch.Watcher into runServe's select loop as a third case alongside the periodic ticker, sharing one rescan/cache/generation-bump code path. Tags every cached report's facts with scan_trigger (scheduled|reactive) so Sensor can prioritize delivery. Part of ROX-32949; see docs/superpowers/specs/2026-07-03-reactive-dnf-scanning-design.md. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds virtual_machine_reactive_index_report_latency_seconds (SLA measurement) and virtual_machine_discovered_data_scan_trigger_total (observability), used by the reactive-scan handling added in the next commits. Part of ROX-32949. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds Handler.SendReactive, backed by a per-VM 'latest pending' map (reactivePending) that handlerImpl.run() drains ahead of the routine indexReports channel on every iteration. Bounds memory by fleet size instead of an arbitrary channel capacity. Falls back to the normal queue if the VM isn't resolvable yet, so a reactive update is never silently dropped. Part of ROX-32949; see docs/superpowers/specs/2026-07-03-reactive-dnf-scanning-design.md. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds isReactiveTrigger (consumed by the scraper routing change in the next commit) and a virtual_machine_discovered_data_scan_trigger_total metric label, following the existing dnf_status pattern. Part of ROX-32949. Co-authored-by: Cursor <cursoragent@cursor.com>
VMScraper.scrapeVM now classifies each report via the scan_trigger fact and calls IndexReportSender.SendReactive instead of Send for reactive ones, passing report_generated_at through for SLA latency measurement. Completes the ROX-32949 Phase 1 delivery path from roxagent's watcher through to Sensor's priority queue. Co-authored-by: Cursor <cursoragent@cursor.com>
Extends TestSendReactive_DeliveredBeforeQueuedScheduledBacklog to assert VirtualMachineReactiveIndexReportLatencySeconds observes exactly one sample for the reactive report and none for the two scheduled reports delivered alongside it, closing a gap flagged in code review: the SLA latency histogram had no direct test coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
|
Skipping CI for Draft Pull Request. |
Contributor
🚀 Build Images ReadyImages are ready for commit 1320470. To use with deploy scripts: export MAIN_IMAGE_TAG=4.12.x-417-g13204703d4 |
…comments docs/superpowers/specs/2026-07-03-reactive-dnf-scanning-design.md is a local planning artifact, not part of the repo, so code comments should not point readers to it. Removes the three "See docs/superpowers/..." references introduced across the ROX-32949 reactive-scan commits; the surrounding comments stand on their own without the pointer. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## piotr/ROX-35195-v2-vsock-pull-poc #21545 +/- ##
=====================================================================
+ Coverage 50.33% 50.36% +0.02%
=====================================================================
Files 2853 2854 +1
Lines 219121 219264 +143
=====================================================================
+ Hits 110290 110427 +137
- Misses 100815 100821 +6
Partials 8016 8016
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Requested during review of the reactive-scanning work: "combine into a single const block" for the three scanTrigger* declarations in facts.go. Purely cosmetic, no behavior change. Partially generated with AI assistance. Co-authored-by: Cursor <cursoragent@cursor.com>
Prompt: "I think there is an issue here... the index report with priority sent from one VM should not be sent earlier than any other particular VM... the reactive scan priority should be higher than the scheduled scan priority, but only within the same VM... if there is a reactive scan being added to that queue, then it should replace the triggered scan for the same VM, but it should not jump higher in the queue to take over other VMs." Confirmed as "Fair FIFO... not the priority FIFO" to avoid a super-active VM starving updates from other VMs under a downstream rate limiter. The previous design gave reactive reports a dedicated map drained ahead of the scheduled-report channel, so a VM with frequent DNF/RPM changes could delay every other VM's scheduled report indefinitely. handlerImpl.Send is now a single method (SendReactive removed) backed by one bounded channel of *slot values, plus a `pending` map keyed by vsock CID. A report for a VM that already has a queued slot mutates that slot in place (coalescing: latest data wins, queue position is unchanged); otherwise a new slot is appended, so delivery order across VMs is strict FIFO regardless of trigger type. `generatedAt` threads through the single Send call purely to feed the reactive-latency histogram, no longer to pick a queue. Same change mirrored in vmscraper.IndexReportSender/vmScraperSenderAdapter. Also fixes a bug found in code review of this rework: Stop() nil'd the pending map, but Send() only re-checks the stopper (not the map) before taking the slots lock, so a Send() racing Stop() could hit a nil-map write panic. Start() already reinitializes pending unconditionally, so Stop() now leaves it alone. Added TestSend_ConcurrentWithStopDoesNotPanicOnNilPendingMap, which reliably reproduced the panic before this fix. Partially generated with AI assistance. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Phase 1 of reactive DNF/RPM package-change scanning (ROX-32949). Today
roxagent serveonly rescans on a fixed 4h timer, so a package change can take up to 4h to reach Central. This adds anfsnotify-based watcher on the RPM database directory that triggers an immediate rescan on change, and makes Sensor prioritize delivery of these reactive reports so the ~5 minute update SLA actually holds under load.Design doc:
docs/superpowers/specs/2026-07-03-reactive-dnf-scanning-design.mdImplementation plan:
docs/superpowers/plans/2026-07-03-reactive-dnf-scanning-phase1.mdWhat changed
roxagent: newwatchpackage watches/usr/lib/sysimage/rpm(falling back to/var/lib/rpm) viafsnotify, with trivial buffered-channel coalescing of event bursts. Wired intorunServe's existingselectloop as a third case alongside the periodic ticker — no protocol changes. Watcher failures are best-effort (falls back to periodic-only scanning, logs a warning).scan_triggerfact (scheduled/reactive) via the existingResponseMeta.factsmap.VMScraperclassifies each pulled report via this fact and routes reactive ones through a newHandler.SendReactivepath instead of the routineSendpath.handlerImplgains a per-VM "latest pending reactive report" map (bounded by fleet size, not an arbitrary channel size) that's drained ahead of the routineindexReportschannel, so a reactive update can't get stuck behind a backlog of routine traffic.virtual_machine_reactive_index_report_latency_seconds(end-to-end SLA measurement) andvirtual_machine_discovered_data_scan_trigger_total(scheduled/reactive/unknown volume).Considered alternatives
factsmap is sufficient; a proto field is deferred to Phase 2 if scale testing shows it's needed).Explicitly out of scope (Phase 2, see design doc)
--reactive-scan/--reactive-debounceCLI flags.sensor/common/sensor/buffered_stream.gogRPC buffer (flagged in the design doc as a known, deliberately-unaddressed risk at scale).User-facing documentation
Testing and quality
Automated testing
How I validated my change
watchpackage (directory-preference resolution, event coalescing, event-type filtering),roxagent serve'sdiscoverFacts/scan_triggertagging, Sensor'sisReactiveTriggerclassification andscan_trigger/reactive-latency metrics, andhandlerImpl's priority path (delivery-ordering, per-VM upsert-replace semantics under concurrent access, fallback-to-scheduled-queue when a VM isn't yet resolvable, and the SLA-latency histogram observation).go test -race:compliance/virtualmachines/roxagent/...,sensor/common/virtualmachine/...,sensor/kubernetes/sensor/....go buildfor bothroxagentandsensor,gofmt,go vet, andgolangci-lintare clean;go mod tidyproduces no unexpected diff beyond promotingfsnotifyto a direct dependency.