From 43123a1bd473ff57459a37c0c80db3619704dc9d Mon Sep 17 00:00:00 2001 From: Andrea Gottardo Date: Tue, 25 Jun 2024 21:55:46 -0700 Subject: [PATCH] health: use Warnables for system DNS configuration errors Updates tailscale/tailscale#4136 Begins to use Warnables to replace the deprecated DNS health subsystem. Signed-off-by: Andrea Gottardo --- health/health.go | 37 +------------------------------------ net/dns/manager.go | 21 +++++++++++++++------ net/dns/manager_linux.go | 12 +++++++++++- net/dns/resolved.go | 6 ++++-- net/dns/resolver/tsdns.go | 3 +-- wgengine/userspace.go | 4 +++- 6 files changed, 35 insertions(+), 48 deletions(-) diff --git a/health/health.go b/health/health.go index c1740b1f1..a8a7c2f7f 100644 --- a/health/health.go +++ b/health/health.go @@ -110,15 +110,6 @@ const ( // SysRouter is the name of the wgengine/router subsystem. SysRouter = Subsystem("router") - // SysDNS is the name of the net/dns subsystem. - SysDNS = Subsystem("dns") - - // SysDNSOS is the name of the net/dns OSConfigurator subsystem. - SysDNSOS = Subsystem("dns-os") - - // SysDNSManager is the name of the net/dns manager subsystem. - SysDNSManager = Subsystem("dns-manager") - // SysTKA is the name of the tailnet key authority subsystem. SysTKA = Subsystem("tailnet-lock") ) @@ -126,7 +117,7 @@ const ( var subsystemsWarnables = map[Subsystem]*Warnable{} func init() { - for _, s := range []Subsystem{SysRouter, SysDNS, SysDNSOS, SysDNSManager, SysTKA} { + for _, s := range []Subsystem{SysRouter, SysTKA} { w := Register(&Warnable{ Code: WarnableCode(s), Severity: SeverityMedium, @@ -406,32 +397,6 @@ func (t *Tracker) SetRouterHealth(err error) { t.setErr(SysRouter, err) } // Deprecated: Warnables should be preferred over Subsystem errors. func (t *Tracker) RouterHealth() error { return t.get(SysRouter) } -// SetDNSHealth sets the state of the net/dns.Manager -// -// Deprecated: Warnables should be preferred over Subsystem errors. -func (t *Tracker) SetDNSHealth(err error) { t.setErr(SysDNS, err) } - -// DNSHealth returns the net/dns.Manager error state. -// -// Deprecated: Warnables should be preferred over Subsystem errors. -func (t *Tracker) DNSHealth() error { return t.get(SysDNS) } - -// SetDNSOSHealth sets the state of the net/dns.OSConfigurator -// -// Deprecated: Warnables should be preferred over Subsystem errors. -func (t *Tracker) SetDNSOSHealth(err error) { t.setErr(SysDNSOS, err) } - -// SetDNSManagerHealth sets the state of the Linux net/dns manager's -// discovery of the /etc/resolv.conf situation. -// -// Deprecated: Warnables should be preferred over Subsystem errors. -func (t *Tracker) SetDNSManagerHealth(err error) { t.setErr(SysDNSManager, err) } - -// DNSOSHealth returns the net/dns.OSConfigurator error state. -// -// Deprecated: Warnables should be preferred over Subsystem errors. -func (t *Tracker) DNSOSHealth() error { return t.get(SysDNSOS) } - // SetTKAHealth sets the health of the tailnet key authority. // // Deprecated: Warnables should be preferred over Subsystem errors. diff --git a/net/dns/manager.go b/net/dns/manager.go index 88fe94fac..7040be7e9 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -147,20 +147,28 @@ func (m *Manager) setLocked(cfg Config) error { ocfg.WriteToBufioWriter(w) })) - if err := m.resolver.SetConfig(rcfg); err != nil { - return err - } + m.resolver.SetConfig(rcfg) + if err := m.os.SetDNS(ocfg); err != nil { - m.health.SetDNSOSHealth(err) + m.health.SetUnhealthy(OSConfigWarnable, health.Args{health.ArgError: err.Error()}) return err } - m.health.SetDNSOSHealth(nil) + m.health.SetHealthy(OSConfigWarnable) m.config = &cfg return nil } +var OSConfigWarnable = health.Register(&health.Warnable{ + Code: "dns-failed-osconfig", + Severity: health.SeverityMedium, + Title: "System DNS configuration failed", + Text: func(args health.Args) string { + return "Could not configure this device to use the Tailscale DNS resolver. Error: " + args[health.ArgError] + }, +}) + // compileHostEntries creates a list of single-label resolutions possible // from the configured hosts and search domains. // The entries are compiled in the order of the search domains, then the hosts. @@ -315,9 +323,10 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig // This is currently (2022-10-13) expected on certain iOS and macOS // builds. } else { - m.health.SetDNSOSHealth(err) + m.health.SetUnhealthy(OSConfigWarnable, health.Args{health.ArgError: err.Error()}) return resolver.Config{}, OSConfig{}, err } + m.health.SetHealthy(OSConfigWarnable) } if baseCfg == nil { diff --git a/net/dns/manager_linux.go b/net/dns/manager_linux.go index 3ba3022b6..dcb178179 100644 --- a/net/dns/manager_linux.go +++ b/net/dns/manager_linux.go @@ -275,7 +275,7 @@ func dnsMode(logf logger.Logf, health *health.Tracker, env newOSConfigEnv) (ret return "direct", nil } - health.SetDNSManagerHealth(errors.New("systemd-resolved and NetworkManager are wired together incorrectly; MagicDNS will probably not work. For more info, see https://tailscale.com/s/resolved-nm")) + health.SetUnhealthy(resolvedNetworkManagerConflictWarnable, nil) dbg("nm-safe", "no") return "systemd-resolved", nil default: @@ -284,6 +284,16 @@ func dnsMode(logf logger.Logf, health *health.Tracker, env newOSConfigEnv) (ret } } +// resolvedNetworkManagerConflictWarnable reports whether the system is in a +// state where NetworkManager and systemd-resolved are in conflict, and we should +// warn the user about it. +var resolvedNetworkManagerConflictWarnable = health.Register(&health.Warnable{ + Code: "resolved-nm-conflict", + Title: "NetworkManager and systemd-resolved conflict", + Text: health.StaticMessage("systemd-resolved and NetworkManager are wired together incorrectly; MagicDNS will probably not work. For more info, see https://tailscale.com/s/resolved-nm"), + Severity: health.SeverityMedium, +}) + func nmVersionBetween(first, last string) (bool, error) { conn, err := dbus.SystemBus() if err != nil { diff --git a/net/dns/resolved.go b/net/dns/resolved.go index d82d3fc31..fe7345fba 100644 --- a/net/dns/resolved.go +++ b/net/dns/resolved.go @@ -165,7 +165,7 @@ func (m *resolvedManager) run(ctx context.Context) { // Reset backoff and SetNSOSHealth after successful on reconnect. bo.BackOff(ctx, nil) - m.health.SetDNSOSHealth(nil) + m.health.SetHealthy(OSConfigWarnable) return nil } @@ -243,9 +243,11 @@ func (m *resolvedManager) run(ctx context.Context) { // Set health while holding the lock, because this will // graciously serialize the resync's health outcome with a // concurrent SetDNS call. - m.health.SetDNSOSHealth(err) if err != nil { m.logf("failed to configure systemd-resolved: %v", err) + m.health.SetUnhealthy(OSConfigWarnable, health.Args{health.ArgError: err.Error()}) + } else { + m.health.SetHealthy(OSConfigWarnable) } } } diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index a140c7e93..6036cf99f 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -253,7 +253,7 @@ func (r *Resolver) SetMissingUpstreamRecovery(f func()) { func (r *Resolver) TestOnlySetHook(hook func(Config)) { r.saveConfigForTests = hook } -func (r *Resolver) SetConfig(cfg Config) error { +func (r *Resolver) SetConfig(cfg Config) { if r.saveConfigForTests != nil { r.saveConfigForTests(cfg) } @@ -273,7 +273,6 @@ func (r *Resolver) SetConfig(cfg Config) error { r.localDomains = cfg.LocalDomains r.hostToIP = cfg.Hosts r.ipToHost = reverse - return nil } // Close shuts down the resolver and ensures poll goroutines have exited. diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 6399476c8..50600f647 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -994,13 +994,15 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, // assigned address. e.logf("wgengine: Reconfig: configuring DNS") err = e.dns.Set(*dnsCfg) - e.health.SetDNSHealth(err) if err != nil { + e.health.SetUnhealthy(dns.OSConfigWarnable, health.Args{health.ArgError: err.Error()}) return err } if err := e.reconfigureVPNIfNecessary(); err != nil { + e.health.SetUnhealthy(dns.OSConfigWarnable, health.Args{health.ArgError: err.Error()}) return err } + e.health.SetHealthy(dns.OSConfigWarnable) } // Shutdown the network logger.