From a28d280b95db23da1f3554d3fa2bd1a6e7a143d2 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Wed, 8 Dec 2021 15:13:39 -0800
Subject: [PATCH] cmd/tailscaled: move start-up failure logging to one place

The caller of func run said:

    // No need to log; the func already did

But that wasn't true. Some return paths didn't log.

So instead, return rich errors and have func main do the logging,
so we can't miss anything in the future.

Prior to this, safesocket.Listen for instance was causing tailscaled
to os.Exit(1) on failure without any clue as to why.

Change-Id: I9d71cc4d73d0fed4aa1b1902cae199f584f25793
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 cmd/tailscaled/tailscaled.go | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go
index ee7792303..974a0ed88 100644
--- a/cmd/tailscaled/tailscaled.go
+++ b/cmd/tailscaled/tailscaled.go
@@ -178,8 +178,7 @@ func main() {
 	osshare.SetFileSharingEnabled(false, logger.Discard)
 
 	if err != nil {
-		// No need to log; the func already did
-		os.Exit(1)
+		log.Fatal(err)
 	}
 }
 
@@ -301,7 +300,7 @@ func run() error {
 
 	linkMon, err := monitor.New(logf)
 	if err != nil {
-		log.Fatalf("creating link monitor: %v", err)
+		return fmt.Errorf("monitor.New: %w", err)
 	}
 	pol.Logtail.SetLinkMonitor(linkMon)
 
@@ -310,8 +309,7 @@ func run() error {
 	dialer := new(tsdial.Dialer) // mutated below (before used)
 	e, useNetstack, err := createEngine(logf, linkMon, dialer)
 	if err != nil {
-		logf("wgengine.New: %v", err)
-		return err
+		return fmt.Errorf("createEngine: %w", err)
 	}
 	if _, ok := e.(wgengine.ResolvingEngine).GetResolver(); !ok {
 		panic("internal error: exit node resolver not wired up")
@@ -324,7 +322,7 @@ func run() error {
 	ns.ProcessLocalIPs = useNetstack
 	ns.ProcessSubnets = useNetstack || wrapNetstack
 	if err := ns.Start(); err != nil {
-		log.Fatalf("failed to start netstack: %v", err)
+		return fmt.Errorf("failed to start netstack: %w", err)
 	}
 
 	if useNetstack {
@@ -380,13 +378,11 @@ func run() error {
 
 	store, err := ipnserver.StateStore(statePathOrDefault(), logf)
 	if err != nil {
-		logf("ipnserver.StateStore: %v", err)
-		return err
+		return fmt.Errorf("ipnserver.StateStore: %w", err)
 	}
 	srv, err := ipnserver.New(logf, pol.PublicID.String(), store, e, dialer, nil, opts)
 	if err != nil {
-		logf("ipnserver.New: %v", err)
-		return err
+		return fmt.Errorf("ipnserver.New: %w", err)
 	}
 
 	if debugMux != nil {
@@ -401,8 +397,7 @@ func run() error {
 	err = srv.Run(ctx, ln)
 	// Cancelation is not an error: it is the only way to stop ipnserver.
 	if err != nil && err != context.Canceled {
-		logf("ipnserver.Run: %v", err)
-		return err
+		return fmt.Errorf("ipnserver.Run: %w", err)
 	}
 
 	return nil