From 96712e10a77302427514255411dbfdf2c208d2f9 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 1 May 2024 13:54:56 -0700 Subject: [PATCH] health, ipn/ipnlocal: move more health warning code into health.Tracker In prep for making health warnings rich objects with metadata rather than a bunch of strings, start moving it all into the same place. We'll still ultimately need the stringified form for the CLI and LocalAPI for compatibility but we'll next convert all these warnings into Warnables that have severity levels and such, and legacy stringification will just be something each Warnable thing can do. Updates #4136 Change-Id: I83e189435daae3664135ed53c98627c66e9e53da Signed-off-by: Brad Fitzpatrick --- health/health.go | 106 +++++++++++++++++++++++++++++++++++---- ipn/ipnlocal/local.go | 26 +++------- ipn/ipnlocal/profiles.go | 17 ++++++- 3 files changed, 116 insertions(+), 33 deletions(-) diff --git a/health/health.go b/health/health.go index 2b1d3f42e..08badf04e 100644 --- a/health/health.go +++ b/health/health.go @@ -23,6 +23,7 @@ import ( "tailscale.com/util/mak" "tailscale.com/util/multierr" "tailscale.com/util/set" + "tailscale.com/version" ) var ( @@ -72,6 +73,9 @@ type Tracker struct { watchers set.HandleSet[func(Subsystem, error)] // opt func to run if error state changes timer *time.Timer + latestVersion *tailcfg.ClientVersion // or nil + checkForUpdates bool + inMapPoll bool inMapPollSince time.Time lastMapPollEndedAt time.Time @@ -543,7 +547,37 @@ func (t *Tracker) SetAuthRoutineInError(err error) { } t.mu.Lock() defer t.mu.Unlock() + if err == nil && t.lastLoginErr == nil { + return + } t.lastLoginErr = err + t.selfCheckLocked() +} + +// SetLatestVersion records the latest version of the Tailscale client. +// v can be nil if unknown. +func (t *Tracker) SetLatestVersion(v *tailcfg.ClientVersion) { + if t.nil() { + return + } + t.mu.Lock() + defer t.mu.Unlock() + t.latestVersion = v + t.selfCheckLocked() +} + +// SetCheckForUpdates sets whether the client wants to check for updates. +func (t *Tracker) SetCheckForUpdates(v bool) { + if t.nil() { + return + } + t.mu.Lock() + defer t.mu.Unlock() + if t.checkForUpdates == v { + return + } + t.checkForUpdates = v + t.selfCheckLocked() } func (t *Tracker) timerSelfCheck() { @@ -567,6 +601,23 @@ func (t *Tracker) selfCheckLocked() { t.setLocked(SysOverall, t.overallErrorLocked()) } +// AppendWarnings appends all current health warnings to dst and returns the +// result. +func (t *Tracker) AppendWarnings(dst []string) []string { + err := t.OverallError() + if err == nil { + return dst + } + if me, ok := err.(multierr.Error); ok { + for _, err := range me.Errors() { + dst = append(dst, err.Error()) + } + } else { + dst = append(dst, err.Error()) + } + return dst +} + // OverallError returns a summary of the health state. // // If there are multiple problems, the error will be of type @@ -609,42 +660,76 @@ var errNetworkDown = errors.New("network down") var errNotInMapPoll = errors.New("not in map poll") var errNoDERPHome = errors.New("no DERP home") var errNoUDP4Bind = errors.New("no udp4 bind") +var errUnstable = errors.New("This is an unstable (development) version of Tailscale; frequent updates and bugs are likely") func (t *Tracker) overallErrorLocked() error { + var errs []error + add := func(err error) { + if err != nil { + errs = append(errs, err) + } + } + merged := func() error { + return multierr.New(errs...) + } + + if t.checkForUpdates { + if cv := t.latestVersion; cv != nil && !cv.RunningLatest && cv.LatestVersion != "" { + if cv.UrgentSecurityUpdate { + add(fmt.Errorf("Security update available: %v -> %v, run `tailscale update` or `tailscale set --auto-update` to update", version.Short(), cv.LatestVersion)) + } else { + add(fmt.Errorf("Update available: %v -> %v, run `tailscale update` or `tailscale set --auto-update` to update", version.Short(), cv.LatestVersion)) + } + } + } + if version.IsUnstableBuild() { + add(errUnstable) + } + if v, ok := t.anyInterfaceUp.Get(); ok && !v { - return errNetworkDown + add(errNetworkDown) + return merged() } if t.localLogConfigErr != nil { - return t.localLogConfigErr + add(t.localLogConfigErr) + return merged() } if !t.ipnWantRunning { - return fmt.Errorf("state=%v, wantRunning=%v", t.ipnState, t.ipnWantRunning) + add(fmt.Errorf("state=%v, wantRunning=%v", t.ipnState, t.ipnWantRunning)) + return merged() } if t.lastLoginErr != nil { - return fmt.Errorf("not logged in, last login error=%v", t.lastLoginErr) + add(fmt.Errorf("not logged in, last login error=%v", t.lastLoginErr)) + return merged() } now := time.Now() if !t.inMapPoll && (t.lastMapPollEndedAt.IsZero() || now.Sub(t.lastMapPollEndedAt) > 10*time.Second) { - return errNotInMapPoll + add(errNotInMapPoll) + return merged() } const tooIdle = 2*time.Minute + 5*time.Second if d := now.Sub(t.lastStreamedMapResponse).Round(time.Second); d > tooIdle { - return t.networkErrorfLocked("no map response in %v", d) + add(t.networkErrorfLocked("no map response in %v", d)) + return merged() } if !t.derpHomeless { rid := t.derpHomeRegion if rid == 0 { - return errNoDERPHome + add(errNoDERPHome) + return merged() } if !t.derpRegionConnected[rid] { - return t.networkErrorfLocked("not connected to home DERP region %v", rid) + add(t.networkErrorfLocked("not connected to home DERP region %v", rid)) + return merged() } if d := now.Sub(t.derpRegionLastFrame[rid]).Round(time.Second); d > tooIdle { - return t.networkErrorfLocked("haven't heard from home DERP region %v in %v", rid, d) + add(t.networkErrorfLocked("haven't heard from home DERP region %v in %v", rid, d)) + return merged() } } if t.udp4Unbound { - return errNoUDP4Bind + add(errNoUDP4Bind) + return merged() } // TODO: use @@ -653,7 +738,6 @@ func (t *Tracker) overallErrorLocked() error { _ = t.lastStreamedMapResponse _ = t.lastMapRequestHeard - var errs []error for i := range t.MagicSockReceiveFuncs { f := &t.MagicSockReceiveFuncs[i] if f.missing { diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 2605db961..8c8e5ea76 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -368,6 +368,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo if err != nil { return nil, err } + pm.health = sys.HealthTracker() if sds, ok := store.(ipn.StateStoreDialerSetter); ok { sds.SetDialer(dialer.SystemDial) } @@ -770,30 +771,14 @@ func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) { s.AuthURL = b.authURLSticky if prefs := b.pm.CurrentPrefs(); prefs.Valid() && prefs.AutoUpdate().Check { s.ClientVersion = b.lastClientVersion - if cv := b.lastClientVersion; cv != nil && !cv.RunningLatest && cv.LatestVersion != "" { - if cv.UrgentSecurityUpdate { - s.Health = append(s.Health, fmt.Sprintf("Security update available: %v -> %v, run `tailscale update` or `tailscale set --auto-update` to update", version.Short(), cv.LatestVersion)) - } else { - s.Health = append(s.Health, fmt.Sprintf("Update available: %v -> %v, run `tailscale update` or `tailscale set --auto-update` to update", version.Short(), cv.LatestVersion)) - } - } - } - if err := b.health.OverallError(); err != nil { - switch e := err.(type) { - case multierr.Error: - for _, err := range e.Errors() { - s.Health = append(s.Health, err.Error()) - } - default: - s.Health = append(s.Health, err.Error()) - } } + s.Health = b.health.AppendWarnings(s.Health) + + // TODO(bradfitz): move this health check into a health.Warnable + // and remove from here. if m := b.sshOnButUnusableHealthCheckMessageLocked(); m != "" { s.Health = append(s.Health, m) } - if version.IsUnstableBuild() { - s.Health = append(s.Health, "This is an unstable (development) version of Tailscale; frequent updates and bugs are likely") - } if b.netMap != nil { s.CertDomains = append([]string(nil), b.netMap.DNS.CertDomains...) s.MagicDNSSuffix = b.netMap.MagicDNSSuffix() @@ -2517,6 +2502,7 @@ func (b *LocalBackend) tellClientToBrowseToURL(url string) { func (b *LocalBackend) onClientVersion(v *tailcfg.ClientVersion) { b.mu.Lock() b.lastClientVersion = v + b.health.SetLatestVersion(v) b.mu.Unlock() b.send(ipn.Notify{ClientVersion: v}) } diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 6acdacc30..3644e0430 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -16,6 +16,7 @@ import ( "tailscale.com/clientupdate" "tailscale.com/envknob" + "tailscale.com/health" "tailscale.com/ipn" "tailscale.com/types/logger" "tailscale.com/util/clientmetric" @@ -30,8 +31,9 @@ var debug = envknob.RegisterBool("TS_DEBUG_PROFILES") // // It is not safe for concurrent use. type profileManager struct { - store ipn.StateStore - logf logger.Logf + store ipn.StateStore + logf logger.Logf + health *health.Tracker currentUserID ipn.WindowsUserID knownProfiles map[ipn.ProfileID]*ipn.LoginProfile // always non-nil @@ -102,6 +104,7 @@ func (pm *profileManager) SetCurrentUserID(uid ipn.WindowsUserID) error { } pm.currentProfile = prof pm.prefs = prefs + pm.updateHealth() return nil } @@ -285,6 +288,7 @@ func newUnusedID(knownProfiles map[ipn.ProfileID]*ipn.LoginProfile) (ipn.Profile // is not new. func (pm *profileManager) setPrefsLocked(clonedPrefs ipn.PrefsView) error { pm.prefs = clonedPrefs + pm.updateHealth() if pm.currentProfile.ID == "" { return nil } @@ -336,6 +340,7 @@ func (pm *profileManager) SwitchProfile(id ipn.ProfileID) error { return err } pm.prefs = prefs + pm.updateHealth() pm.currentProfile = kp return pm.setAsUserSelectedProfileLocked() } @@ -443,12 +448,20 @@ func (pm *profileManager) writeKnownProfiles() error { return pm.WriteState(ipn.KnownProfilesStateKey, b) } +func (pm *profileManager) updateHealth() { + if !pm.prefs.Valid() { + return + } + pm.health.SetCheckForUpdates(pm.prefs.AutoUpdate().Check) +} + // NewProfile creates and switches to a new unnamed profile. The new profile is // not persisted until SetPrefs is called with a logged-in user. func (pm *profileManager) NewProfile() { metricNewProfile.Add(1) pm.prefs = defaultPrefs + pm.updateHealth() pm.currentProfile = &ipn.LoginProfile{} }