From 979569c3c2542f1b1bf1503138f4f2b488149489 Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Sat, 2 Dec 2023 18:19:02 +0100
Subject: [PATCH] fix save node when routes/services

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
---
 hscontrol/db/node.go |  5 ----
 hscontrol/poll.go    | 55 +++++++++++++++++++++++++++++++++++---------
 integration/utils.go |  4 ++--
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/hscontrol/db/node.go b/hscontrol/db/node.go
index ca390f8a7e..e4c454d80b 100644
--- a/hscontrol/db/node.go
+++ b/hscontrol/db/node.go
@@ -61,11 +61,6 @@ func (hsdb *HSDatabase) listPeers(node *types.Node) (types.Nodes, error) {
 
 	sort.Slice(nodes, func(i, j int) bool { return nodes[i].ID < nodes[j].ID })
 
-	log.Trace().
-		Caller().
-		Str("node", node.Hostname).
-		Msgf("Found peers: %s", nodes.String())
-
 	return nodes, nil
 }
 
diff --git a/hscontrol/poll.go b/hscontrol/poll.go
index 79a584859c..916a06dd35 100644
--- a/hscontrol/poll.go
+++ b/hscontrol/poll.go
@@ -80,7 +80,7 @@ func (h *Headscale) handlePoll(
 			Str("node_key", node.NodeKey.ShortString()).
 			Str("node", node.Hostname).
 			Int("cap_ver", int(mapRequest.Version)).
-			Msg("Received endpoint update")
+			Msg("Received update")
 
 		change := node.PeerChangeFromMapRequest(mapRequest)
 
@@ -89,6 +89,10 @@ func (h *Headscale) handlePoll(
 
 		node.ApplyPeerChange(&change)
 
+		hostInfoChange := node.Hostinfo.Equal(mapRequest.Hostinfo)
+
+		logTracePeerChange(node.Hostname, hostInfoChange, &change)
+
 		// Check if the Hostinfo of the node has changed.
 		// If it has changed, check if there has been a change tod
 		// the routable IPs of the host and update update them in
@@ -97,7 +101,7 @@ func (h *Headscale) handlePoll(
 		// the route change.
 		// If the hostinfo has changed, but not the routes, just update
 		// hostinfo and let the function continue.
-		if !node.Hostinfo.Equal(mapRequest.Hostinfo) {
+		if !hostInfoChange {
 			oldRoutes := node.Hostinfo.RoutableIPs
 			newRoutes := mapRequest.Hostinfo.RoutableIPs
 
@@ -121,13 +125,6 @@ func (h *Headscale) handlePoll(
 					return
 				}
 
-				if err := h.db.NodeSave(node); err != nil {
-					logErr(err, "Failed to persist/update node in the database")
-					http.Error(writer, "", http.StatusInternalServerError)
-
-					return
-				}
-
 				sendUpdate = true
 			}
 
@@ -142,6 +139,13 @@ func (h *Headscale) handlePoll(
 			}
 
 			if sendUpdate {
+				if err := h.db.NodeSave(node); err != nil {
+					logErr(err, "Failed to persist/update node in the database")
+					http.Error(writer, "", http.StatusInternalServerError)
+
+					return
+				}
+
 				stateUpdate := types.StateUpdate{
 					Type:        types.StatePeerChanged,
 					ChangeNodes: types.Nodes{node},
@@ -269,7 +273,6 @@ func (h *Headscale) handlePoll(
 
 	logInfo("Sending initial map")
 
-	log.Trace().Msgf("NODE FULLMAP BEGIN %s", node.Hostname)
 	mapResp, err := mapp.FullMapResponse(mapRequest, node, h.ACLPolicy)
 	if err != nil {
 		logErr(err, "Failed to create MapResponse")
@@ -277,7 +280,6 @@ func (h *Headscale) handlePoll(
 
 		return
 	}
-	log.Trace().Msgf("NODE FULLMAP END %s", node.Hostname)
 
 	// Send the client an update to make sure we send an initial mapresponse
 	_, err = writer.Write(mapResp)
@@ -532,3 +534,34 @@ func (h *Headscale) handleLiteRequest(
 		logErr(err, "Failed to write response")
 	}
 }
+
+func logTracePeerChange(hostname string, hostinfoChange bool, change *tailcfg.PeerChange) {
+	trace := log.Trace().Str("node_id", change.NodeID.String()).Str("hostname", hostname)
+
+	if change.Key != nil {
+		trace = trace.Str("node_key", change.Key.ShortString())
+	}
+
+	if change.DiscoKey != nil {
+		trace = trace.Str("disco_key", change.DiscoKey.ShortString())
+	}
+
+	if change.Online != nil {
+		trace = trace.Bool("online", *change.Online)
+	}
+
+	if change.Endpoints != nil {
+		eps := make([]string, len(change.Endpoints))
+		for idx, ep := range change.Endpoints {
+			eps[idx] = ep.String()
+		}
+
+		trace = trace.Strs("endpoints", eps)
+	}
+
+	if hostinfoChange {
+		trace = trace.Bool("hostinfo_changed", hostinfoChange)
+	}
+
+	trace.Time("last_seen", *change.LastSeen).Msg("PeerChange received")
+}
diff --git a/integration/utils.go b/integration/utils.go
index cccf63aeaa..e17e18a274 100644
--- a/integration/utils.go
+++ b/integration/utils.go
@@ -75,13 +75,13 @@ func assertContains(t *testing.T, str, subStr string) {
 	}
 }
 
-func pingAllHelper(t *testing.T, clients []TailscaleClient, addrs []string) int {
+func pingAllHelper(t *testing.T, clients []TailscaleClient, addrs []string, opts ...tsic.PingOption) int {
 	t.Helper()
 	success := 0
 
 	for _, client := range clients {
 		for _, addr := range addrs {
-			err := client.Ping(addr)
+			err := client.Ping(addr, opts...)
 			if err != nil {
 				t.Fatalf("failed to ping %s from %s: %s", addr, client.Hostname(), err)
 			} else {