state: mark expired nodes offline without ending the session

Online now requires a live session and an unexpired key, derived in one
place by Node.ShouldBeOnline so every writer agrees what online means.

Fixes #3470
This commit is contained in:
Kristoffer Dalby
2026-09-15 16:15:59 +00:00
parent c90ba0f0d6
commit 80c863b15a
6 changed files with 349 additions and 35 deletions
+5 -5
View File
@@ -182,12 +182,12 @@ func (m *mapSession) serveLongPoll() {
// handler ran late is exactly such a session: if it kept its session
// acquired on this path, the surviving session's release could never
// take the node offline (the relogin flake).
// A deleted node cannot reconnect, so waiting for it only delays the
// client's next map request, and with it the re-authentication signal
// it needs. See: https://github.com/juanfont/headscale/issues/3410
_, nodeExists := m.h.state.GetNodeByID(m.node.ID)
// A deleted or expired node cannot return online through a map
// reconnect, so release its session without the reconnect grace.
// See: https://github.com/juanfont/headscale/issues/3410
node, nodeExists := m.h.state.GetNodeByID(m.node.ID)
if !stillConnected && nodeExists {
if !stillConnected && nodeExists && !node.IsExpired() {
// Wait up to 10 seconds for the node to reconnect.
// 10 seconds was arbitrary chosen as a reasonable time to reconnect.
ticker := time.NewTicker(time.Second)
+210
View File
@@ -4,12 +4,14 @@ import (
"context"
"fmt"
"math/rand/v2"
"net/netip"
"strings"
"testing"
"time"
"github.com/juanfont/headscale/hscontrol/servertest"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/types/change"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"tailscale.com/types/netmap"
@@ -122,6 +124,214 @@ func TestConnectionLifecycle(t *testing.T) {
})
}
// TestNodeExpiryPreservesControlConnection exercises the same-key map repoll
// that controlclient.Auto performs while the backend is in NeedsLogin.
func TestNodeExpiryPreservesControlConnection(t *testing.T) {
t.Parallel()
for _, scheduled := range []bool{false, true} {
t.Run(fmt.Sprintf("scheduled=%v", scheduled), func(t *testing.T) {
t.Parallel()
h := servertest.NewHarness(t, 2,
servertest.WithServerOptions(servertest.WithBatchDelay(10*time.Millisecond)),
)
client, observer := h.Client(0), h.Client(1)
id := findNodeID(t, h.Server, client.Name)
node, ok := h.Server.State().GetNodeByID(id)
require.True(t, ok)
epoch, nodeKey := node.SessionEpoch(), node.NodeKey()
require.True(t, node.IsOnline().Get())
lastCheck := time.Now()
expiry := lastCheck
if scheduled {
expiry = lastCheck.Add(time.Second)
}
node, c, err := h.Server.State().SetNodeExpiry(id, &expiry)
require.NoError(t, err)
require.Equal(t, scheduled, node.IsOnline().Get())
h.Server.App.Change(c)
if scheduled {
require.Eventually(t, func() bool { return time.Now().After(expiry) },
3*time.Second, 10*time.Millisecond)
_, changes, changed := h.Server.State().ExpireExpiredNodes(lastCheck)
require.True(t, changed)
h.Server.App.Change(changes...)
}
require.EventuallyWithT(t, func(c *assert.CollectT) {
current, found := h.Server.State().GetNodeByID(id)
if !assert.True(c, found) {
return
}
assert.False(c, current.IsOnline().Get())
assert.Equal(c, 1, current.ActiveSessions())
assert.Equal(c, epoch, current.SessionEpoch())
assert.True(c, h.Server.App.MapBatcher().IsConnected(id))
assert.True(c, client.Netmap().SelfNode.Expired())
assert.False(c, client.Netmap().SelfNode.Online().Get())
peer, found := observer.PeerByName(client.Name)
if assert.True(c, found) {
assert.True(c, peer.Expired())
assert.False(c, peer.Online().Get())
}
}, 5*time.Second, 10*time.Millisecond, "expiry must take the node offline without ending its control session")
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
require.NoError(t, client.RestartPoll(ctx))
require.EventuallyWithT(t, func(c *assert.CollectT) {
current, found := h.Server.State().GetNodeByID(id)
if !assert.True(c, found) {
return
}
assert.Greater(c, current.SessionEpoch(), epoch)
assert.Equal(c, 1, current.ActiveSessions())
assert.Equal(c, nodeKey, current.NodeKey())
assert.False(c, current.IsOnline().Get(), "an expired-key repoll cannot bring a node online")
assert.True(c, h.Server.App.MapBatcher().IsConnected(id))
}, 5*time.Second, 10*time.Millisecond, "expired-key repoll must preserve offline status")
// Restoring expiry must reach the client through that same stream.
current, _ := h.Server.State().GetNodeByID(id)
epoch = current.SessionEpoch()
_, c, err = h.Server.State().SetNodeExpiry(id, nil)
require.NoError(t, err)
h.Server.App.Change(c)
require.EventuallyWithT(t, func(c *assert.CollectT) {
current, found := h.Server.State().GetNodeByID(id)
if !assert.True(c, found) {
return
}
assert.True(c, current.IsOnline().Get())
assert.Equal(c, epoch, current.SessionEpoch())
assert.Equal(c, nodeKey, current.NodeKey())
assert.Equal(c, 1, current.ActiveSessions())
assert.False(c, client.Netmap().SelfNode.Expired())
assert.True(c, client.Netmap().SelfNode.Online().Get())
peer, found := observer.PeerByName(client.Name)
if assert.True(c, found) {
assert.False(c, peer.Expired())
assert.True(c, peer.Online().Get())
}
}, 5*time.Second, 10*time.Millisecond, "restoring expiry must recover both clients without another login or poll")
})
}
}
func TestRestoredExpirySurvivesQueuedChanges(t *testing.T) {
t.Parallel()
for _, full := range []bool{false, true} {
t.Run(fmt.Sprintf("full=%v", full), func(t *testing.T) {
t.Parallel()
h := servertest.NewHarness(t, 1,
servertest.WithServerOptions(servertest.WithBatchDelay(10*time.Millisecond)),
)
client := h.Client(0)
node := h.Server.State().ListNodes().At(0)
past := time.Now()
_, expired, err := h.Server.State().SetNodeExpiry(node.ID(), &past)
require.NoError(t, err)
future := past.Add(time.Hour)
_, restored, err := h.Server.State().SetNodeExpiry(node.ID(), &future)
require.NoError(t, err)
changes := []change.Change{expired, restored}
if full {
changes = append(changes, change.FullUpdate())
}
h.Server.App.Change(changes...)
client.WaitForCondition(t, "restored expiry delivered", 5*time.Second,
func(nm *netmap.NetworkMap) bool { return nm.SelfKeyExpiry().Equal(future) })
// Observe beyond delivery: the old worker cancelled the stream only
// after sending its map, even when that map had the restored expiry.
require.Never(t, func() bool { return len(h.ConnectedClients()) == 0 },
200*time.Millisecond, 10*time.Millisecond, "a queued expiry must not close the restored stream")
current, found := h.Server.State().GetNodeByID(node.ID())
require.True(t, found)
require.True(t, current.IsOnline().Get())
require.Equal(t, node.SessionEpoch(), current.SessionEpoch())
})
}
}
func TestNodeExpiryRouteFailover(t *testing.T) {
t.Parallel()
for _, scheduled := range []bool{false, true} {
t.Run(fmt.Sprintf("scheduled=%v", scheduled), func(t *testing.T) {
t.Parallel()
h := servertest.NewHarness(t, 3,
servertest.WithServerOptions(servertest.WithBatchDelay(10*time.Millisecond)),
)
route := netip.MustParsePrefix("10.70.0.0/24")
primary := advertiseAndApproveRoute(t, h.Server, h.Client(0), route)
standby := advertiseAndApproveRoute(t, h.Server, h.Client(1), route)
require.Contains(t, h.Server.State().GetNodePrimaryRoutes(primary), route)
lastCheck := time.Now()
expiry := lastCheck
if scheduled {
expiry = lastCheck.Add(time.Second)
}
_, c, err := h.Server.State().SetNodeExpiry(primary, &expiry)
require.NoError(t, err)
h.Server.App.Change(c)
if scheduled {
require.Eventually(t, func() bool { return time.Now().After(expiry) },
3*time.Second, 10*time.Millisecond)
_, changes, _ := h.Server.State().ExpireExpiredNodes(lastCheck)
h.Server.App.Change(changes...)
}
require.EventuallyWithT(t, func(c *assert.CollectT) {
assert.Empty(c, h.Server.State().GetNodePrimaryRoutes(primary))
assert.Contains(c, h.Server.State().GetNodePrimaryRoutes(standby), route)
assert.True(c, h.Server.App.MapBatcher().IsConnected(primary))
peer, found := h.Client(2).PeerByName(h.Client(1).Name)
if assert.True(c, found) {
assert.Contains(c, peer.PrimaryRoutes().AsSlice(), route)
}
}, 5*time.Second, 10*time.Millisecond, "expiry must move the route to the standby while preserving control connectivity")
_, c, err = h.Server.State().SetNodeExpiry(primary, nil)
require.NoError(t, err)
h.Server.App.Change(c)
_, c, err = h.Server.State().SetNodeExpiry(standby, &expiry)
require.NoError(t, err)
h.Server.App.Change(c)
require.EventuallyWithT(t, func(c *assert.CollectT) {
assert.Contains(c, h.Server.State().GetNodePrimaryRoutes(primary), route)
peer, found := h.Client(2).PeerByName(h.Client(0).Name)
if assert.True(c, found) {
assert.Contains(c, peer.PrimaryRoutes().AsSlice(), route)
}
}, 5*time.Second, 10*time.Millisecond, "restored router must be eligible for failover without reconnecting")
})
}
}
// TestLogoutReloginAllClientsConverge is an in-process reproduction of the
// flaky integration tests TestAuthKeyLogoutAndReloginSameUser,
// TestAuthWebFlowLogoutAndReloginSameUser and
+49
View File
@@ -1,8 +1,10 @@
package state
import (
"fmt"
"net/netip"
"testing"
"time"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/types/change"
@@ -282,3 +284,50 @@ func TestDisconnectOutOfOrderSessionsCannotStrandNodeOnline(t *testing.T) {
require.True(t, known)
assert.False(t, online, "node must be offline after its last session is released")
}
func TestExpiredNodeSessionAccounting(t *testing.T) {
for _, expiry := range []*time.Time{nil, new(time.Time{}), new(time.Now().Add(time.Hour))} {
t.Run(fmt.Sprint(expiry), func(t *testing.T) {
_, s, id := persistTestSetup(t)
t.Cleanup(func() { _ = s.Close() })
_, first := s.Connect(id)
past := time.Now()
node, _, err := s.SetNodeExpiry(id, &past)
require.NoError(t, err)
require.False(t, node.IsOnline().Get())
require.Equal(t, 1, node.ActiveSessions())
changes, second := s.Connect(id)
require.Greater(t, second, first)
for _, c := range changes {
for _, patch := range c.PeerPatches {
if patch.Online != nil {
require.False(t, *patch.Online)
}
}
}
node, ok := s.GetNodeByID(id)
require.True(t, ok)
require.False(t, node.IsOnline().Get())
require.Equal(t, 2, node.ActiveSessions())
_, err = s.Disconnect(id, second)
require.NoError(t, err)
node, _, err = s.SetNodeExpiry(id, expiry)
require.NoError(t, err)
require.True(t, node.IsOnline().Get(), "restoring a key with a live session restores online state")
require.Equal(t, 1, node.ActiveSessions())
_, err = s.Disconnect(id, first)
require.NoError(t, err)
node, _, err = s.SetNodeExpiry(id, expiry)
require.NoError(t, err)
require.False(t, node.IsOnline().Get(), "restoring expiry cannot connect a disconnected node")
require.Zero(t, node.ActiveSessions())
})
}
}
+1 -4
View File
@@ -210,10 +210,7 @@ func updateChanges(pre, post *types.Node) (bool, bool) {
return true, true
}
wasOnline := pre.IsOnline != nil && *pre.IsOnline
isOnline := post.IsOnline != nil && *post.IsOnline
return false, wasOnline != isOnline || pre.Unhealthy != post.Unhealthy
return false, pre.Online() != post.Online() || pre.Unhealthy != post.Unhealthy
}
// PutNode adds or updates a node in the store.
+55 -22
View File
@@ -634,10 +634,11 @@ func (s *State) DeleteNode(node types.NodeView) (change.Change, error) {
return c, nil
}
// Connect marks a node connected and returns the resulting changes
// Connect acquires a control session and returns the resulting changes
// plus a session epoch identifying this poll session. Every Connect
// acquires one live session; the caller must release it with exactly
// one [State.Disconnect] call once the session ends (see poll.go).
// An expired key can keep polling control, but cannot make the node online.
func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) {
prevRoutes := s.nodeStore.PrimaryRoutes()
@@ -649,7 +650,7 @@ func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) {
n.SessionEpoch++
epoch = n.SessionEpoch
n.ActiveSessions++
n.IsOnline = new(true)
n.IsOnline = new(n.ShouldBeOnline())
n.Unhealthy = false
})
if !ok {
@@ -660,6 +661,9 @@ func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) {
// routers, relay targets, and via targets get their full peer recompute
// from the gated PolicyChange below, so no full update is needed here.
c := []change.Change{change.NodeOnline(node.ID())}
if !node.Online() {
c[0] = change.NodeAdded(node.ID())
}
log.Info().EmbedObject(node).Msg("node connected")
@@ -680,7 +684,7 @@ func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) {
}
// Disconnect releases one poll session previously acquired by
// [State.Connect] and marks the node offline only when that was its
// [State.Connect] and marks the node offline when that was its
// last live session. Sessions are counted rather than compared by
// epoch: overlapping sessions for one node — a rapid reconnect, or a
// cancelled map request whose handler ran late — release in any order
@@ -707,7 +711,7 @@ func (s *State) Disconnect(id types.NodeID, epoch uint64) ([]change.Change, erro
now := time.Now()
n.LastSeen = &now
n.IsOnline = new(false)
n.IsOnline = new(n.ShouldBeOnline())
// Offline nodes are not HA candidates; drop any stale
// Unhealthy bit so it does not surface in DebugRoutes.
n.Unhealthy = false
@@ -721,7 +725,7 @@ func (s *State) Disconnect(id types.NodeID, epoch uint64) ([]change.Change, erro
log.Debug().
Uint64("disconnect_epoch", epoch).
Int("active_sessions", node.ActiveSessions()).
Msg("session released, other sessions keep node online")
Msg("session released, other control sessions remain")
return nil, nil
}
@@ -906,13 +910,20 @@ func (s *State) ListEphemeralNodes() views.Slice[types.NodeView] {
// SetNodeExpiry updates the expiration time for a node.
// If expiry is nil, the node's expiry is disabled (node will never expire).
func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.NodeView, change.Change, error) {
var onlineChanged bool
// Update [NodeStore] before database to ensure consistency. The [NodeStore] update
// is blocking and will be the source of truth for the batcher. The database update
// must make the exact same change. If the database update fails, the [NodeStore]
// change will remain, but since we return an error, no change notification will be
// sent to the batcher, preventing inconsistent state propagation.
n, ok := s.nodeStore.UpdateNode(nodeID, func(node *types.Node) {
wasOnline := node.Online()
node.Expiry = expiry
// Control stays connected in NeedsLogin so an expiry extension can
// recover the client, but an expired key is not online.
node.IsOnline = new(node.ShouldBeOnline())
onlineChanged = wasOnline != node.Online()
})
if !ok {
@@ -931,8 +942,11 @@ func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.Nod
return n, change.Change{}, fmt.Errorf("updating policy manager after setting expiry: %w", err)
}
if c.IsEmpty() {
c = change.NodeAdded(n.ID())
// Resolve expiry and online status together from the current snapshot
// when the mapper sends the change, including after a rapid restoration.
c = c.Merge(change.NodeAdded(n.ID()))
if onlineChanged && s.polMan.NodeNeedsPeerRecompute(n) {
c = c.Merge(change.PolicyChange())
}
return n, c, nil
@@ -1113,7 +1127,8 @@ func (s *State) ExpireExpiredNodes(lastCheck time.Time) (time.Time, []change.Cha
// while this function is running by using a consistent timestamp for the next check
started := time.Now()
var updates []change.Change
nodeUpdates := make(map[types.NodeID]UpdateNodeFunc)
expiredNodes := make(map[types.NodeID]bool)
for _, node := range s.nodeStore.ListNodes().All() { //nolint:unqueryvet // NodeStore.ListNodes not a SQL query
if !node.Valid() {
@@ -1122,9 +1137,34 @@ func (s *State) ExpireExpiredNodes(lastCheck time.Time) (time.Time, []change.Cha
// Why check After(lastCheck): We only want to notify about nodes that
// expired since the last check to avoid duplicate notifications
if node.IsExpired() && node.Expiry().Valid() && node.Expiry().Get().After(lastCheck) {
updates = append(updates, change.KeyExpiryFor(node.ID(), node.Expiry().Get()))
if !node.IsExpired() || !node.Expiry().Get().After(lastCheck) {
continue
}
nodeUpdates[node.ID()] = func(n *types.Node) {
// The key may have been restored since the snapshot was read.
if !n.IsExpired() || !n.Expiry.After(lastCheck) {
return
}
expiredNodes[n.ID] = n.Online()
n.IsOnline = new(n.ShouldBeOnline())
}
}
// Publish simultaneous expirations together so route election sees all
// unavailable nodes in one snapshot.
s.nodeStore.UpdateNodes(nodeUpdates)
updates := make([]change.Change, 0, len(expiredNodes))
for id, wasOnline := range expiredNodes {
c := change.NodeAdded(id)
if current, ok := s.nodeStore.GetNode(id); ok && wasOnline && s.polMan.NodeNeedsPeerRecompute(current) {
c = c.Merge(change.PolicyChange())
}
updates = append(updates, c)
}
if len(updates) > 0 {
@@ -1403,8 +1443,7 @@ var haHealthUpdates = promauto.NewCounterVec(prometheus.CounterOpts{
func healthSetter(healthy bool) UpdateNodeFunc {
return func(n *types.Node) {
if !healthy {
online := n.IsOnline != nil && *n.IsOnline
if !online || len(n.AllApprovedRoutes()) == 0 {
if !n.Online() || len(n.AllApprovedRoutes()) == 0 {
haHealthUpdates.WithLabelValues("rejected").Inc()
return
@@ -1805,12 +1844,8 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
if len(regData.Endpoints) > 0 {
node.Endpoints = regData.Endpoints
}
// Do NOT reset IsOnline here. Online status is managed exclusively by
// [State.Connect]/[State.Disconnect] in the poll session lifecycle.
// Resetting it during re-registration causes a false offline blip: the
// change notification triggers a map regeneration showing the node as
// offline to peers, even though [State.Connect] will immediately set it
// back to true.
// Preserve online state during re-registration so a live node does
// not appear offline before the client restarts its map stream.
node.LastSeen = new(time.Now())
// On conversion (tagged → user) we set the new register method.
@@ -2696,10 +2731,8 @@ func (s *State) HandleNodeFromPreAuthKey(
node.AuthKey = pak
node.AuthKeyID = &pak.ID
// Do NOT reset IsOnline here. Online status is managed exclusively by
// [State.Connect]/[State.Disconnect] in the poll session lifecycle.
// Resetting it during re-registration causes a false offline blip
// to peers.
// Preserve online state during re-registration so a live node does
// not appear offline before the client restarts its map stream.
node.LastSeen = new(time.Now())
// Tagged nodes keep their existing expiry (disabled).
+29 -4
View File
@@ -181,6 +181,8 @@ type Node struct {
UpdatedAt time.Time
DeletedAt *time.Time
// IsOnline caches [Node.ShouldBeOnline]; read it through [Node.Online].
// Every writer must derive it, so online means the same thing everywhere.
IsOnline *bool `gorm:"-"`
// Unhealthy excludes the node from primary route election while
@@ -189,10 +191,9 @@ type Node struct {
// ActiveSessions counts live poll sessions for this node.
// [State.Connect] increments it and every session release
// ([State.Disconnect]) decrements it, so the node goes offline
// exactly when its last session ends — regardless of the order in
// which overlapping sessions' cleanups run. Never persisted, like
// SessionEpoch.
// ([State.Disconnect]) decrements it. Releasing the last session
// takes the node offline; expiry can take it offline while sessions
// remain. Never persisted, like SessionEpoch.
ActiveSessions int `gorm:"-"`
// SessionEpoch identifies a poll session generation; Connect bumps
@@ -228,6 +229,21 @@ func (node *Node) IsExpired() bool {
return time.Since(*node.Expiry) > 0
}
// Online reports the node's last known connectivity. Unknown counts as
// offline. Use [Node.ShouldBeOnline] to derive the value, not to read it.
func (node *Node) Online() bool {
return node.IsOnline != nil && *node.IsOnline
}
// ShouldBeOnline derives what [Node.IsOnline] must hold from its two inputs:
// a live control session and an unexpired node key. An expired client keeps
// polling control to receive auth updates, so a session alone is not enough.
// Call it only from a NodeStore write closure, where ActiveSessions is
// stable; elsewhere read [Node.Online].
func (node *Node) ShouldBeOnline() bool {
return node.ActiveSessions > 0 && !node.IsExpired()
}
// IsEphemeral returns if the node is registered as an Ephemeral node.
// https://tailscale.com/docs/features/ephemeral-nodes
func (node *Node) IsEphemeral() bool {
@@ -917,6 +933,15 @@ func (nv NodeView) IsExpired() bool {
return nv.ж.IsExpired()
}
// Online reports the node's last known connectivity.
func (nv NodeView) Online() bool {
if !nv.Valid() {
return false
}
return nv.ж.Online()
}
// IsEphemeral returns if the node is registered as an Ephemeral node.
// https://tailscale.com/docs/features/ephemeral-nodes
func (nv NodeView) IsEphemeral() bool {