tstest/integration: add tests for tun mode (requiring root)

Updates #7894

Change-Id: Iff0b07b21ae28c712dd665b12918fa28d6f601d0
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

.github/workflows/test.yml

@@ -90,6 +90,8 @@ jobs:
         sudo apt-get -y install qemu-user
     - name: build test wrapper
       run: ./tool/go build -o /tmp/testwrapper ./cmd/testwrapper
+    - name: integration tests as root
+      run: PATH=$PWD/tool:$PATH /tmp/testwrapper --sudo ./tstest/integration/ ${{matrix.buildflags}}
     - name: test all
       run: PATH=$PWD/tool:$PATH /tmp/testwrapper ./... ${{matrix.buildflags}}
       env:

cmd/testwrapper/testwrapper.go

@@ -73,11 +73,15 @@ type goTestOutput struct {
 // It calls close(ch) when it's done.
 func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []string, ch chan<- *testAttempt) error {
 	defer close(ch)
-	args := []string{"test", "-json", pt.Pattern}
+	args := []string{"test", "--json"}
+	if *flagSudo {
+		args = append(args, "--exec", "sudo -E")
+	}
+	args = append(args, pt.Pattern)
 	args = append(args, otherArgs...)
 	if len(pt.Tests) > 0 {
 		runArg := strings.Join(pt.Tests, "|")
-		args = append(args, "-run", runArg)
+		args = append(args, "--run", runArg)
 	}
 	if debug {
 		fmt.Println("running", strings.Join(args, " "))
@@ -177,6 +181,11 @@ func runTests(ctx context.Context, attempt int, pt *packageTests, otherArgs []st
 	return nil
 }
 
+var (
+	flagVerbose = flag.Bool("v", false, "verbose")
+	flagSudo    = flag.Bool("sudo", false, "run tests with -exec=sudo")
+)
+
 func main() {
 	ctx := context.Background()
 
@@ -187,7 +196,6 @@ func main() {
 	// We run `go test -json` which returns the same information as `go test -v`,
 	// but in a machine-readable format. So this flag is only for testwrapper's
 	// output.
-	v := flag.Bool("v", false, "verbose")
 
 	flag.Usage = func() {
 		fmt.Println("usage: testwrapper [testwrapper-flags] [pattern] [build/test flags & test binary flags]")
@@ -285,7 +293,7 @@ type nextRun struct {
 					printPkgOutcome(tr.pkg, tr.outcome, thisRun.attempt)
 					continue
 				}
-				if *v || tr.outcome == "fail" {
+				if *flagVerbose || tr.outcome == "fail" {
 					io.Copy(os.Stdout, &tr.logs)
 				}
 				if tr.outcome != "fail" {

tstest/integration/integration.go

@@ -142,6 +142,16 @@ func findGo() (string, error) {
 	// 2. Look for a go binary in runtime.GOROOT()/bin if runtime.GOROOT() is non-empty.
 	// 3. Look for a go binary in $PATH.
 
+	// For tests we want to run as root on GitHub actions, we run with -exec=sudo,
+	// but that results in this test running with a different PATH and picking the
+	// wrong Go. So hard code the GitHub Actions case.
+	if os.Getuid() == 0 && os.Getenv("GITHUB_ACTIONS") == "true" {
+		const sudoGithubGo = "/home/runner/.cache/tailscale-go/bin/go"
+		if _, err := os.Stat(sudoGithubGo); err == nil {
+			return sudoGithubGo, nil
+		}
+	}
+
 	paths := strings.FieldsFunc(os.Getenv("PATH"), func(r rune) bool { return os.IsPathSeparator(uint8(r)) })
 	if len(paths) > 0 {
 		candidate := filepath.Join(paths[0], "go"+exe())

tstest/integration/integration_test.go

@@ -68,6 +68,27 @@ func TestMain(m *testing.M) {
 	os.Exit(0)
 }
 
+// Tests that tailscaled starts up in TUN mode, and also without data races:
+// https://github.com/tailscale/tailscale/issues/7894
+func TestTUNMode(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("skipping when not root")
+	}
+	t.Parallel()
+	env := newTestEnv(t)
+	env.tunMode = true
+	n1 := newTestNode(t, env)
+	d1 := n1.StartDaemon()
+
+	n1.AwaitResponding()
+	n1.MustUp()
+
+	t.Logf("Got IP: %v", n1.AwaitIP4())
+	n1.AwaitRunning()
+
+	d1.MustCleanShutdown(t)
+}
+
 func TestOneNodeUpNoAuth(t *testing.T) {
 	t.Parallel()
 	env := newTestEnv(t)
@@ -808,9 +829,10 @@ func TestLogoutRemovesAllPeers(t *testing.T) {
 // testEnv contains the test environment (set of servers) used by one
 // or more nodes.
 type testEnv struct {
-	t      testing.TB
-	cli    string
-	daemon string
+	t       testing.TB
+	tunMode bool
+	cli     string
+	daemon  string
 
 	LogCatcher       *LogCatcher
 	LogCatcherServer *httptest.Server
@@ -899,12 +921,25 @@ func newTestNode(t *testing.T, env *testEnv) *testNode {
 		sockFile = filepath.Join(os.TempDir(), rands.HexString(8)+".sock")
 		t.Cleanup(func() { os.Remove(sockFile) })
 	}
-	return &testNode{
+	n := &testNode{
 		env:       env,
 		dir:       dir,
 		sockFile:  sockFile,
 		stateFile: filepath.Join(dir, "tailscale.state"),
 	}
+
+	// Look for a data race. Once we see the start marker, start logging the rest.
+	var sawRace bool
+	n.addLogLineHook(func(line []byte) {
+		if mem.Contains(mem.B(line), mem.S("WARNING: DATA RACE")) {
+			sawRace = true
+		}
+		if sawRace {
+			t.Logf("%s", line)
+		}
+	})
+
+	return n
 }
 
 func (n *testNode) diskPrefs() *ipn.Prefs {
@@ -963,7 +998,7 @@ func (n *testNode) socks5AddrChan() <-chan string {
 		if i == -1 {
 			return
 		}
-		addr := string(line)[i+len(sub):]
+		addr := strings.TrimSpace(string(line)[i+len(sub):])
 		select {
 		case ch <- addr:
 		default:
@@ -1010,11 +1045,10 @@ func (op *nodeOutputParser) parseLines() {
 		}
 		line := buf[:nl+1]
 		buf = buf[nl+1:]
-		lineTrim := bytes.TrimSpace(line)
 
 		n.mu.Lock()
 		for _, f := range n.onLogLine {
-			f(lineTrim)
+			f(line)
 		}
 		n.mu.Unlock()
 	}
@@ -1048,8 +1082,8 @@ func (n *testNode) StartDaemon() *Daemon {
 
 func (n *testNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon {
 	t := n.env.t
-	cmd := exec.Command(n.env.daemon,
-		"--tun=userspace-networking",
+	cmd := exec.Command(n.env.daemon)
+	cmd.Args = append(cmd.Args,
 		"--state="+n.stateFile,
 		"--socket="+n.sockFile,
 		"--socks5-server=localhost:0",
@@ -1057,6 +1091,11 @@ func (n *testNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon {
 	if *verboseTailscaled {
 		cmd.Args = append(cmd.Args, "-verbose=2")
 	}
+	if !n.env.tunMode {
+		cmd.Args = append(cmd.Args,
+			"--tun=userspace-networking",
+		)
+	}
 	cmd.Env = append(os.Environ(),
 		"TS_DEBUG_PERMIT_HTTP_C2N=1",
 		"TS_LOG_TARGET="+n.env.LogCatcherServer.URL,
@@ -1067,10 +1106,7 @@ func (n *testNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon {
 		"TS_NETCHECK_GENERATE_204_URL="+n.env.ControlServer.URL+"/generate_204",
 	)
 	if version.IsRace() {
-		const knownBroken = true // TODO(bradfitz,maisem): enable this once we fix all the races :(
-		if !knownBroken {
-			cmd.Env = append(cmd.Env, "GORACE=halt_on_error=1")
-		}
+		cmd.Env = append(cmd.Env, "GORACE=halt_on_error=1")
 	}
 	cmd.Stderr = &nodeOutputParser{n: n}
 	if *verboseTailscaled {
@@ -1143,11 +1179,10 @@ func (n *testNode) AwaitListening() {
 	s := safesocket.DefaultConnectionStrategy(n.sockFile)
 	if err := tstest.WaitFor(20*time.Second, func() (err error) {
 		c, err := safesocket.Connect(s)
-		if err != nil {
-			return err
+		if err == nil {
+			c.Close()
 		}
-		c.Close()
-		return nil
+		return err
 	}); err != nil {
 		t.Fatal(err)
 	}
@@ -1241,7 +1276,8 @@ func (n *testNode) AwaitNeedsLogin() {
 // Tailscale returns a command that runs the tailscale CLI with the provided arguments.
 // It does not start the process.
 func (n *testNode) Tailscale(arg ...string) *exec.Cmd {
-	cmd := exec.Command(n.env.cli, "--socket="+n.sockFile)
+	cmd := exec.Command(n.env.cli)
+	cmd.Args = append(cmd.Args, "--socket="+n.sockFile)
 	cmd.Args = append(cmd.Args, arg...)
 	cmd.Dir = n.dir
 	cmd.Env = append(os.Environ(),

wgengine/magicsock/endpoint.go

@@ -484,9 +484,9 @@ func (p *pingResultAndCallback) reply() bool {
 	return p != nil && p.taken.CompareAndSwap(false, true)
 }
 
-// discoPing starts a disc-level ping for the "tailscale ping" command (or other
+// discoPing starts a disco-level ping for the "tailscale ping" command (or other
 // callers, such as c2n). res is value to call cb with, already partially
-// filled. cb must be called at most once. Once called, ownership of res passes to db.
+// filled. cb must be called at most once. Once called, ownership of res passes to cb.
 func (de *endpoint) discoPing(res *ipnstate.PingResult, size int, cb func(*ipnstate.PingResult)) {
 	de.mu.Lock()
 	defer de.mu.Unlock()