mirror of
https://github.com/tailscale/tailscale.git
synced 2026-05-05 04:06:35 +02:00
net/dns: fix TestDNSTrampleRecovery failure under flakestress
The test had two problems: 1. runFileWatcher passed hardcoded "/etc/" to the inotify watcher, but the test filesystem uses a temp directory prefix. The watcher was watching the real /etc/, never seeing the test's file writes. 2. The test's watchFile used gonotify.NewDirWatcher which creates goroutines that block on real inotify syscalls. These don't work inside synctest's fake-time bubble. The test only passed standalone by accident: gonotify walks /etc/ on startup producing fake events that happened to trigger trample detection at the right time. Fix the path issue by adding ActualPath to the wholeFileFS interface, which translates logical paths (like "/etc/resolv.conf") to real filesystem paths (respecting any test prefix). Use it in runFileWatcher so the inotify watch targets the correct directory. Replace gonotify in the test with a one-shot timer that synctest can advance through fake time, reliably triggering the trample check. Fixes #19400 Change-Id: Idb252881ec24d0ab3b3c1d154dbdaf532db837d4 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
parent
27f1d4c15d
commit
49eb1b5d26
@ -442,7 +442,9 @@ func (m *directManager) runFileWatcher() {
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if err := watchFile(m.ctx, "/etc/", resolvConf, m.checkForFileTrample); err != nil {
|
||||
dir := m.fs.ActualPath(filepath.Dir(resolvConf))
|
||||
file := m.fs.ActualPath(resolvConf)
|
||||
if err := watchFile(m.ctx, dir, file, m.checkForFileTrample); err != nil {
|
||||
// This is all best effort for now, so surface warnings to users.
|
||||
m.logf("dns: inotify: %s", err)
|
||||
}
|
||||
@ -597,6 +599,19 @@ type wholeFileFS interface {
|
||||
ReadFile(name string) ([]byte, error)
|
||||
Remove(name string) error
|
||||
Rename(oldName, newName string) error
|
||||
// ActualPath returns the real filesystem path for the given absolute
|
||||
// logical path. All other methods in this interface accept logical
|
||||
// paths (like "/etc/resolv.conf") and translate them internally;
|
||||
// ActualPath exposes that same translation for callers that need
|
||||
// the real path for use outside the interface (e.g. setting up an
|
||||
// inotify watch on the correct directory).
|
||||
//
|
||||
// For directFS with an empty prefix (production), the input is
|
||||
// returned unchanged ("/etc" → "/etc"). For directFS with a test
|
||||
// prefix like "/tmp/test123", the prefix is joined
|
||||
// ("/etc" → "/tmp/test123/etc"). For wslFS the input is returned
|
||||
// unchanged, since paths are passed through to wsl.exe as-is.
|
||||
ActualPath(name string) string
|
||||
Stat(name string) (isRegular bool, err error)
|
||||
Truncate(name string) error
|
||||
WriteFile(name string, contents []byte, perm os.FileMode) error
|
||||
@ -613,6 +628,8 @@ type directFS struct {
|
||||
|
||||
func (fs directFS) path(name string) string { return filepath.Join(fs.prefix, name) }
|
||||
|
||||
func (fs directFS) ActualPath(name string) string { return fs.path(name) }
|
||||
|
||||
func (fs directFS) Stat(name string) (isRegular bool, err error) {
|
||||
fi, err := os.Stat(fs.path(name))
|
||||
if err != nil {
|
||||
|
||||
@ -7,14 +7,12 @@ package dns
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/netip"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
|
||||
"github.com/illarion/gonotify/v3"
|
||||
"time"
|
||||
|
||||
"tailscale.com/util/dnsname"
|
||||
"tailscale.com/util/eventbus/eventbustest"
|
||||
@ -77,33 +75,20 @@ search ts.net ts-dns.test
|
||||
})
|
||||
}
|
||||
|
||||
// watchFile is generally copied from linuxtrample, but cancels the context
|
||||
// after the first call to cb() after the first trample to end the test.
|
||||
// watchFile is a test implementation of the file watcher that uses a timer
|
||||
// instead of inotify. Real inotify (gonotify.NewDirWatcher) creates goroutines
|
||||
// that block on real syscalls, which don't work inside synctest's fake-time
|
||||
// bubble. Instead, we use a one-shot timer that synctest.Wait() will advance,
|
||||
// triggering a callback to check for file trampling.
|
||||
func watchFile(ctx context.Context, dir, filename string, cb func()) error {
|
||||
ctx, cancel := context.WithCancel(ctx)
|
||||
defer cancel()
|
||||
|
||||
const events = gonotify.IN_ATTRIB |
|
||||
gonotify.IN_CLOSE_WRITE |
|
||||
gonotify.IN_CREATE |
|
||||
gonotify.IN_DELETE |
|
||||
gonotify.IN_MODIFY |
|
||||
gonotify.IN_MOVE
|
||||
|
||||
watcher, err := gonotify.NewDirWatcher(ctx, events, dir)
|
||||
if err != nil {
|
||||
return fmt.Errorf("NewDirWatcher: %w", err)
|
||||
}
|
||||
|
||||
for {
|
||||
select {
|
||||
case event := <-watcher.C:
|
||||
if event.Name == filename {
|
||||
cb()
|
||||
cancel()
|
||||
}
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
}
|
||||
timer := time.NewTimer(time.Millisecond)
|
||||
defer timer.Stop()
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
case <-timer.C:
|
||||
cb()
|
||||
}
|
||||
<-ctx.Done()
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
@ -316,6 +316,7 @@ func (m memFS) Stat(name string) (isRegular bool, err error) {
|
||||
func (m memFS) Chmod(name string, mode os.FileMode) error { panic("TODO") }
|
||||
func (m memFS) Rename(oldName, newName string) error { panic("TODO") }
|
||||
func (m memFS) Remove(name string) error { panic("TODO") }
|
||||
func (m memFS) ActualPath(name string) string { return name }
|
||||
func (m memFS) ReadFile(name string) ([]byte, error) {
|
||||
v, ok := m[name]
|
||||
if !ok {
|
||||
|
||||
@ -148,6 +148,8 @@ type wslFS struct {
|
||||
distro string
|
||||
}
|
||||
|
||||
func (fs wslFS) ActualPath(name string) string { return name }
|
||||
|
||||
func (fs wslFS) Stat(name string) (isRegular bool, err error) {
|
||||
err = wslRun(fs.cmd("test", "-f", name))
|
||||
if ee, _ := err.(*exec.ExitError); ee != nil {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user