diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index 94c57c3c..a52bd16b 100644 --- a/hscontrol/mapper/mapper.go +++ b/hscontrol/mapper/mapper.go @@ -18,7 +18,6 @@ import ( "tailscale.com/envknob" "tailscale.com/tailcfg" "tailscale.com/types/dnstype" - "tailscale.com/types/ptr" "tailscale.com/types/views" ) @@ -50,37 +49,6 @@ type mapper struct { created time.Time } -// addOnlineStatusToPeers adds fresh online status from batcher to peer nodes. -// -// We do a last-minute copy-and-write on the NodeView to inject current online status -// from the batcher's connection map. Online status is not populated upstream in NodeStore -// for consistency reasons - it's runtime connection state that should come from the -// connection manager (batcher) to ensure map responses have the freshest data. -func (m *mapper) addOnlineStatusToPeers(peers views.Slice[types.NodeView]) views.Slice[types.NodeView] { - if peers.Len() == 0 || m.batcher == nil { - return peers - } - - result := make([]types.NodeView, 0, peers.Len()) - for _, peer := range peers.All() { - if !peer.Valid() { - result = append(result, peer) - continue - } - - // Get online status from batcher connection map - // The batcher respects grace periods for logout scenarios - isOnline := m.batcher.IsConnected(peer.ID()) - - // Create a mutable copy and set online status - peerCopy := peer.AsStruct() - peerCopy.IsOnline = ptr.To(isOnline) - result = append(result, peerCopy.View()) - } - - return views.SliceOf(result) -} - type patch struct { timestamp time.Time change *tailcfg.PeerChange @@ -174,9 +142,6 @@ func (m *mapper) fullMapResponse( ) (*tailcfg.MapResponse, error) { peers := m.state.ListPeers(nodeID) - // Add fresh online status to peers from batcher connection state - // peersWithOnlineStatus := m.addOnlineStatusToPeers(peers) - return m.NewMapResponseBuilder(nodeID). WithCapabilityVersion(capVer). WithSelfNode(). @@ -219,9 +184,6 @@ func (m *mapper) peerChangeResponse( ) (*tailcfg.MapResponse, error) { peers := m.state.ListPeers(nodeID, changedNodeID) - // Add fresh online status to peers from batcher connection state - // peersWithOnlineStatus := m.addOnlineStatusToPeers(peers) - return m.NewMapResponseBuilder(nodeID). WithCapabilityVersion(capVer). WithSelfNode(). diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index fa9721cf..4e52eace 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -439,9 +439,21 @@ func (s *State) Connect(id types.NodeID) change.ChangeSet { c := change.NodeOnline(id) // Update the online status in NodeStore + now := time.Now() s.nodeStore.UpdateNode(id, func(n *types.Node) { n.IsOnline = ptr.To(true) + n.LastSeen = ptr.To(now) }) + + // Also persist the last seen time to the database + // Note: IsOnline is managed only in NodeStore (marked with gorm:"-"), not persisted to database + _, err := s.updateNodeTx(id, func(tx *gorm.DB) error { + // Update last_seen in the database + return hsdb.SetLastSeen(tx, id, now) + }) + if err != nil { + log.Error().Err(err).Uint64("node.id", id.Uint64()).Msg("Failed to update last seen time in database") + } // Get fresh node data from NodeStore after the online status update node, found := s.GetNodeByID(id) @@ -470,22 +482,28 @@ func (s *State) Disconnect(id types.NodeID) (change.ChangeSet, error) { now := time.Now() s.nodeStore.UpdateNode(id, func(n *types.Node) { n.LastSeen = ptr.To(now) - // Mark as offline immediately in NodeStore - this is the source of truth - // The batcher's grace period will still apply when sending to clients + // CRITICAL: Mark as offline immediately in NodeStore. + // NodeStore is the source of truth for all node state including online status. n.IsOnline = ptr.To(false) }) _, err := s.updateNodeTx(id, func(tx *gorm.DB) error { + // Update last_seen in the database + // Note: IsOnline is managed only in NodeStore (marked with gorm:"-"), not persisted to database return hsdb.SetLastSeen(tx, id, now) }) if err != nil { - return change.EmptySet, fmt.Errorf("setting last seen: %w", err) + // Log error but don't fail the disconnection - NodeStore is already updated + // and we need to send change notifications to peers + log.Error().Err(err).Uint64("node.id", id.Uint64()).Msg("Failed to update last seen in database") } // Check if policy manager needs updating c, err := s.updatePolicyManagerNodes() if err != nil { - return change.EmptySet, fmt.Errorf("failed to update policy manager after node update: %w", err) + // Log error but continue - disconnection must proceed + log.Error().Err(err).Uint64("node.id", id.Uint64()).Msg("Failed to update policy manager after node disconnect") + c = change.EmptySet } // The node is disconnecting so make sure that none of the routes it