[PR #6575] [MERGED] [client] report management unhealthy while Sync stream is failing #29676

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

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6575
Author: @pappz
Created: 6/28/2026
Status: Merged
Merged: 6/29/2026
Merged by: @pappz

Base: mainHead: fix/mgm-health-check


📝 Commits (2)

  • 561084d fix(mgm): report management unhealthy while Sync stream is failing
  • 16c39cc fix(mgm): record disconnected state when sync stream setup fails

📊 Changes

1 file changed (+37 additions, -0 deletions)

View changed files

📝 shared/management/client/grpc.go (+37 -0)

📄 Description

Describe your changes

The health probe (IsHealthy) only checked the gRPC transport and a GetServerKey call. GetServerKey succeeds even when the peer cannot sync (e.g. the server returns "settings not found"), so the probe kept marking management Connected while the Sync stream failed in a tight retry loop — pinning the status to "Connected" forever despite no sync ever succeeding.

Track the last Sync stream error and have IsHealthy consult it, so a healthy transport is no longer enough to report the connection healthy.

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)
  • 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)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes
    • Improved management connection health reporting so sync streaming failures are no longer incorrectly treated as healthy.
    • Health checks now more accurately reflect the combined connection status, preventing misleading “connected” indicators and ensuring disconnect handling triggers when sync streaming breaks.

🔄 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/6575 **Author:** [@pappz](https://github.com/pappz) **Created:** 6/28/2026 **Status:** ✅ Merged **Merged:** 6/29/2026 **Merged by:** [@pappz](https://github.com/pappz) **Base:** `main` ← **Head:** `fix/mgm-health-check` --- ### 📝 Commits (2) - [`561084d`](https://github.com/netbirdio/netbird/commit/561084d67e1bd81c8e8bbc9cc92d2c2bf7c951c2) fix(mgm): report management unhealthy while Sync stream is failing - [`16c39cc`](https://github.com/netbirdio/netbird/commit/16c39ccb39bad85a02f535756f99d0f8a8ecc326) fix(mgm): record disconnected state when sync stream setup fails ### 📊 Changes **1 file changed** (+37 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `shared/management/client/grpc.go` (+37 -0) </details> ### 📄 Description ## Describe your changes The health probe (IsHealthy) only checked the gRPC transport and a GetServerKey call. GetServerKey succeeds even when the peer cannot sync (e.g. the server returns "settings not found"), so the probe kept marking management Connected while the Sync stream failed in a tight retry loop — pinning the status to "Connected" forever despite no sync ever succeeding. Track the last Sync stream error and have IsHealthy consult it, so a healthy transport is no longer enough to report the connection healthy. ## Issue ticket number and link ## Stack <!-- branch-stack --> ### 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) - [ ] 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) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved management connection health reporting so sync streaming failures are no longer incorrectly treated as healthy. * Health checks now more accurately reflect the combined connection status, preventing misleading “connected” indicators and ensuring disconnect handling triggers when sync streaming breaks. <!-- 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:08:39 -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#29676