control/controlclient: move lastPrintMap field from Direct to mapSession

It was a really a mutable field owned by mapSession that we didn't move
in earlier commits.

Once moved, it's then possible to de-func-ify the code and turn it into
a regular method rather than an installed optional hook.

Noticed while working to move map session lifetimes out of
Direct.sendMapRequest's single-HTTP-connection scope.

Updates #7175
Updates #cleanup

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

control/controlclient/auto.go

@@ -481,7 +481,7 @@ func (mrs mapRoutineState) UpdateNetmapDelta(muts []netmap.NodeMutation) bool {
 // control server, and keeping the netmap up to date.
 func (c *Auto) mapRoutine() {
 	defer close(c.mapDone)
-	mrs := &mapRoutineState{
+	mrs := mapRoutineState{
 		c:  c,
 		bo: backoff.NewBackoff("mapRoutine", c.logf, 30*time.Second),
 	}

control/controlclient/direct.go

@@ -67,7 +67,6 @@ type Direct struct {
 	controlKnobs          *controlknobs.Knobs // always non-nil
 	serverURL             string              // URL of the tailcontrol server
 	clock                 tstime.Clock
-	lastPrintMap          time.Time
 	logf                  logger.Logf
 	netMon                *netmon.Monitor // or nil
 	discoPubKey           key.DiscoPublic
@@ -969,8 +968,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap
 		return nil
 	}
 
-	var mapResIdx int // 0 for first message, then 1+ for deltas
-
 	sess := newMapSession(persist.PrivateNodeKey(), nu, c.controlKnobs)
 	defer sess.Close()
 	sess.cancel = cancel
@@ -979,17 +976,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap
 	sess.altClock = c.clock
 	sess.machinePubKey = machinePubKey
 	sess.onDebug = c.handleDebugMessage
-	sess.onConciseNetMapSummary = func(summary string) {
-		// Occasionally print the netmap header.
-		// This is handy for debugging, and our logs processing
-		// pipeline depends on it. (TODO: Remove this dependency.)
-		now := c.clock.Now()
-		if now.Sub(c.lastPrintMap) < 5*time.Minute {
-			return
-		}
-		c.lastPrintMap = now
-		c.logf("[v1] new network map[%d]:\n%s", mapResIdx, summary)
-	}
 	sess.onSelfNodeChanged = func(nm *netmap.NetworkMap) {
 		c.mu.Lock()
 		defer c.mu.Unlock()
@@ -1019,7 +1005,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap
 	// the same format before just closing the connection.
 	// We can use this same read loop either way.
 	var msg []byte
-	for ; mapResIdx == 0 || isStreaming; mapResIdx++ {
+	for mapResIdx := 0; mapResIdx == 0 || isStreaming; mapResIdx++ {
 		vlogf("netmap: starting size read after %v (poll %v)", time.Since(t0).Round(time.Millisecond), mapResIdx)
 		var siz [4]byte
 		if _, err := io.ReadFull(res.Body, siz[:]); err != nil {

control/controlclient/map.go

@@ -64,15 +64,12 @@ type mapSession struct {
 	// to request that the long poll activity watchdog timeout be reset.
 	onDebug func(_ context.Context, _ *tailcfg.Debug, watchdogReset chan<- struct{}) error
 
-	// onConciseNetMapSummary, if non-nil, is called with the Netmap.VeryConcise summary
-	// whenever a map response is received.
-	onConciseNetMapSummary func(string)
-
 	// onSelfNodeChanged is called before the NetmapUpdater if the self node was
 	// changed.
 	onSelfNodeChanged func(*netmap.NetworkMap)
 
 	// Fields storing state over the course of multiple MapResponses.
+	lastPrintMap           time.Time
 	lastNode               tailcfg.NodeView
 	peers                  map[tailcfg.NodeID]*tailcfg.NodeView // pointer to view (oddly). same pointers as sortedPeers.
 	sortedPeers            []*tailcfg.NodeView                  // same pointers as peers, but sorted by Node.ID
@@ -108,17 +105,30 @@ func newMapSession(privateNodeKey key.NodePrivate, nu NetmapUpdater, controlKnob
 		watchdogReset:   make(chan struct{}),
 
 		// Non-nil no-op defaults, to be optionally overridden by the caller.
-		logf:                   logger.Discard,
+		logf:              logger.Discard,
-		vlogf:                  logger.Discard,
+		vlogf:             logger.Discard,
-		cancel:                 func() {},
+		cancel:            func() {},
-		onDebug:                func(context.Context, *tailcfg.Debug, chan<- struct{}) error { return nil },
+		onDebug:           func(context.Context, *tailcfg.Debug, chan<- struct{}) error { return nil },
-		onConciseNetMapSummary: func(string) {},
+		onSelfNodeChanged: func(*netmap.NetworkMap) {},
-		onSelfNodeChanged:      func(*netmap.NetworkMap) {},
 	}
 	ms.sessionAliveCtx, ms.sessionAliveCtxClose = context.WithCancel(context.Background())
 	return ms
 }
 
+// occasionallyPrintSummary logs summary at most once very 5 minutes. The
+// summary is the Netmap.VeryConcise result from the last received map response.
+func (ms *mapSession) occasionallyPrintSummary(summary string) {
+	// Occasionally print the netmap header.
+	// This is handy for debugging, and our logs processing
+	// pipeline depends on it. (TODO: Remove this dependency.)
+	now := ms.clock().Now()
+	if now.Sub(ms.lastPrintMap) < 5*time.Minute {
+		return
+	}
+	ms.lastPrintMap = now
+	ms.logf("[v1] new network map (periodic):\n%s", summary)
+}
+
 func (ms *mapSession) clock() tstime.Clock {
 	return cmpx.Or[tstime.Clock](ms.altClock, tstime.StdClock{})
 }
@@ -200,7 +210,7 @@ func (ms *mapSession) HandleNonKeepAliveMapResponse(ctx context.Context, resp *t
 	ms.updateStateFromResponse(resp)
 
 	if ms.tryHandleIncrementally(resp) {
-		ms.onConciseNetMapSummary(ms.lastNetmapSummary) // every 5s log
+		ms.occasionallyPrintSummary(ms.lastNetmapSummary)
 		return nil
 	}
 
@@ -210,7 +220,7 @@ func (ms *mapSession) HandleNonKeepAliveMapResponse(ctx context.Context, resp *t
 
 	nm := ms.netmap()
 	ms.lastNetmapSummary = nm.VeryConcise()
-	ms.onConciseNetMapSummary(ms.lastNetmapSummary)
+	ms.occasionallyPrintSummary(ms.lastNetmapSummary)
 
 	// If the self node changed, we might need to update persist.
 	if resp.Node != nil {