From 0f3c767d1769f22236c50107b5cf371d5748cf9a Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Thu, 15 Oct 2026 14:58:56 +0000 Subject: [PATCH] control/controlclient,ipn/ipnlocal,tsconst: go to NeedsLogin on node removal When a node is deleted, control answers its map requests with a 404 and the client keeps retrying in its current state, so watchers like containerboot can't tell a deleted node from a lost connection. This wraps that error in ErrNodeRemoved, and LocalBackend then moves to NeedsLogin and sets a node-removed health warning, both cleared on the next netmap or profile switch. It repurposes #18701 as a stopgap until a broader fix, such as structured errors from control, exists. Updates #18700 Signed-off-by: chaosinthecrd --- control/controlclient/direct.go | 11 ++++++- ipn/ipnlocal/local.go | 54 ++++++++++++++++++++++++++++----- tsconst/health.go | 1 + 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index abe814109..00941c5ed 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -588,6 +588,11 @@ var macOSScreenTime = health.Register(&health.Warnable{ ImpactsConnectivity: true, }) +// ErrNodeRemoved is wrapped by the error returned from a map request that +// control responded to with a 404, which it only does once the node, or the +// user or login that owns it, has been deleted. +var ErrNodeRemoved = errors.New("node removed from tailnet") + type rateLimitError struct { msg string retryAfter time.Duration @@ -1212,8 +1217,12 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap } msg, _ := io.ReadAll(res.Body) res.Body.Close() - return fmt.Errorf("initial fetch failed %d: %.200s", + err := fmt.Errorf("initial fetch failed %d: %.200s", res.StatusCode, strings.TrimSpace(string(msg))) + if res.StatusCode == http.StatusNotFound { + err = fmt.Errorf("%w: %w", ErrNodeRemoved, err) + } + return err } defer res.Body.Close() diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 816c02327..cfadce2ce 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -71,6 +71,7 @@ import ( "tailscale.com/syncs" "tailscale.com/tailcfg" "tailscale.com/tailcfg/nodecap" + "tailscale.com/tsconst" "tailscale.com/tsd" "tailscale.com/tstime" "tailscale.com/types/appctype" @@ -349,6 +350,7 @@ type LocalBackend struct { endpoints []tailcfg.Endpoint blocked bool keyExpired bool // TODO(nickkhyl): move to nodeBackend + nodeRemoved bool // control said this node was deleted; cleared on the next netmap or profile switch authURL string // non-empty if not Running; TODO(nickkhyl): move to nodeBackend authURLTime time.Time // when the authURL was received from the control server; TODO(nickkhyl): move to nodeBackend authActor ipnauth.Actor // an actor who called [LocalBackend.StartLoginInteractive] last, or nil; TODO(nickkhyl): move to nodeBackend @@ -1853,6 +1855,30 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control b.setControlClientStatusLocked(c, st) } +// nodeRemovedWarnable is set while control says this node has been deleted. +var nodeRemovedWarnable = health.Register(&health.Warnable{ + Code: tsconst.HealthWarnableNodeRemoved, + Title: "Device removed", + Severity: health.SeverityHigh, + Text: health.StaticMessage("This device has been removed from the tailnet. Log in again to reconnect."), + DependsOn: []*health.Warnable{health.IPNStateWarnable}, + ImpactsConnectivity: true, +}) + +// setNodeRemovedLocked sets b.nodeRemoved and the matching health warning. +// b.mu must be held. +func (b *LocalBackend) setNodeRemovedLocked(removed bool) { + if removed == b.nodeRemoved { + return + } + b.nodeRemoved = removed + if removed { + b.health.SetUnhealthy(nodeRemovedWarnable, nil) + } else { + b.health.SetHealthy(nodeRemovedWarnable) + } +} + // setControlClientStatusLocked is the locked version of SetControlClientStatus. // // b.mu must be held. @@ -1871,6 +1897,10 @@ func (b *LocalBackend) setControlClientStatusLocked(c controlclient.Client, st c s := vizerr.Error() b.sendLocked(ipn.Notify{ErrMessage: &s}) } + if errors.Is(st.Err, controlclient.ErrNodeRemoved) && !b.nodeRemoved { + b.setNodeRemovedLocked(true) + b.stateMachineLocked() + } return } @@ -1915,6 +1945,7 @@ func (b *LocalBackend) setControlClientStatusLocked(c controlclient.Client, st c keyExpiryExtended = true } b.keyExpired = isExpired + b.setNodeRemovedLocked(false) } if keyExpiryExtended && wasBlocked { @@ -6929,13 +6960,14 @@ func (b *LocalBackend) nextStateLocked() ipn.State { } var ( - cc = b.cc - cn = b.currentNode() - netMap = cn.NetMap() - state = b.state - blocked = b.blocked - st = b.engineStatus - keyExpired = b.keyExpired + cc = b.cc + cn = b.currentNode() + netMap = cn.NetMap() + state = b.state + blocked = b.blocked + st = b.engineStatus + keyExpired = b.keyExpired + nodeRemoved = b.nodeRemoved wantRunning = false loggedOut = false @@ -6948,6 +6980,13 @@ func (b *LocalBackend) nextStateLocked() ipn.State { switch { case !wantRunning && !loggedOut && !blocked && b.hasNodeKeyLocked(): return ipn.Stopped + case wantRunning && nodeRemoved: + // Control said this node was deleted, need to relogin. Checked + // before netMap so a node deleted after registering but before + // its first netmap, which then never gets one, goes to NeedsLogin + // rather than staying in NoState. + // See https://github.com/tailscale/tailscale/issues/18922#issuecomment-6198375286 + return ipn.NeedsLogin case netMap == nil: if (cc != nil && cc.AuthCantContinue()) || loggedOut { // Auth was interrupted or waiting for URL visit, @@ -8491,6 +8530,7 @@ func (b *LocalBackend) resetForProfileChangeLocked() error { b.serveConfig = ipn.ServeConfigView{} b.lastSuggestedExitNode = "" b.keyExpired = false + b.setNodeRemovedLocked(false) b.resetAlwaysOnOverrideLocked() b.extHost.NotifyProfileChange(b.pm.CurrentProfile(), b.pm.CurrentPrefs(), false) b.setAtomicValuesFromPrefsLocked(b.pm.CurrentPrefs()) diff --git a/tsconst/health.go b/tsconst/health.go index 2dede3031..952dd323f 100644 --- a/tsconst/health.go +++ b/tsconst/health.go @@ -11,6 +11,7 @@ const ( HealthWarnableWantRunningFalse = "wantrunning-false" HealthWarnableLocalLogConfigError = "local-log-config-error" HealthWarnableLoginState = "login-state" + HealthWarnableNodeRemoved = "node-removed" HealthWarnableNotInMapPoll = "not-in-map-poll" HealthWarnableNoDERPHome = "no-derp-home" HealthWarnableNoDERPConnection = "no-derp-connection"