mirror of
https://github.com/tailscale/tailscale.git
synced 2026-07-17 02:54:21 -04:00
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 <bcreane@gmail.com>
This commit is contained in:
Brendan Creane
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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", ""),
|
||||
|
||||
Reference in New Issue
Block a user