Skip to content

fix(controller): restart an unhealthy backend on a same-config Start #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
T3ST3ST3R0N merged 2 commits into from
Oct 22, 2026
Merged

fix(controller): restart an unhealthy backend on a same-config Start #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fca6ace
fix(controller): restart an unhealthy backend on a same-config Start
T3ST3ST3R0N Oct 22, 2026
349fa3e
Merge remote-tracking branch 'origin/dev' into fix/start-heartbeat-ne…
T3ST3ST3R0N Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 30 additions & 9 deletions controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ const keepAliveGrace = 5 * time.Second
// user list takes xray longer than that to load.
const startTimeout = 3 * time.Minute

// heartbeatHealthTimeout bounds the health check that tells a heartbeat Start
// from a repair Start. It runs under LockControl, so a hung core API must not
// hold the lock for long.
var heartbeatHealthTimeout = 3 * time.Second

type Service interface {
Disconnect()
}
Expand Down Expand Up @@ -160,7 +165,7 @@ func sameBackendConfig(active, incoming *common.Backend) bool {
// StartOrAttach starts the core, or refreshes keep-alive if the same core is
// already running (heartbeat case). It only restarts when:
// - No backend is running, or
// - The running backend has stopped, or
// - The running backend has stopped or fails its health check, or
// - The incoming config differs from what started the current backend, and
// the backend cannot apply the difference live.
//
Expand All @@ -180,21 +185,27 @@ func (c *Controller) StartOrAttach(ctx context.Context, data *common.Backend) er
c.mu.RUnlock()

if sameBackendConfig(active, data) {
// Same config, backend healthy: this is a heartbeat. Just refresh.
log.Println("StartOrAttach: backend already running with same config, refreshing keep-alive.")
c.Connect(data.GetKeepAlive())
return nil
}
if applyConfigLive(ctx, back, active, data) {
err := backendHealthy(ctx, back)
if err == nil {
// Same config, backend healthy: this is a heartbeat. Just refresh.
log.Println("StartOrAttach: backend already running with same config, refreshing keep-alive.")
c.Connect(data.GetKeepAlive())
return nil
}
// A backend can report started while it is broken (a WireGuard
// interface that is gone), and the panel sends Start to repair it.
log.Printf("StartOrAttach: running backend with same config is unhealthy, restarting: %v", err)
} else if applyConfigLive(ctx, back, active, data) {
log.Println("StartOrAttach: config changed, applied to the running backend without a restart.")
c.mu.Lock()
c.activeData = data
c.mu.Unlock()
c.Connect(data.GetKeepAlive())
return nil
} else {
// Config changed while core is running: panel is sending a new config.
log.Println("StartOrAttach: config changed, restarting backend.")
}
// Config changed while core is running: panel is sending a new config.
log.Println("StartOrAttach: config changed, restarting backend.")
} else if back != nil {
// Backend object exists but has stopped.
log.Println("StartOrAttach: replacing a backend that is no longer running.")
Expand All @@ -213,6 +224,16 @@ func (c *Controller) StartOrAttach(ctx context.Context, data *common.Backend) er
return nil
}

// backendHealthy reports why a running backend cannot serve, or nil. It does not
// follow the caller's cancellation, so an abandoned request does not restart a
// healthy core.
func backendHealthy(ctx context.Context, back backend.Backend) error {
ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), heartbeatHealthTimeout)
defer cancel()
_, err := back.GetSysStats(ctx)
return err
}

// applyConfigLive asks a running backend to take a changed config without a
// restart. It reports false when the backend has to be restarted instead.
func applyConfigLive(ctx context.Context, back backend.Backend, active, incoming *common.Backend) bool {
Expand Down
89 changes: 86 additions & 3 deletions controller/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@ import (

"github.com/google/uuid"

"github.com/pasarguard/node/backend"
"github.com/pasarguard/node/common"
"github.com/pasarguard/node/config"
)

type stubBackend struct {
started atomic.Bool
shutdowns atomic.Int32
started atomic.Bool
shutdowns atomic.Int32
statsCalls atomic.Int32
statsErr error
statsBlocks bool // GetSysStats waits for its context to end
}

func (s *stubBackend) Started() bool { return s.started.Load() }
Expand All @@ -27,7 +31,15 @@ func (s *stubBackend) SyncUser(context.Context, *common.User) error { return nil
func (s *stubBackend) SyncUsers(context.Context, []*common.User) error { return nil }
func (s *stubBackend) UpdateUsers(context.Context, []*common.User) error { return nil }
func (s *stubBackend) UpdateUsersAndRestart(context.Context, []*common.User) error { return nil }
func (s *stubBackend) GetSysStats(context.Context) (*common.BackendStatsResponse, error) {
func (s *stubBackend) GetSysStats(ctx context.Context) (*common.BackendStatsResponse, error) {
s.statsCalls.Add(1)
if s.statsBlocks {
<-ctx.Done()
return nil, ctx.Err()
}
if s.statsErr != nil {
return nil, s.statsErr
}
return &common.BackendStatsResponse{}, nil
}
func (s *stubBackend) GetStats(context.Context, *common.StatRequest) (*common.StatResponse, error) {
Expand Down Expand Up @@ -82,6 +94,61 @@ func TestStartOrAttachHeartbeat(t *testing.T) {
if running.shutdowns.Load() != 0 {
t.Fatal("heartbeat: StartOrAttach shut down a running backend")
}
if running.statsCalls.Load() != 1 {
t.Fatalf("heartbeat: GetSysStats calls = %d, want 1", running.statsCalls.Load())
}
}

// startSameConfigOn sends a Start with the config the running stub was started with.
func startSameConfigOn(t *testing.T, running backend.Backend, started *atomic.Bool) (*Controller, error) {
t.Helper()
c := New(config.NewTestConfig(t.TempDir(), uuid.New()))
t.Cleanup(c.Disconnect)

started.Store(true)
c.backend = running
c.activeData = &common.Backend{Type: common.BackendType_WIREGUARD, Config: "cfg1", KeepAlive: 30}

c.LockControl()
defer c.UnlockControl()
return c, c.StartOrAttach(context.Background(), &common.Backend{Type: common.BackendType_WIREGUARD, Config: "cfg1", KeepAlive: 30})
}

// TestStartOrAttachRestartsAnUnhealthyBackendWithSameConfig: a WireGuard
// backend keeps reporting started after its interface is gone, so a repair
// Start with the same config must restart it instead of acting as a heartbeat.
func TestStartOrAttachRestartsAnUnhealthyBackendWithSameConfig(t *testing.T) {
broken := &stubBackend{statsErr: errors.New("wireguard interface not available")}
c, err := startSameConfigOn(t, broken, &broken.started)
if err == nil {
t.Fatal("expected StartBackend to fail with fake config")
}
if broken.shutdowns.Load() != 1 {
t.Fatalf("unhealthy backend shutdowns = %d, want 1", broken.shutdowns.Load())
}
if c.Backend() == broken {
t.Fatal("unhealthy backend is still attached")
}
}

// TestStartOrAttachBoundsTheHeartbeatHealthCheck: a hung core API must not
// hold LockControl for long; it counts as unhealthy.
func TestStartOrAttachBoundsTheHeartbeatHealthCheck(t *testing.T) {
previous := heartbeatHealthTimeout
heartbeatHealthTimeout = 50 * time.Millisecond
t.Cleanup(func() { heartbeatHealthTimeout = previous })

hung := &stubBackend{statsBlocks: true}
begin := time.Now()
if _, err := startSameConfigOn(t, hung, &hung.started); err == nil {
t.Fatal("expected StartBackend to fail with fake config")
}
if elapsed := time.Since(begin); elapsed > 5*time.Second {
t.Fatalf("StartOrAttach took %s with a hung health check", elapsed)
}
if hung.shutdowns.Load() != 1 {
t.Fatalf("hung backend shutdowns = %d, want 1", hung.shutdowns.Load())
}
}

// TestStartOrAttachConfigChanged: different config + running backend → restart.
Expand Down Expand Up @@ -153,6 +220,22 @@ func (s *liveStubBackend) ApplyConfigLive(context.Context, string, []string) (bo
return s.applied, s.err
}

// TestStartOrAttachDoesNotApplyLiveToAnUnhealthyBackend: an unhealthy backend
// with the same config is restarted, not handed the config again live.
func TestStartOrAttachDoesNotApplyLiveToAnUnhealthyBackend(t *testing.T) {
broken := &liveStubBackend{applied: true}
broken.statsErr = errors.New("xray api unavailable")
if _, err := startSameConfigOn(t, broken, &broken.started); err == nil {
t.Fatal("expected StartBackend to fail with fake config")
}
if broken.calls.Load() != 0 {
t.Fatalf("ApplyConfigLive calls = %d, want 0", broken.calls.Load())
}
if broken.shutdowns.Load() != 1 {
t.Fatalf("unhealthy backend shutdowns = %d, want 1", broken.shutdowns.Load())
}
}

func startOnLiveStub(t *testing.T, running *liveStubBackend) (*Controller, error) {
t.Helper()
c := New(config.NewTestConfig(t.TempDir(), uuid.New()))
Expand Down
Loading