[PR #6707] [MERGED] [client] Restore residual state in foreground mode before login #29820

Closed
opened 2026-08-05 08:09:04 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6707
Author: @dfry
Created: 7/9/2026
Status: Merged
Merged: 7/14/2026
Merged by: @theodorsm

Base: mainHead: fix/foreground-residual-state


📝 Commits (3)

  • 95401a4 [client] Restore residual state in foreground mode before login
  • 8811866 [client] Enable advanced routing in foreground mode before login
  • 2d08be3 [client] Correct RestoreResidualState comments per review

📊 Changes

4 files changed (+36 additions, -6 deletions)

View changed files

📝 client/cmd/login.go (+10 -0)
📝 client/cmd/up.go (+20 -0)
📝 client/server/server.go (+3 -3)
📝 client/server/state.go (+3 -3)

📄 Description

Describe your changes

netbird up --foreground-mode goes straight to foregroundLogin without the residual-state recovery the daemon performs — restoreResidualState is called at three points in client/server before touching management, with a comment describing exactly why: "Otherwise, we might not be able to connect to the management server to retrieve new config." The foreground path never calls it.

If a previous run did not shut down cleanly (crash/kill — deferred cleanup never runs), leftover routing policy from SetupRouting (e.g. the not fwmark → NetbirdVPNTableID rule) can shunt the management connection into a stale routing table whose default route points at a dead wt0, and every subsequent login times out.

Foreground mode is particularly exposed in containers (its primary use): a crashed container restarts inside the same pod network namespace, so the stale rules survive while the process state does not. Observed in the field: a netbird router container crash-looped ~99 times over 8 hours on

Error: foreground login failed: ... dial tcp <mgmt-ip>:443: i/o timeout

while other pods in the same namespace reached the same management endpoint fine — only deleting the pod (fresh netns) recovered it. Daemon-mode installs never hit this, which is likely why it hasn't been reported.

Fix: export RestoreResidualState and call it in runInForegroundMode before foregroundLogin, mirroring the daemon's behaviour (warn and continue on error).

go build ./client/... and go vet clean; client/server tests pass (client/cmd's TestRemoveStaleUnixSocket* failures are pre-existing on main in my environment, unchanged by this PR).

None — field incident described above; happy to open an issue if preferred.

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)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

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)

Documentation is not needed: this fixes an internal crash-recovery omission — foreground mode now performs the same residual-state cleanup the daemon already does. No user-facing behaviour, flags, API, or configuration change.

Summary by CodeRabbit

  • Bug Fixes
    • Improved residual state restoration during foreground startup and foreground login, ensuring consistent routing behavior.
    • Residual-state restore errors now warn without interrupting the ongoing start or login flow.
    • Residual-state cleanup now consistently performs restore/persist steps to keep residual handling accurate.
    • Foreground flows now initialize networking earlier so routing is available when login proceeds.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6707 **Author:** [@dfry](https://github.com/dfry) **Created:** 7/9/2026 **Status:** ✅ Merged **Merged:** 7/14/2026 **Merged by:** [@theodorsm](https://github.com/theodorsm) **Base:** `main` ← **Head:** `fix/foreground-residual-state` --- ### 📝 Commits (3) - [`95401a4`](https://github.com/netbirdio/netbird/commit/95401a492b7c9c0f02ed7f1218f35a2fc98f085c) [client] Restore residual state in foreground mode before login - [`8811866`](https://github.com/netbirdio/netbird/commit/8811866cbaca84ae99616a7f434ac86960b6a314) [client] Enable advanced routing in foreground mode before login - [`2d08be3`](https://github.com/netbirdio/netbird/commit/2d08be39689428204d99cfaf0d30a8eab9f05569) [client] Correct RestoreResidualState comments per review ### 📊 Changes **4 files changed** (+36 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `client/cmd/login.go` (+10 -0) 📝 `client/cmd/up.go` (+20 -0) 📝 `client/server/server.go` (+3 -3) 📝 `client/server/state.go` (+3 -3) </details> ### 📄 Description ## Describe your changes `netbird up --foreground-mode` goes straight to `foregroundLogin` without the residual-state recovery the daemon performs — `restoreResidualState` is called at three points in `client/server` before touching management, with a comment describing exactly why: *"Otherwise, we might not be able to connect to the management server to retrieve new config."* The foreground path never calls it. If a previous run did not shut down cleanly (crash/kill — deferred cleanup never runs), leftover routing policy from `SetupRouting` (e.g. the `not fwmark → NetbirdVPNTableID` rule) can shunt the management connection into a stale routing table whose default route points at a dead `wt0`, and every subsequent login times out. **Foreground mode is particularly exposed in containers** (its primary use): a crashed container restarts inside the same pod network namespace, so the stale rules survive while the process state does not. Observed in the field: a netbird router container crash-looped ~99 times over 8 hours on ``` Error: foreground login failed: ... dial tcp <mgmt-ip>:443: i/o timeout ``` while other pods in the same namespace reached the same management endpoint fine — only deleting the pod (fresh netns) recovered it. Daemon-mode installs never hit this, which is likely why it hasn't been reported. **Fix:** export `RestoreResidualState` and call it in `runInForegroundMode` before `foregroundLogin`, mirroring the daemon's behaviour (warn and continue on error). `go build ./client/...` and `go vet` clean; `client/server` tests pass (`client/cmd`'s `TestRemoveStaleUnixSocket*` failures are pre-existing on `main` in my environment, unchanged by this PR). ## Issue ticket number and link None — field incident described above; happy to open an issue if preferred. ### Checklist - [x] 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) - [x] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) Documentation is not needed: this fixes an internal crash-recovery omission — foreground mode now performs the same residual-state cleanup the daemon already does. No user-facing behaviour, flags, API, or configuration change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved residual state restoration during foreground startup and foreground login, ensuring consistent routing behavior. * Residual-state restore errors now warn without interrupting the ongoing start or login flow. * Residual-state cleanup now consistently performs restore/persist steps to keep residual handling accurate. * Foreground flows now initialize networking earlier so routing is available when login proceeds. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:09:04 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#29820