tailcfg, control/controlclient: (mapver 16) add Node.Online, MapResponse.OnlineChange

And fix PeerSeenChange bug where it was ignored unless there were
other peer changes.

Updates tailscale/corp#1574

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

control/controlclient/direct.go

@@ -1097,8 +1097,13 @@ func envBool(k string) bool {
 	return v
 }
 
-// undeltaPeers updates mapRes.Peers to be complete based on the provided previous peer list
+var clockNow = time.Now
-// and the PeersRemoved and PeersChanged fields in mapRes.
+
+// undeltaPeers updates mapRes.Peers to be complete based on the
+// provided previous peer list and the PeersRemoved and PeersChanged
+// fields in mapRes, as well as the PeerSeenChange and OnlineChange
+// maps.
+//
 // It then also nils out the delta fields.
 func undeltaPeers(mapRes *tailcfg.MapResponse, prev []*tailcfg.Node) {
 	if len(mapRes.Peers) > 0 {
@@ -1119,12 +1124,6 @@ func undeltaPeers(mapRes *tailcfg.MapResponse, prev []*tailcfg.Node) {
 	}
 	changed := mapRes.PeersChanged
 
-	if len(removed) == 0 && len(changed) == 0 {
-		// No changes fast path.
-		mapRes.Peers = prev
-		return
-	}
-
 	if !nodesSorted(changed) {
 		log.Printf("netmap: undeltaPeers: MapResponse.PeersChanged not sorted; sorting")
 		sortNodes(changed)
@@ -1135,40 +1134,43 @@ func undeltaPeers(mapRes *tailcfg.MapResponse, prev []*tailcfg.Node) {
 		sortNodes(prev)
 	}
 
-	newFull := make([]*tailcfg.Node, 0, len(prev)-len(removed))
+	newFull := prev
-	for len(prev) > 0 && len(changed) > 0 {
+	if len(removed) > 0 || len(changed) > 0 {
-		pID := prev[0].ID
+		newFull = make([]*tailcfg.Node, 0, len(prev)-len(removed))
-		cID := changed[0].ID
+		for len(prev) > 0 && len(changed) > 0 {
-		if removed[pID] {
+			pID := prev[0].ID
-			prev = prev[1:]
+			cID := changed[0].ID
-			continue
+			if removed[pID] {
+				prev = prev[1:]
+				continue
+			}
+			switch {
+			case pID < cID:
+				newFull = append(newFull, prev[0])
+				prev = prev[1:]
+			case pID == cID:
+				newFull = append(newFull, changed[0])
+				prev, changed = prev[1:], changed[1:]
+			case cID < pID:
+				newFull = append(newFull, changed[0])
+				changed = changed[1:]
+			}
 		}
-		switch {
+		newFull = append(newFull, changed...)
-		case pID < cID:
+		for _, n := range prev {
-			newFull = append(newFull, prev[0])
+			if !removed[n.ID] {
-			prev = prev[1:]
+				newFull = append(newFull, n)
-		case pID == cID:
+			}
-			newFull = append(newFull, changed[0])
-			prev, changed = prev[1:], changed[1:]
-		case cID < pID:
-			newFull = append(newFull, changed[0])
-			changed = changed[1:]
 		}
+		sortNodes(newFull)
 	}
-	newFull = append(newFull, changed...)
-	for _, n := range prev {
-		if !removed[n.ID] {
-			newFull = append(newFull, n)
-		}
-	}
-	sortNodes(newFull)
 
-	if mapRes.PeerSeenChange != nil {
+	if len(mapRes.PeerSeenChange) != 0 || len(mapRes.OnlineChange) != 0 {
 		peerByID := make(map[tailcfg.NodeID]*tailcfg.Node, len(newFull))
 		for _, n := range newFull {
 			peerByID[n.ID] = n
 		}
-		now := time.Now()
+		now := clockNow()
 		for nodeID, seen := range mapRes.PeerSeenChange {
 			if n, ok := peerByID[nodeID]; ok {
 				if seen {
@@ -1178,6 +1180,12 @@ func undeltaPeers(mapRes *tailcfg.MapResponse, prev []*tailcfg.Node) {
 				}
 			}
 		}
+		for nodeID, online := range mapRes.OnlineChange {
+			if n, ok := peerByID[nodeID]; ok {
+				online := online
+				n.Online = &online
+			}
+		}
 	}
 
 	mapRes.Peers = newFull

control/controlclient/direct_test.go

@@ -10,6 +10,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"inet.af/netaddr"
 	"tailscale.com/tailcfg"
@@ -17,15 +18,36 @@ import (
 )
 
 func TestUndeltaPeers(t *testing.T) {
-	n := func(id tailcfg.NodeID, name string) *tailcfg.Node {
+	defer func(old func() time.Time) { clockNow = old }(clockNow)
-		return &tailcfg.Node{ID: id, Name: name}
+
+	var curTime time.Time
+	clockNow = func() time.Time {
+		return curTime
+	}
+	online := func(v bool) func(*tailcfg.Node) {
+		return func(n *tailcfg.Node) {
+			n.Online = &v
+		}
+	}
+	seenAt := func(t time.Time) func(*tailcfg.Node) {
+		return func(n *tailcfg.Node) {
+			n.LastSeen = &t
+		}
+	}
+	n := func(id tailcfg.NodeID, name string, mod ...func(*tailcfg.Node)) *tailcfg.Node {
+		n := &tailcfg.Node{ID: id, Name: name}
+		for _, f := range mod {
+			f(n)
+		}
+		return n
 	}
 	peers := func(nv ...*tailcfg.Node) []*tailcfg.Node { return nv }
 	tests := []struct {
-		name   string
+		name    string
-		mapRes *tailcfg.MapResponse
+		mapRes  *tailcfg.MapResponse
-		prev   []*tailcfg.Node
+		curTime time.Time
-		want   []*tailcfg.Node
+		prev    []*tailcfg.Node
+		want    []*tailcfg.Node
 	}{
 		{
 			name: "full_peers",
@@ -73,9 +95,54 @@ func TestUndeltaPeers(t *testing.T) {
 			mapRes: &tailcfg.MapResponse{},
 			want:   peers(n(1, "foo"), n(2, "bar")),
 		},
+		{
+			name: "online_change",
+			prev: peers(n(1, "foo"), n(2, "bar")),
+			mapRes: &tailcfg.MapResponse{
+				OnlineChange: map[tailcfg.NodeID]bool{
+					1: true,
+				},
+			},
+			want: peers(
+				n(1, "foo", online(true)),
+				n(2, "bar"),
+			),
+		},
+		{
+			name: "online_change_offline",
+			prev: peers(n(1, "foo"), n(2, "bar")),
+			mapRes: &tailcfg.MapResponse{
+				OnlineChange: map[tailcfg.NodeID]bool{
+					1: false,
+					2: true,
+				},
+			},
+			want: peers(
+				n(1, "foo", online(false)),
+				n(2, "bar", online(true)),
+			),
+		},
+		{
+			name:    "peer_seen_at",
+			prev:    peers(n(1, "foo", seenAt(time.Unix(111, 0))), n(2, "bar")),
+			curTime: time.Unix(123, 0),
+			mapRes: &tailcfg.MapResponse{
+				PeerSeenChange: map[tailcfg.NodeID]bool{
+					1: false,
+					2: true,
+				},
+			},
+			want: peers(
+				n(1, "foo"),
+				n(2, "bar", seenAt(time.Unix(123, 0))),
+			),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			if !tt.curTime.IsZero() {
+				curTime = tt.curTime
+			}
 			undeltaPeers(tt.mapRes, tt.prev)
 			if !reflect.DeepEqual(tt.mapRes.Peers, tt.want) {
 				t.Errorf("wrong results\n got: %s\nwant: %s", formatNodes(tt.mapRes.Peers), formatNodes(tt.want))
@@ -90,7 +157,14 @@ func formatNodes(nodes []*tailcfg.Node) string {
 		if i > 0 {
 			sb.WriteString(", ")
 		}
-		fmt.Fprintf(&sb, "(%d, %q)", n.ID, n.Name)
+		var extra string
+		if n.Online != nil {
+			extra += fmt.Sprintf(", online=%v", *n.Online)
+		}
+		if n.LastSeen != nil {
+			extra += fmt.Sprintf(", lastSeen=%v", n.LastSeen.Unix())
+		}
+		fmt.Fprintf(&sb, "(%d, %q%s)", n.ID, n.Name, extra)
 	}
 	return sb.String()
 }

tailcfg/tailcfg.go

@@ -38,7 +38,8 @@ import (
 //    13: 2021-03-19: client understands FilterRule.IPProto
 //    14: 2021-04-07: client understands DNSConfig.Routes and DNSConfig.Resolvers
 //    15: 2021-04-12: client treats nil MapResponse.DNSConfig as meaning unchanged
-const CurrentMapRequestVersion = 15
+//    16: 2021-04-15: client understands Node.Online, MapResponse.OnlineChange
+const CurrentMapRequestVersion = 16
 
 type StableID string
 
@@ -156,7 +157,17 @@ type Node struct {
 	DERP       string             `json:",omitempty"` // DERP-in-IP:port ("127.3.3.40:N") endpoint
 	Hostinfo   Hostinfo
 	Created    time.Time
-	LastSeen   *time.Time `json:",omitempty"`
+
+	// LastSeen is when the node was last online. It is not
+	// updated when Online is true. It is nil if the current
+	// node doesn't have permission to know, or the node
+	// has never been online.
+	LastSeen *time.Time `json:",omitempty"`
+
+	// Online is whether the node is currently connected to the
+	// coordination server.  A value of nil means unknown, or the
+	// current node doesn't have permission to know.
+	Online *bool `json:",omitempty"`
 
 	KeepAlive bool `json:",omitempty"` // open and keep open a connection to this peer
 
@@ -907,6 +918,9 @@ type MapResponse struct {
 	// the LastSeen time is now. Absent means unchanged.
 	PeerSeenChange map[NodeID]bool `json:",omitempty"`
 
+	// OnlineChange changes the value of a Peer Node.Online value.
+	OnlineChange map[NodeID]bool `json:",omitempty"`
+
 	// DNS is the same as DNSConfig.Nameservers.
 	// Only populated if MapRequest.Version < 9.
 	DNS []netaddr.IP `json:",omitempty"`
@@ -1048,6 +1062,7 @@ func (n *Node) Equal(n2 *Node) bool {
 		n.KeyExpiry.Equal(n2.KeyExpiry) &&
 		n.Machine == n2.Machine &&
 		n.DiscoKey == n2.DiscoKey &&
+		eqBoolPtr(n.Online, n2.Online) &&
 		eqCIDRs(n.Addresses, n2.Addresses) &&
 		eqCIDRs(n.AllowedIPs, n2.AllowedIPs) &&
 		eqStrings(n.Endpoints, n2.Endpoints) &&
@@ -1062,6 +1077,17 @@ func (n *Node) Equal(n2 *Node) bool {
 		n.ComputedNameWithHost == n2.ComputedNameWithHost
 }
 
+func eqBoolPtr(a, b *bool) bool {
+	if a == b { // covers nil
+		return true
+	}
+	if a == nil || b == nil {
+		return false
+	}
+	return *a == *b
+
+}
+
 func eqStrings(a, b []string) bool {
 	if len(a) != len(b) || ((a == nil) != (b == nil)) {
 		return false

tailcfg/tailcfg_clone.go

@@ -53,6 +53,10 @@ func (src *Node) Clone() *Node {
 		dst.LastSeen = new(time.Time)
 		*dst.LastSeen = *src.LastSeen
 	}
+	if dst.Online != nil {
+		dst.Online = new(bool)
+		*dst.Online = *src.Online
+	}
 	dst.Capabilities = append(src.Capabilities[:0:0], src.Capabilities...)
 	return dst
 }
@@ -76,6 +80,7 @@ var _NodeNeedsRegeneration = Node(struct {
 	Hostinfo                Hostinfo
 	Created                 time.Time
 	LastSeen                *time.Time
+	Online                  *bool
 	KeepAlive               bool
 	MachineAuthorized       bool
 	Capabilities            []string

tailcfg/tailcfg_test.go

@@ -193,7 +193,7 @@ func TestNodeEqual(t *testing.T) {
 		"ID", "StableID", "Name", "User", "Sharer",
 		"Key", "KeyExpiry", "Machine", "DiscoKey",
 		"Addresses", "AllowedIPs", "Endpoints", "DERP", "Hostinfo",
-		"Created", "LastSeen", "KeepAlive", "MachineAuthorized",
+		"Created", "LastSeen", "Online", "KeepAlive", "MachineAuthorized",
 		"Capabilities",
 		"ComputedName", "computedHostIfDifferent", "ComputedNameWithHost",
 	}

version/version.go

@@ -10,7 +10,7 @@ package version
 // Long is a full version number for this build, of the form
 // "x.y.z-commithash", or "date.yyyymmdd" if no actual version was
 // provided.
-const Long = "date.20210316"
+const Long = "date.20210415"
 
 // Short is a short version number for this build, of the form
 // "x.y.z", or "date.yyyymmdd" if no actual version was provided.