From e9733d0de5fbc200bb7b04e2fdbfa1cde201cfd4 Mon Sep 17 00:00:00 2001 From: Brendan Creane Date: Mon, 6 Jul 2026 21:08:17 +0000 Subject: [PATCH] net/dns: withhold DNS takeover until the OS has upstream resolvers On backends that can't do OS-level split DNS, the "." default route comes from the system's own resolvers. If tailscaled reads them before the OS has populated them (e.g. before NetworkManager/systemd-resolved run at boot), it would point the OS at 100.100.100.100 with an empty "." route and SERVFAIL all non-Tailscale DNS. Track base-config readiness as explicit Manager state, withhold takeover while empty, and re-probe until resolvers appear, driven by a backing-off retry timer (general) and a resolv.conf-change event (Linux direct-mode fast path). Fixes #20341 Signed-off-by: Brendan Creane --- net/dns/direct.go | 15 +- net/dns/manager.go | 173 ++++++++++++++++++++++ net/dns/manager_test.go | 316 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 493 insertions(+), 11 deletions(-) diff --git a/net/dns/direct.go b/net/dns/direct.go index 82ddc9012..8cdc2f556 100644 --- a/net/dns/direct.go +++ b/net/dns/direct.go @@ -139,10 +139,11 @@ type directManager struct { // but is better than having non-functioning DNS. renameBroken bool - trampleCount atomic.Int64 - trampleTimer *time.Timer - eventClient *eventbus.Client - trampleDNSPub *eventbus.Publisher[TrampleDNS] + trampleCount atomic.Int64 + trampleTimer *time.Timer + eventClient *eventbus.Client + trampleDNSPub *eventbus.Publisher[TrampleDNS] + osDNSChangedPub *eventbus.Publisher[OSDNSConfigChanged] ctx context.Context // valid until Close ctxClose context.CancelFunc // closes ctx @@ -172,6 +173,7 @@ func newDirectManagerOnFS(logf logger.Logf, health *health.Tracker, bus *eventbu if bus != nil { m.eventClient = bus.Client("dns.directManager") m.trampleDNSPub = eventbus.Publish[TrampleDNS](m.eventClient) + m.osDNSChangedPub = eventbus.Publish[OSDNSConfigChanged](m.eventClient) } m.trampleTimer = time.AfterFunc(trampleWatchDuration, func() { m.trampleCount.Store(0) @@ -517,6 +519,11 @@ func (m *directManager) checkForFileTrample() { m.mu.Unlock() if want == nil { + // Not managing /etc/resolv.conf; the change may be the OS publishing + // resolvers the Manager is waiting on, so nudge it to re-probe. + if m.osDNSChangedPub != nil { + m.osDNSChangedPub.Publish(OSDNSConfigChanged{}) + } return } diff --git a/net/dns/manager.go b/net/dns/manager.go index b34c5b632..804caafc1 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -41,6 +41,36 @@ // ErrNoDNSConfig is returned by RecompileDNSConfig when the Manager // has no existing DNS configuration. ErrNoDNSConfig = errors.New("no DNS configuration") + // errBaseConfigNotReady is returned by compileConfig while takeover is + // withheld for a not-ready base config (see baseConfigStatus). + errBaseConfigNotReady = errors.New("system DNS base config not ready (no upstream resolvers)") +) + +// baseConfigStatus tracks whether the OS has published the upstream resolvers +// that Tailscale forwards the default route to, on backends that can't do +// OS-level split DNS and must blend the system base config into their own. +// +// If tailscaled reads the OS resolver config before the system has populated +// it (e.g. before NetworkManager/systemd-resolved run at boot), taking over +// DNS would point the OS at 100.100.100.100 with an empty "." route and +// SERVFAIL all non-Tailscale lookups. Instead we withhold takeover and re-probe +// until upstream resolvers appear. +// See https://github.com/tailscale/tailscale/issues/20341 +type baseConfigStatus int + +const ( + baseConfigReady baseConfigStatus = iota // OS has upstream resolvers (default); takeover proceeds + baseConfigEmpty // OS has no upstream resolvers; takeover withheld +) + +// baseConfigRetryInterval is the initial delay before the Manager re-probes the +// OS base config while withholding takeover, backing up the best-effort +// resolv.conf watch so the state progresses even on backends without a file +// watch. It backs off exponentially up to maxBaseConfigRetryInterval so a host +// that never gets upstream resolvers eventually goes quiet. +const ( + baseConfigRetryInterval = 2 * time.Second + maxBaseConfigRetryInterval = 60 * time.Second ) // maxActiveQueries returns the maximal number of DNS requests that can @@ -65,6 +95,11 @@ type Manager struct { activeQueriesAtomic int32 + // withholdingTakeover mirrors baseStatus == baseConfigEmpty for lock-free + // reads on the OSDNSConfigChanged hot path, which fires on every unowned + // resolv.conf change. See https://github.com/tailscale/tailscale/issues/20341 + withholdingTakeover atomic.Bool + ctx context.Context // good until Down ctxCancel context.CancelFunc // closes ctx @@ -76,6 +111,9 @@ type Manager struct { mu sync.Mutex // guards following config *Config // Tracks the last viable DNS configuration set by Set. nil on failures other than compilation failures or if set has never been called. queryResponseMapper ResponseMapper + baseStatus baseConfigStatus // readiness of the OS base config + baseRetryTimer *time.Timer // re-probes base config while baseConfigEmpty; nil when not waiting + baseRetryInterval time.Duration // current backoff delay for baseRetryTimer } // NewManager created a new manager from the given config. @@ -118,6 +156,18 @@ func NewManager(logf logger.Logf, oscfg OSConfigurator, health *health.Tracker, m.logf("error setting DNS config: %s", err) } }) + eventbus.SubscribeFunc(m.eventClient, func(OSDNSConfigChanged) { + // The OS resolver config changed while we weren't managing it; re-probe + // in case upstream resolvers we were waiting on have appeared. This + // fires on every unowned resolv.conf change, so skip the lock entirely + // unless we're actually withholding takeover. + if !m.withholdingTakeover.Load() { + return + } + m.mu.Lock() + defer m.mu.Unlock() + m.reprobeBaseConfigLocked("os dns config changed") + }) m.ctx, m.ctxCancel = context.WithCancel(context.Background()) m.logf("using %T", m.os) @@ -218,6 +268,10 @@ func (m *Manager) setLocked(cfg Config) error { m.health.SetHealthy(osConfigurationSetWarnable) m.config = &cfg + // A successful set means we're no longer withholding takeover for a + // missing base config; clear any not-ready state and stop re-probing. + m.setBaseConfigStatusLocked(baseConfigReady) + return nil } @@ -444,6 +498,11 @@ func (m *Manager) compileConfig(cfg Config) (rcfg resolver.Config, ocfg OSConfig } } var defaultRoutes []*dnstype.Resolver + if len(base.Nameservers) == 0 && !appleSplitDNSWorkaround { + // No upstream resolvers yet; withhold takeover. See baseConfigStatus. + m.setBaseConfigStatusLocked(baseConfigEmpty) + return resolver.Config{}, OSConfig{}, errBaseConfigNotReady + } for _, ip := range base.Nameservers { defaultRoutes = append(defaultRoutes, &dnstype.Resolver{Addr: ip.String()}) } @@ -464,6 +523,107 @@ func (m *Manager) disableSplitDNSOptimization() bool { return m.knobs != nil && m.knobs.DisableSplitDNSWhenNoCustomResolvers.Load() } +// dnsBaseConfigNotReadyWarnable warns that takeover is withheld while waiting +// for the system to publish upstream resolvers (see baseConfigStatus); MagicDNS +// is temporarily inactive but the existing system DNS is left untouched. +var dnsBaseConfigNotReadyWarnable = health.Register(&health.Warnable{ + Code: "dns-base-config-not-ready", + Title: "Waiting for system DNS configuration", + Text: health.StaticMessage("Tailscale is waiting for the system DNS configuration to become available before taking over DNS; MagicDNS is temporarily inactive."), + Severity: health.SeverityMedium, + DependsOn: []*health.Warnable{health.NetworkStatusWarnable}, +}) + +// setBaseConfigStatusLocked records the readiness of the OS base config and +// keeps the health warnable and retry timer consistent with it. +// +// m.mu must be held. +func (m *Manager) setBaseConfigStatusLocked(s baseConfigStatus) { + syncs.AssertLocked(&m.mu) + prev := m.baseStatus + m.baseStatus = s + m.withholdingTakeover.Store(s == baseConfigEmpty) + switch s { + case baseConfigEmpty: + if prev != baseConfigEmpty { + m.logf("base config not ready (no OS upstream resolvers); withholding takeover") + } + m.health.SetUnhealthy(dnsBaseConfigNotReadyWarnable, nil) + m.armBaseConfigRetryLocked() + default: // baseConfigReady + if prev == baseConfigEmpty { + m.logf("base config became ready; taking over DNS") + } + m.health.SetHealthy(dnsBaseConfigNotReadyWarnable) + m.stopBaseConfigRetryLocked() + } +} + +// armBaseConfigRetryLocked ensures a timer is running to re-probe the OS base +// config, backing up the best-effort resolv.conf watch so the empty state +// progresses even on backends without a file watch. The delay backs off +// exponentially up to maxBaseConfigRetryInterval. +// +// m.mu must be held. +func (m *Manager) armBaseConfigRetryLocked() { + syncs.AssertLocked(&m.mu) + if m.baseRetryTimer != nil { + return // already waiting + } + if m.ctx.Err() != nil { + return // shutting down; don't schedule more work + } + if m.baseRetryInterval == 0 { + m.baseRetryInterval = baseConfigRetryInterval + } + m.baseRetryTimer = time.AfterFunc(m.baseRetryInterval, func() { + m.mu.Lock() + defer m.mu.Unlock() + // Clear the handle so reprobe can re-arm with a backed-off delay. + m.baseRetryTimer = nil + m.baseRetryInterval = min(m.baseRetryInterval*2, maxBaseConfigRetryInterval) + m.reprobeBaseConfigLocked("retry timer") + }) +} + +// stopBaseConfigRetryLocked stops any pending base-config retry and resets the +// backoff. +// +// m.mu must be held. +func (m *Manager) stopBaseConfigRetryLocked() { + syncs.AssertLocked(&m.mu) + if m.baseRetryTimer != nil { + m.baseRetryTimer.Stop() + m.baseRetryTimer = nil + } + m.baseRetryInterval = 0 +} + +// reprobeBaseConfigLocked re-applies the last good config so compileConfig +// re-reads the OS base config. No-op unless we're withholding takeover. If the +// base config is still not ready afterwards (e.g. still empty, or GetBaseConfig +// returned a transient error), it re-arms the retry timer so the state always +// progresses. +// +// m.mu must be held. +func (m *Manager) reprobeBaseConfigLocked(reason string) { + syncs.AssertLocked(&m.mu) + if m.baseStatus != baseConfigEmpty || m.config == nil { + return + } + if m.ctx.Err() != nil { + return // shutting down; don't touch the OS config after Down + } + m.logf("[v1] re-probing base config (%s)", reason) + if err := m.setLocked(*m.config); err != nil && err != errBaseConfigNotReady { + // A non-empty failure (e.g. a transient GetBaseConfig error) leaves + // baseStatus at baseConfigEmpty without re-arming via + // setBaseConfigStatusLocked, so ensure a retry is scheduled here. + m.logf("error re-probing base config: %s", err) + m.armBaseConfigRetryLocked() + } +} + var isSandboxedMacOS = version.IsSandboxedMacOS // toIPsOnly returns only the IP portion of dnstype.Resolver. @@ -529,6 +689,16 @@ type TrampleDNS struct { TramplesInTimeout int64 } +// OSDNSConfigChanged is an event indicating that the OS resolver +// configuration (e.g. /etc/resolv.conf) changed while Tailscale was not +// managing it, used to re-probe a not-ready base config (see baseConfigStatus). +// +// This is only a latency optimization, and only directManager publishes it +// (from its resolv.conf inotify watch). The baseConfigRetryInterval timer is +// the general convergence mechanism that works on every backend; this event +// just lets Linux direct mode recover faster than the next timer tick. +type OSDNSConfigChanged struct{} + // dnsTCPSession services DNS requests sent over TCP. type dnsTCPSession struct { m *Manager @@ -654,6 +824,9 @@ func (m *Manager) Down() error { return nil } m.ctxCancel() + m.mu.Lock() + m.stopBaseConfigRetryLocked() + m.mu.Unlock() if err := m.os.Close(); err != nil { return err } diff --git a/net/dns/manager_test.go b/net/dns/manager_test.go index b10c89056..a12185a1e 100644 --- a/net/dns/manager_test.go +++ b/net/dns/manager_test.go @@ -38,12 +38,14 @@ ) type fakeOSConfigurator struct { - SplitDNS bool - BaseConfig OSConfig + SplitDNS bool - OSConfig OSConfig - ResolverConfig resolver.Config - GetBaseConfigErr *error + mu sync.Mutex // guards BaseConfig/baseConfigErrOnce; only contended in concurrent (synctest) tests + BaseConfig OSConfig + baseConfigErrOnce error // if non-nil, returned by the next GetBaseConfig then cleared + OSConfig OSConfig + ResolverConfig resolver.Config + GetBaseConfigErr *error } func (c *fakeOSConfigurator) SetDNS(cfg OSConfig) error { @@ -62,10 +64,32 @@ func (c *fakeOSConfigurator) SupportsSplitDNS() bool { return c.SplitDNS } +// setBaseConfig updates BaseConfig safely for tests where a background +// re-probe goroutine may read it concurrently via GetBaseConfig. +func (c *fakeOSConfigurator) setBaseConfig(cfg OSConfig) { + c.mu.Lock() + defer c.mu.Unlock() + c.BaseConfig = cfg +} + +// setBaseConfigErrOnce arms GetBaseConfig to return err exactly once (then +// resume returning BaseConfig), simulating a transient read failure. +func (c *fakeOSConfigurator) setBaseConfigErrOnce(err error) { + c.mu.Lock() + defer c.mu.Unlock() + c.baseConfigErrOnce = err +} + func (c *fakeOSConfigurator) GetBaseConfig() (OSConfig, error) { if c.GetBaseConfigErr != nil { return OSConfig{}, *c.GetBaseConfigErr } + c.mu.Lock() + defer c.mu.Unlock() + if err := c.baseConfigErrOnce; err != nil { + c.baseConfigErrOnce = nil + return OSConfig{}, err + } return c.BaseConfig, nil } @@ -1163,12 +1187,290 @@ func TestConfigRecompilation(t *testing.T) { } } +// TestBaseConfigEmptyWithholdsTakeover is a regression test for +// https://github.com/tailscale/tailscale/issues/20341: when the OS can't do +// split DNS and the system base config has no upstream nameservers (e.g. +// tailscaled started before NetworkManager/systemd-resolved populated +// /etc/resolv.conf), the Manager must withhold takeover rather than point the +// OS at 100.100.100.100 with an empty "." route, which would SERVFAIL all +// non-Tailscale DNS. The healthy path (a populated base config) is covered by +// TestManager's "routes-multi" case. +func TestBaseConfigEmptyWithholdsTakeover(t *testing.T) { + f := &fakeOSConfigurator{ + SplitDNS: false, // can't split at OS -> blend base config path + BaseConfig: OSConfig{}, // empty: no upstream nameservers + } + bus := eventbustest.NewBus(t) + ht := health.NewTracker(bus) + dialer := tsdial.NewDialer(netmon.NewStatic()) + dialer.SetBus(bus) + m := NewManager(t.Logf, f, ht, dialer, nil, nil, "linux", bus) + m.resolver.TestOnlySetHook(f.SetResolver) + + // Split-DNS-only tailnet config: a route plus MagicDNS, no DefaultResolvers. + cfg := Config{ + Routes: upstreams("ts.net", "199.247.155.53"), + SearchDomains: fqdns("foo.ts.net"), + } + if err := m.Set(cfg); err == nil { + t.Fatal("m.Set: want error for empty base config, got nil") + } + // Must not have installed a self-referential OS config that funnels all DNS + // to 100.100.100.100 with no upstream to forward to. + if len(f.OSConfig.Nameservers) != 0 { + t.Errorf("OSConfig.Nameservers = %v; want none (should not hijack OS DNS)", f.OSConfig.Nameservers) + } + m.mu.Lock() + gotStatus := m.baseStatus + m.mu.Unlock() + if gotStatus != baseConfigEmpty { + t.Errorf("baseStatus = %v; want baseConfigEmpty", gotStatus) + } + if _, ok := ht.CurrentState().Warnings[dnsBaseConfigNotReadyWarnable.Code]; !ok { + t.Error("dnsBaseConfigNotReadyWarnable not set; want set while withholding takeover") + } +} + +// TestBaseConfigBecomesReady verifies that once the OS publishes upstream +// resolvers, an OSDNSConfigChanged event drives the Manager out of the +// withheld state and into a normal takeover, clearing the health warning. +func TestBaseConfigBecomesReady(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + f := &fakeOSConfigurator{ + SplitDNS: false, + BaseConfig: OSConfig{}, // start empty + } + bus := eventbustest.NewBus(t) + ht := health.NewTracker(bus) + dialer := tsdial.NewDialer(netmon.NewStatic()) + dialer.SetBus(bus) + m := NewManager(t.Logf, f, ht, dialer, nil, nil, "linux", bus) + m.resolver.TestOnlySetHook(f.SetResolver) + + cfg := Config{ + Routes: upstreams("ts.net", "199.247.155.53"), + SearchDomains: fqdns("foo.ts.net"), + } + if err := m.Set(cfg); err == nil { + t.Fatal("m.Set: want error for empty base config, got nil") + } + + // The OS finally publishes its resolvers. + f.setBaseConfig(OSConfig{Nameservers: mustIPs("8.8.8.8")}) + + inj := eventbustest.NewInjector(t, bus) + eventbustest.Inject(inj, OSDNSConfigChanged{}) + synctest.Wait() + + if len(f.OSConfig.Nameservers) == 0 { + t.Error("OSConfig.Nameservers empty after base config became ready; want takeover") + } + m.mu.Lock() + gotStatus := m.baseStatus + m.mu.Unlock() + if gotStatus != baseConfigReady { + t.Errorf("baseStatus = %v; want baseConfigReady", gotStatus) + } + if _, ok := ht.CurrentState().Warnings[dnsBaseConfigNotReadyWarnable.Code]; ok { + t.Error("dnsBaseConfigNotReadyWarnable still set after takeover; want cleared") + } + }) +} + +// TestBaseConfigTimeoutReprobe verifies the fixed-interval retry timer (which +// backs up the best-effort resolv.conf watch) re-probes and takes over once +// the OS base config becomes available, even with no OSDNSConfigChanged event. +// This is the convergence path for backends without a file watch (BSDs, +// resolvconf family). +func TestBaseConfigTimeoutReprobe(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + f := &fakeOSConfigurator{ + SplitDNS: false, + BaseConfig: OSConfig{}, // start empty + } + bus := eventbustest.NewBus(t) + dialer := tsdial.NewDialer(netmon.NewStatic()) + dialer.SetBus(bus) + m := NewManager(t.Logf, f, health.NewTracker(bus), dialer, nil, nil, "linux", bus) + m.resolver.TestOnlySetHook(f.SetResolver) + + cfg := Config{ + Routes: upstreams("ts.net", "199.247.155.53"), + SearchDomains: fqdns("foo.ts.net"), + } + if err := m.Set(cfg); err == nil { + t.Fatal("m.Set: want error for empty base config, got nil") + } + + // Base config becomes available with no event; only the timer will notice. + f.setBaseConfig(OSConfig{Nameservers: mustIPs("8.8.8.8")}) + + time.Sleep(baseConfigRetryInterval + time.Second) + synctest.Wait() + + if len(f.OSConfig.Nameservers) == 0 { + t.Error("OSConfig.Nameservers empty after retry timer; want takeover") + } + m.mu.Lock() + gotStatus := m.baseStatus + gotTimer := m.baseRetryTimer + m.mu.Unlock() + if gotStatus != baseConfigReady { + t.Errorf("baseStatus = %v; want baseConfigReady", gotStatus) + } + if gotTimer != nil { + t.Error("baseRetryTimer still armed after reaching ready; want stopped") + } + }) +} + +// TestBaseConfigTransientErrorKeepsRetrying is a regression test: a transient +// GetBaseConfig error during a timer re-probe must not permanently drop the +// retry timer (issue #20341 review finding). After the transient error, the +// timer must re-arm and eventually converge once the base config is available. +func TestBaseConfigTransientErrorKeepsRetrying(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + f := &fakeOSConfigurator{ + SplitDNS: false, + BaseConfig: OSConfig{}, // start empty -> withhold + arm timer + } + bus := eventbustest.NewBus(t) + dialer := tsdial.NewDialer(netmon.NewStatic()) + dialer.SetBus(bus) + m := NewManager(t.Logf, f, health.NewTracker(bus), dialer, nil, nil, "linux", bus) + m.resolver.TestOnlySetHook(f.SetResolver) + + cfg := Config{ + Routes: upstreams("ts.net", "199.247.155.53"), + SearchDomains: fqdns("foo.ts.net"), + } + if err := m.Set(cfg); err == nil { + t.Fatal("m.Set: want error for empty base config, got nil") + } + + // The next re-probe hits a transient GetBaseConfig error. + f.setBaseConfigErrOnce(errors.New("transient resolvconf failure")) + time.Sleep(baseConfigRetryInterval + time.Second) + synctest.Wait() + + // The transient error must not have dropped the retry: still withholding, + // timer re-armed. + m.mu.Lock() + gotStatus := m.baseStatus + gotTimer := m.baseRetryTimer + m.mu.Unlock() + if gotStatus != baseConfigEmpty { + t.Errorf("after transient error: baseStatus = %v; want baseConfigEmpty", gotStatus) + } + if gotTimer == nil { + t.Fatal("after transient error: baseRetryTimer is nil; want re-armed (would be stuck forever)") + } + + // Now the base config becomes available; the re-armed (backed-off) timer + // should converge. + f.setBaseConfig(OSConfig{Nameservers: mustIPs("8.8.8.8")}) + time.Sleep(maxBaseConfigRetryInterval + time.Second) + synctest.Wait() + + if len(f.OSConfig.Nameservers) == 0 { + t.Error("OSConfig.Nameservers empty after recovery; want takeover") + } + m.mu.Lock() + gotStatus = m.baseStatus + m.mu.Unlock() + if gotStatus != baseConfigReady { + t.Errorf("baseStatus = %v; want baseConfigReady", gotStatus) + } + }) +} + +// TestBaseConfigReadyStaysReady verifies the healthy path never enters the +// withheld state or arms the retry timer. +func TestBaseConfigReadyStaysReady(t *testing.T) { + f := &fakeOSConfigurator{ + SplitDNS: false, + BaseConfig: OSConfig{Nameservers: mustIPs("8.8.8.8")}, + } + bus := eventbustest.NewBus(t) + dialer := tsdial.NewDialer(netmon.NewStatic()) + dialer.SetBus(bus) + m := NewManager(t.Logf, f, health.NewTracker(bus), dialer, nil, nil, "linux", bus) + m.resolver.TestOnlySetHook(f.SetResolver) + + cfg := Config{ + Routes: upstreams("ts.net", "199.247.155.53"), + SearchDomains: fqdns("foo.ts.net"), + } + if err := m.Set(cfg); err != nil { + t.Fatalf("m.Set: %v", err) + } + m.mu.Lock() + gotStatus := m.baseStatus + gotTimer := m.baseRetryTimer + m.mu.Unlock() + if gotStatus != baseConfigReady { + t.Errorf("baseStatus = %v; want baseConfigReady", gotStatus) + } + if gotTimer != nil { + t.Error("baseRetryTimer armed on healthy path; want none") + } +} + +// TestBaseConfigNoReprobeAfterDown is a regression test: after Down() cancels +// the Manager's context, a racing retry-timer fire or OSDNSConfigChanged event +// must not re-apply DNS takeover on the (now closed) OSConfigurator +// (issue #20341 review finding). +func TestBaseConfigNoReprobeAfterDown(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + f := &fakeOSConfigurator{ + SplitDNS: false, + BaseConfig: OSConfig{}, // empty -> withhold + arm timer + } + bus := eventbustest.NewBus(t) + dialer := tsdial.NewDialer(netmon.NewStatic()) + dialer.SetBus(bus) + m := NewManager(t.Logf, f, health.NewTracker(bus), dialer, nil, nil, "linux", bus) + m.resolver.TestOnlySetHook(f.SetResolver) + + cfg := Config{ + Routes: upstreams("ts.net", "199.247.155.53"), + SearchDomains: fqdns("foo.ts.net"), + } + if err := m.Set(cfg); err == nil { + t.Fatal("m.Set: want error for empty base config, got nil") + } + + // Shut down while withholding, then make the base config "ready" and + // try to drive a reprobe via both paths. Neither should take over. + if err := m.Down(); err != nil { + t.Fatalf("Down: %v", err) + } + f.setBaseConfig(OSConfig{Nameservers: mustIPs("8.8.8.8")}) + + m.mu.Lock() + m.reprobeBaseConfigLocked("post-down test") + m.mu.Unlock() + + time.Sleep(maxBaseConfigRetryInterval + time.Second) + synctest.Wait() + + if len(f.OSConfig.Nameservers) != 0 { + t.Errorf("OSConfig.Nameservers = %v after Down; want none (no takeover after shutdown)", f.OSConfig.Nameservers) + } + m.mu.Lock() + gotTimer := m.baseRetryTimer + m.mu.Unlock() + if gotTimer != nil { + t.Error("baseRetryTimer re-armed after Down; want none") + } + }) +} + func TestTrampleRetrample(t *testing.T) { synctest.Test(t, func(t *testing.T) { f := &fakeOSConfigurator{} f.BaseConfig = OSConfig{ - Nameservers: mustIPs("1.1.1.1"), - } + Nameservers: mustIPs("1.1.1.1")} config := Config{ Routes: upstreams("ts.net", "69.4.2.0", "foo.ts.net", ""),