Skip to content

[client] Restart engine when peer IP address changes#5614

Merged
lixmal merged 2 commits intomainfrom
restart-on-ip-change
Mar 17, 2026
Merged

[client] Restart engine when peer IP address changes#5614
lixmal merged 2 commits intomainfrom
restart-on-ip-change

Conversation

@lixmal
Copy link
Copy Markdown
Collaborator

@lixmal lixmal commented Mar 17, 2026

Describe your changes

When management reallocates peer IPs (e.g. after a network range change), the client now restarts the engine instead of just logging the change. The firewall, route manager, and other components cache the old address, so a restart is needed for the new IP to take effect.

Uses the same ErrResetConnection + clientCancel() pattern as the network monitor and interface monitor restarts.

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Not needed: no user-facing API or configuration change.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of network interface address changes: the client now performs a clean restart to apply new IP configurations, ensuring updates take effect reliably.
    • Enhanced restart behavior and logging for clearer diagnostics and more robust connection recovery.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f3881af1-ae78-47bd-b0b7-774eb0895668

📥 Commits

Reviewing files that changed from the base of the PR and between f9d596b and 527b904.

📒 Files selected for processing (1)
  • client/internal/engine.go

📝 Walkthrough

Walkthrough

Detects when the WireGuard interface IP differs from the new config and triggers a client restart: logs the restart, wraps ErrResetConnection into the context state, cancels the client, and returns ErrResetConnection. Remaining function behavior unchanged. (≤50 words)

Changes

Cohort / File(s) Summary
IP Change Detection & Client Restart
client/internal/engine.go
Replaces a comment with active logic: on WireGuard interface IP mismatch, log restart, set context state to ErrResetConnection, cancel current client operations, and return ErrResetConnection. Minor log text update.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • pappz

Poem

🐰 I sniffed the network, found a new IP tune,
I thumped my paw and hummed to the moon.
A gentle restart, a hop, then the mend—
Connections reset, and all's well again. 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title '[client] Restart engine when peer IP address changes' is clear, concise, and directly describes the main change in the PR—implementing engine restart logic when peer IP addresses are reallocated.
Description check ✅ Passed The description is comprehensive and well-structured. It explains the motivation (cached components), implementation approach (reusing ErrResetConnection pattern), and provides proper documentation justification. All major template sections are addressed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch restart-on-ip-change
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/internal/engine.go`:
- Around line 993-997: The restart path currently nulls out the wrapped error
and returns nil, allowing updateNetworkMap to continue; change the branch so it
returns the non-nil wrapped error instead of nil: return the result of
CtxGetState(e.ctx).Wrap(ErrResetConnection) after calling e.clientCancel()
(i.e., do not assign to `_`), so the caller (updateNetworkMap) short-circuits
further processing when e.clientCancel() is invoked.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 828d9746-7245-4979-86f3-37193fdd5304

📥 Commits

Reviewing files that changed from the base of the PR and between 628eb56 and f9d596b.

📒 Files selected for processing (1)
  • client/internal/engine.go

Comment thread client/internal/engine.go
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

@lixmal lixmal merged commit af8eaa2 into main Mar 17, 2026
41 checks passed
@lixmal lixmal deleted the restart-on-ip-change branch March 17, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants