Skip to content

refactor(ipscanner): isolate scanner activations (3/9) - #782

Merged
ankitgoswami merged 2 commits into
mainfrom
ankitg/runtimejobs-ipscanner
Jul 22, 2026
Merged

refactor(ipscanner): isolate scanner activations (3/9)#782
ankitgoswami merged 2 commits into
mainfrom
ankitg/runtimejobs-ipscanner

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +90/-53 across 2 files (excludes generated, test, and story files).

Summary

IP discovery now has activation-scoped queues and bounded shutdown, preventing one Fleet activation from leaking scan work or results into the next. Standalone scanning keeps its current startup behavior while gaining a clean Start -> Stop -> Start path.

Stack:

  • Foundation: #780 (merged)
  • Parallel domain refactors: diagnostics #781, IP scanning #782, scheduling #783, curtailment #785, command execution #787, and telemetry #786
  • Orchestration: #784
  • Catalog and fleetd cutover: #788

This PR is the IP scanner lifecycle slice and now targets main. Merged #780 supplies the shared runtimejobs.Lifecycle contract; the remaining domain and orchestration refactors are independent sibling PRs and can merge in any order. #788 is temporarily based on the integration branch so its reviewable diff contains only catalog/cutover work; after the siblings merge it will be rebased and retargeted to main. Passive-mode coordinator wiring, epoch fencing, and request gating remain later HA work.

How it works

Before this PR, the scanner's task queue, result queue, cancellation function, and goroutine tracking belonged to the long-lived service and assumed one activation for the lifetime of the process. Adding a lifecycle interface around that state would not make it safely restartable: buffered work could cross into a later activation, a worker blocked while publishing a result could prevent shutdown, and a timed-out stop could otherwise be followed by an overlapping start.

Each successful Start now creates fresh task and result channels plus service-owned cancellation, then launches the scanner workers for that activation. Worker sends select on cancellation, so a full result queue cannot trap shutdown. Stop cancels producers and consumers, waits for every activation-owned goroutine within the caller's deadline, and only then permits restart. The underlying discovery flow—selecting offline devices, grouping them by subnet, verifying identity, and persisting a corrected address—remains unchanged.

flowchart LR
    S["Start activation"] --> Q["Fresh task and result queues"]
    Q --> W["Scanner workers"]
    W --> C["Cancellation-aware result send"]
    X["Stop"] --> C
    X --> D["Drain activation goroutines"]
    D --> R["Fresh later activation"]
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
server/internal/domain/ipscanner/service.go Adds lifecycle state, per-run queues, cancellation-aware sends, and bounded drain Prevents cross-activation results and shutdown hangs
server/cmd/fleetd/main.go Routes scanner teardown through the bounded standalone helper Preserves process shutdown limits
IP scanner tests Add restart, blocked-send cancellation, and stop-deadline coverage Exercises the concurrency edges introduced by activation switching

Key technical decisions & trade-offs

  • Allocate queues per activation rather than draining shared queues after stop; ownership is clearer and stale results become unreachable.
  • Keep a timed-out activation in stopping state so restart cannot overlap surviving scanner work.

Related

Related: #740

Testing & validation

  • go test -short -race -count=1 ./internal/domain/ipscanner ./cmd/fleetd
  • Covers duplicate start, restart, full result queue cancellation, and caller stop deadlines.

Post-Deploy Monitoring & Validation

Watch IP scanner errors, scan completion rates, and shutdown drain logs through one normal scan interval. Roll back if scans stop being scheduled, discovered devices stop appearing, or scanner shutdown repeatedly exceeds its budget.


Compound Engineering

@github-actions github-actions Bot added server review-policy: needs-review Managed by the Review Policy workflow. labels Jul 21, 2026
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

  • Reviewed pull request diff only (4bf7e7a0e3c126f3cf358badbdd50fc12a4a0860...3cc1605c80af827234c81196feec25a249bf0ce2, exact PR three-dot diff)
  • Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.


Review Summary

Overall Risk: NONE

Findings

No security, correctness, or reliability findings were identified in the scoped diff.

Notes

Reviewed only .git/codex-review.diff for commit 3cc1605c80af827234c81196feec25a249bf0ce2. The diff is limited to IP scanner lifecycle shutdown/restart handling plus tests. I could not run Go tests in this read-only sandbox because the Go tool could not create its module/cache directories, so this is a static review.


Generated by Codex Security Review |
Triggered by: @ankitgoswami |
Review workflow run

@ankitgoswami
ankitgoswami force-pushed the ankitg/runtimejobs-ipscanner branch from f422020 to 1c7da92 Compare July 21, 2026 20:22
@ankitgoswami
ankitgoswami marked this pull request as ready for review July 21, 2026 22:42
@ankitgoswami
ankitgoswami requested a review from a team as a code owner July 21, 2026 22:42
Copilot AI review requested due to automatic review settings July 21, 2026 22:42

This comment was marked as low quality.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c7da92519

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/ipscanner/service.go Outdated
- Keep scanner activations alive after startup context cancellation
- Add regression coverage for lifecycle-owned cancellation
@github-actions github-actions Bot added review-policy: human-approved Managed by the Review Policy workflow. and removed review-policy: needs-review Managed by the Review Policy workflow. labels Jul 22, 2026
@ankitgoswami
ankitgoswami merged commit 24be148 into main Jul 22, 2026
71 checks passed
@ankitgoswami
ankitgoswami deleted the ankitg/runtimejobs-ipscanner branch July 22, 2026 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-policy: human-approved Managed by the Review Policy workflow. server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants