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

Connect keeps 0.29's NodeOnlineFor peer patch; only the not-online branch
is new here. The updateChanges hunk is dropped: 0.29 has no caller for it.

(cherry picked from commit 80c863b15a)

hscontrol/poll.go

@@ -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
+		// A deleted or expired node cannot return online through a map
-		// client's next map request, and with it the re-authentication signal
+		// reconnect, so release its session without the reconnect grace.
-		// it needs. See: https://github.com/juanfont/headscale/issues/3410
+		// See: https://github.com/juanfont/headscale/issues/3410
-		_, nodeExists := m.h.state.GetNodeByID(m.node.ID)
+		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)

hscontrol/servertest/lifecycle_test.go

@@ -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

hscontrol/state/connect_test.go

@@ -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())
+		})
+	}
+}

hscontrol/state/state.go

@@ -643,10 +643,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()
 
@@ -658,7 +659,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 {
@@ -666,6 +667,9 @@ func (s *State) Connect(id types.NodeID) ([]change.Change, uint64) {
 	}
 
 	c := []change.Change{change.NodeOnlineFor(node)}
+	if !node.Online() {
+		c[0] = change.NodeAdded(node.ID())
+	}
 
 	log.Info().EmbedObject(node).Msg("node connected")
 
@@ -686,7 +690,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
@@ -713,7 +717,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
@@ -727,7 +731,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
 	}
@@ -921,13 +925,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 {
@@ -946,8 +957,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() {
+	// Resolve expiry and online status together from the current snapshot
-		c = change.NodeAdded(n.ID())
+	// 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
@@ -1128,7 +1142,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() {
@@ -1137,9 +1152,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) {
+		if !node.IsExpired() || !node.Expiry().Get().After(lastCheck) {
-			updates = append(updates, change.KeyExpiryFor(node.ID(), node.Expiry().Get()))
+			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 {
@@ -1412,8 +1452,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 !n.Online() || len(n.AllApprovedRoutes()) == 0 {
-			if !online || len(n.AllApprovedRoutes()) == 0 {
 				haHealthUpdates.WithLabelValues("rejected").Inc()
 
 				return
@@ -1723,12 +1762,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
+		// Preserve online state during re-registration so a live node does
-		// [State.Connect]/[State.Disconnect] in the poll session lifecycle.
+		// not appear offline before the client restarts its map stream.
-		// 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.
 		node.LastSeen = new(time.Now())
 
 		// On conversion (tagged → user) we set the new register method.
@@ -2613,10 +2648,8 @@ func (s *State) HandleNodeFromPreAuthKey(
 			}
 			node.AuthKey = pak
 			node.AuthKeyID = &pak.ID
-			// Do NOT reset IsOnline here. Online status is managed exclusively by
+			// Preserve online state during re-registration so a live node does
-			// [State.Connect]/[State.Disconnect] in the poll session lifecycle.
+			// not appear offline before the client restarts its map stream.
-			// Resetting it during re-registration causes a false offline blip
-			// to peers.
 			node.LastSeen = new(time.Now())
 
 			// Tagged nodes keep their existing expiry (disabled).

hscontrol/types/node.go

@@ -176,6 +176,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
@@ -184,10 +186,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
+	// ([State.Disconnect]) decrements it. Releasing the last session
-	// exactly when its last session ends — regardless of the order in
+	// takes the node offline; expiry can take it offline while sessions
-	// which overlapping sessions' cleanups run. Never persisted, like
+	// remain. Never persisted, like SessionEpoch.
-	// SessionEpoch.
 	ActiveSessions int `gorm:"-"`
 
 	// SessionEpoch identifies a poll session generation; Connect bumps
@@ -223,6 +224,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 {
@@ -1000,6 +1016,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 {