state, change: send the last seen time in the offline patch

As SaaS, on every offline patch and no online one. Peers otherwise show no
last-seen time until the next full map. SaaS sends a coarser time; the exact
one here is deliberate.

Updates #3626
This commit is contained in:
Kristoffer Dalby committed 2026-11-03 16:29:03 +01:00
1 parent 5083d77731
commit ef6d033d19
13 files changed
+101 -28

No files matched your search

+1 -1
View File
@@ -125,7 +125,7 @@ func TestAppendPendingBoundsFoldedPatches(t *testing.T) {
if i%2 == 0 {
nc.appendPending(change.NodeOnline(2))
} else {
nc.appendPending(change.NodeOffline(2))
nc.appendPending(change.NodeOffline(2, time.Time{}))
}
}
+3 -3
View File
@@ -287,7 +287,7 @@ func TestAddToBatchDifferential(t *testing.T) {
change.SelfUpdate(88), ping, change.NodeOnline(2),
}},
{"repeated_kinds", []change.Change{
change.NodeOnline(2), change.NodeOffline(2), change.NodeOnline(2),
change.NodeOnline(2), change.NodeOffline(2, time.Time{}), change.NodeOnline(2),
change.PeersChanged("several peers", 3, 1),
}},
{"deletion", []change.Change{
@@ -359,7 +359,7 @@ func TestAddToBatchRecipientStorage(t *testing.T) {
lb := setupLightweightBatcher(t, 2, 1)
t.Cleanup(lb.cleanup)
incoming := []change.Change{change.NodeOnline(1), change.NodeOffline(1)}
incoming := []change.Change{change.NodeOnline(1), change.NodeOffline(1, time.Time{})}
impl.add(lb.b, incoming...)
incoming[0] = change.FullUpdate()
@@ -370,7 +370,7 @@ func TestAddToBatchRecipientStorage(t *testing.T) {
assert.Equal(
t,
[]change.Change{change.NodeOnline(1), change.NodeOffline(1)},
[]change.Change{change.NodeOnline(1), change.NodeOffline(1, time.Time{})},
getPendingForNode(lb.b, 2),
"recipients must own their pending slices",
)
+1 -1
View File
@@ -94,7 +94,7 @@ func (t *testBatcherWrapper) RemoveNode(id types.NodeID, c chan<- *tailcfg.MapRe
// Do this BEFORE removing from batcher so the change can be processed
node, ok := t.state.GetNodeByID(id)
if ok {
t.AddWork(change.NodeOffline(node.ID()))
t.AddWork(change.NodeOffline(node.ID(), time.Time{}))
}
// Finally remove from the real batcher
+1 -1
View File
@@ -186,7 +186,7 @@ func TestHeldChangesKeepOrderWhenStreamTurnsReady(t *testing.T) {
name: "online_then_offline",
bundle: func(p, _ types.NodeID) []change.Change {
return []change.Change{
change.NodeOnline(p), change.DERPMap(), change.NodeOffline(p),
change.NodeOnline(p), change.DERPMap(), change.NodeOffline(p, time.Time{}),
}
},
want: func(p, _ types.NodeID) []string {
+1 -1
View File
@@ -921,7 +921,7 @@ func TestMapResponseNeverContainsSelfAsPeer(t *testing.T) {
"policy change": change.PolicyChange(),
"self added": change.NodeAdded(recipient),
"self online": change.NodeOnline(recipient),
"self offline": change.NodeOffline(recipient),
"self offline": change.NodeOffline(recipient, time.Time{}),
// A batch naming every node, the recipient included.
// change.PeersChanged carries no OriginNode, so the
// self-update short circuit in buildFromChange never fires
+17 -4
View File
@@ -70,9 +70,10 @@ type TestClient struct {
// streams and streamMaps: see [TestClient.Stream].
streams, streamMaps int
// deltaUpdates and deltaRemoved: see [WithDeltaUpdates].
// deltaUpdates, deltaRemoved and deltaSeen: see [WithDeltaUpdates].
deltaUpdates bool
deltaRemoved []tailcfg.NodeID
deltaSeen []time.Time
// registers: see [TestClient.RegisterAttempts].
registers []testcapture.RegisterAttempt
@@ -482,8 +483,8 @@ func (c *TestClient) UpdateFullNetmap(nm *netmap.NetworkMap) {
// deltaRecorder is the [controlclient.NetmapUpdater] of a [WithDeltaUpdates]
// client. Like tailscaled it takes packet filter and user profile updates
// narrowly, which lets [controlclient.Direct] try the delta path. It
// records removals and declines the mutations, so the full netmap is
// still delivered and the client's peer state stays exact.
// records removals and last seen times and declines the mutations, so the
// full netmap is still delivered and the client's peer state stays exact.
type deltaRecorder struct{ *TestClient }
func (d deltaRecorder) UpdatePacketFilter(views.Slice[tailcfg.FilterRule], []filter.Match) bool {
@@ -499,14 +500,26 @@ func (d deltaRecorder) UpdateNetmapDelta(muts []netmap.NodeMutation) bool {
defer d.mu.Unlock()
for _, m := range muts {
if _, ok := m.(netmap.NodeMutationRemove); ok {
switch m := m.(type) {
case netmap.NodeMutationRemove:
d.deltaRemoved = append(d.deltaRemoved, m.NodeIDBeingMutated())
case netmap.NodeMutationLastSeen:
d.deltaSeen = append(d.deltaSeen, m.LastSeen)
}
}
return false
}
// DeltaLastSeen returns the last seen times that arrived in a response the
// client could apply incrementally; see [WithDeltaUpdates].
func (c *TestClient) DeltaLastSeen() []time.Time {
c.mu.RLock()
defer c.mu.RUnlock()
return slices.Clone(c.deltaSeen)
}
// DeltaRemovedPeers returns the peers whose removal arrived in a response
// the client could apply incrementally; see [WithDeltaUpdates].
func (c *TestClient) DeltaRemovedPeers() []tailcfg.NodeID {
@@ -516,6 +516,15 @@ func peerOnline(p tailcfg.NodeView, missing string) string {
return strconv.FormatBool(p.Online().Get())
}
// peerSeen compares presence only: SaaS sends a coarser time than headscale.
func peerSeen(p tailcfg.NodeView, missing string) string {
if missing != "" {
return missing
}
return strconv.FormatBool(p.LastSeen().Valid())
}
// saasRemoved reports whether the peer, present before the step or once a
// frame named it, was then removed or left out of a full peer list.
func saasRemoved(before *netmap.NetworkMap, frames []testcapture.Frame, id tailcfg.NodeID) string {
+37
View File
@@ -917,6 +917,43 @@ func describeNetmap(c *servertest.TestClient) string {
return out.String()
}
// TestPeerLastSeenPatches takes a node down and up. As on Tailscale SaaS, its
// peer gets a last seen time with the offline patch and none with the online
// one. The recorder runs before the netmap lands, so a netmap settles both.
func TestPeerLastSeenPatches(t *testing.T) {
t.Parallel()
srv := servertest.NewServer(t)
user := srv.CreateUser(t, "seen-user")
watcher := servertest.NewClient(t, srv, "seen-watcher",
servertest.WithUser(user), servertest.WithDeltaUpdates())
node := servertest.NewClient(t, srv, "seen-node", servertest.WithUser(user))
peerOnline := func(want bool) func(*netmap.NetworkMap) bool {
return func(nm *netmap.NetworkMap) bool {
return len(nm.Peers) == 1 && nm.Peers[0].Online().Valid() && nm.Peers[0].Online().Get() == want
}
}
// A new peer can first appear offline, which must not pass for going down.
watcher.WaitForCondition(t, "peer online", 15*time.Second, peerOnline(true))
down := time.Now()
node.Disconnect(t)
watcher.WaitForCondition(t, "peer offline", 30*time.Second, peerOnline(false))
seen := watcher.DeltaLastSeen()
require.Len(t, seen, 1, "offline patch carried no last seen time")
assert.WithinRange(t, seen[0], down, time.Now())
node.Reconnect(t)
watcher.WaitForCondition(t, "peer online", 15*time.Second, peerOnline(true))
assert.Len(t, watcher.DeltaLastSeen(), 1, "online patch carried a last seen time")
}
func TestServerStateCleanup(t *testing.T) {
var srv *servertest.TestServer
+12 -4
View File
@@ -743,9 +743,9 @@ func deliveryChecks(d *deliveryStep) []stepCheck {
return checks
}
// subjectChecks compares how v sees x: online, its DERP home, endpoints,
// name, routes, whether its disco key changed during the step and whether
// its endpoints were cleared on the way.
// subjectChecks compares how v sees x: online, last seen while offline, its
// DERP home, endpoints, name, routes, whether its disco key changed during
// the step and whether its endpoints were cleared on the way.
func (d *deliveryStep) subjectChecks(x, v string) []stepCheck {
r := d.r
saasID := r.c.NodeIDs[x]
@@ -773,7 +773,7 @@ func (d *deliveryStep) subjectChecks(x, v string) []stepCheck {
hsBefore = peerDisco(d.hsBefore[v], id)
}
return []stepCheck{
checks := []stepCheck{
{x + "/online-" + v, peerOnline(saasPeer, saasMissing), func() string { return peerOnline(hsPeer()) }},
{x + "/derp-" + v, peerDERP(saasPeer, saasMissing), func() string { return peerDERP(hsPeer()) }},
{x + "/endpoints-" + v, peerEndpoints(saasPeer, saasMissing), func() string { return peerEndpoints(hsPeer()) }},
@@ -800,6 +800,14 @@ func (d *deliveryStep) subjectChecks(x, v string) []stepCheck {
return hsEndpointsCleared(r.clients[v].History()[d.updates[v]:], id)
}},
}
// A client can keep an old last-seen time through an online patch, so only
// an offline peer's says anything.
if saasMissing == "" && !saasPeer.Online().Get() {
checks = append(checks, stepCheck{x + "/seen-" + v, peerSeen(saasPeer, saasMissing), func() string { return peerSeen(hsPeer()) }})
}
return checks
}
// topologyAddrNames names each SaaS address in c's topology by alias, so a
+1 -1
View File
@@ -1046,7 +1046,7 @@ func (s *State) Disconnect(id types.NodeID, epoch uint64) ([]change.Change, erro
// An ordinary node going offline only needs the lightweight offline
// patch. Subnet routers, relay targets, and via targets change what
// peers compute, so they additionally force a peer recompute.
cs := []change.Change{change.NodeOffline(node.ID())}
cs := []change.Change{change.NodeOffline(node.ID(), node.LastSeen().Get())}
if s.polMan.NodeNeedsPeerRecompute(node) {
cs = append(cs, change.PolicyChange())
}
+7 -4
View File
@@ -530,14 +530,17 @@ func NodeOnline(nodeID types.NodeID) Change {
}
}
// NodeOffline creates a patch response for a node going offline.
func NodeOffline(nodeID types.NodeID) Change {
// NodeOffline creates a patch response for a node going offline. As on
// Tailscale SaaS it says when, so peers need no full map to show it. SaaS
// sends a coarser time; the exact one here is deliberate.
func NodeOffline(nodeID types.NodeID, lastSeen time.Time) Change {
return Change{
Reason: "node offline",
PeerPatches: []*tailcfg.PeerChange{
{
NodeID: nodeID.NodeID(),
Online: new(false),
NodeID: nodeID.NodeID(),
Online: new(false),
LastSeen: &lastSeen,
},
},
}
+9 -5
View File
@@ -451,10 +451,10 @@ func TestDedupePolicyChanges(t *testing.T) {
{
name: "peer patches survive between collapsed policy changes",
changes: []Change{
NodeOnline(1), PolicyChange(), NodeOnline(2), PolicyChange(), NodeOffline(3),
NodeOnline(1), PolicyChange(), NodeOnline(2), PolicyChange(), NodeOffline(3, time.Time{}),
},
want: []Change{
NodeOnline(1), PolicyChange(), NodeOnline(2), NodeOffline(3),
NodeOnline(1), PolicyChange(), NodeOnline(2), NodeOffline(3, time.Time{}),
},
},
{
@@ -514,8 +514,8 @@ func TestCoalescePeerDeltas(t *testing.T) {
},
{
name: "patches for peers sent whole are dropped, others kept in order",
changes: []Change{NodeOnline(4), NodeAdded(2), NodeOffline(4), NodeOnline(2)},
want: []Change{merged([]types.NodeID{2}, patch(NodeOnline(4)), patch(NodeOffline(4)))},
changes: []Change{NodeOnline(4), NodeAdded(2), NodeOffline(4, time.Time{}), NodeOnline(2)},
want: []Change{merged([]types.NodeID{2}, patch(NodeOnline(4)), patch(NodeOffline(4, time.Time{})))},
},
{
name: "runs do not cross other changes",
@@ -940,13 +940,16 @@ func TestNodeOnlineOfflineForSubnetRouter(t *testing.T) {
view := router.View()
require.True(t, view.IsSubnetRouter(), "test node must be a subnet router")
seen := time.Date(2026, 1, 2, 3, 4, 5, 123456789, time.UTC)
tests := []struct {
name string
got Change
wantOnline bool
wantSeen *time.Time
}{
{name: "online", got: NodeOnline(view.ID()), wantOnline: true},
{name: "offline", got: NodeOffline(view.ID()), wantOnline: false},
{name: "offline", got: NodeOffline(view.ID(), seen), wantOnline: false, wantSeen: &seen},
}
for _, tt := range tests {
@@ -965,6 +968,7 @@ func TestNodeOnlineOfflineForSubnetRouter(t *testing.T) {
require.NotNil(t, patch.Online)
assert.Equal(t, tt.wantOnline, *patch.Online)
assert.Equal(t, tt.wantSeen, patch.LastSeen)
})
}
}
+2 -3
View File
@@ -1487,9 +1487,8 @@ func (nv NodeView) TailNode(
Expired: nv.IsExpired(),
}
// Set LastSeen only for offline nodes to avoid confusing Tailscale clients
// during rapid reconnection cycles. Online nodes should not have LastSeen set
// as this can make clients interpret them as "not online" despite Online=true.
// Only an offline entry has LastSeen, as on Tailscale SaaS. An online peer
// can still show one on a client, which keeps it through an online patch.
if nv.LastSeen().Valid() && nv.IsOnline().Valid() && !nv.IsOnline().Get() {
lastSeen := nv.LastSeen().Get()
tNode.LastSeen = &lastSeen