From 4482b27837f85c4d7efd5c25da4890e5dd0590a9 Mon Sep 17 00:00:00 2001 From: Val Date: Wed, 28 Jun 2023 21:57:53 +0200 Subject: [PATCH] logpolicy,ipn,cmd/tailscaled: make log files unique Proof of concept to generate unique log files and store them in the client's persistent config so that multiple clients can be run on the same machine without stomping on each other's log files. This uses os.MkdirTemp() to generate uniquely named subdirectories within the existing suite of potential log directories. Fixes #8476 Signed-off-by: Val --- cmd/tailscaled/tailscaled.go | 43 +++++++++++++++++++++++++++--------- ipn/ipnlocal/local.go | 8 +++++-- ipn/store.go | 3 +++ logpolicy/logpolicy.go | 31 +++++++++++++++++--------- 4 files changed, 62 insertions(+), 23 deletions(-) diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 95193f1e7..802d1a139 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -32,6 +32,7 @@ import ( "tailscale.com/cmd/tailscaled/childproc" "tailscale.com/control/controlclient" "tailscale.com/envknob" + "tailscale.com/ipn" "tailscale.com/ipn/ipnlocal" "tailscale.com/ipn/ipnserver" "tailscale.com/ipn/store" @@ -342,7 +343,38 @@ func run() error { } sys.Set(netMon) - pol := logpolicy.New(logtail.CollectionNode, netMon) + // Load the config from state before starting the logger + if args.statepath == "" && args.statedir == "" { + log.Fatalf("--statedir (or at least --state) is required") + } + // We have some in-memory thing that remembers where the + // statedir is. We should use it to read the name of the log + // file out of the state store. If it doesn't exist, we should + // make it. + + // Pull the path out here and use newwithconfig + store, err := store.New(logf, statePathOrDefault()) + if err != nil { + return fmt.Errorf("store.New: %w", err) + } + sys.Set(store) + + // At this point we have read the state off of the disk and + // can look up the log file directory. + + logDir := "" + if v, err := store.ReadState(ipn.LogDirStateKey); err == nil { + logDir = string(v) + } + // XXX ignore other errors? + if logDir == "" { + // Then we need to set the log dir and store it to the config + logDir = logpolicy.LogsDir(logf) + if err := store.WriteState(ipn.LogDirStateKey, []byte(logDir)); err != nil { + return fmt.Errorf("store.New: %w", err) + } + } + pol := logpolicy.NewWithConfigPath(logtail.CollectionNode, logDir, "", netMon) pol.SetVerbosityLevel(args.verbose) logPol = pol defer func() { @@ -380,9 +412,6 @@ func run() error { return nil } - if args.statepath == "" && args.statedir == "" { - log.Fatalf("--statedir (or at least --state) is required") - } if err := trySynologyMigration(statePathOrDefault()); err != nil { log.Printf("error in synology migration: %v", err) } @@ -524,12 +553,6 @@ func getLocalBackend(ctx context.Context, logf logger.Logf, logID logid.PublicID opts := ipnServerOpts() - store, err := store.New(logf, statePathOrDefault()) - if err != nil { - return nil, fmt.Errorf("store.New: %w", err) - } - sys.Set(store) - lb, err := ipnlocal.NewLocalBackend(logf, logID, sys, opts.LoginFlags) if err != nil { return nil, fmt.Errorf("ipnlocal.NewLocalBackend: %w", err) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 762dfba9b..abafb07bd 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -45,7 +45,6 @@ import ( "tailscale.com/ipn/ipnstate" "tailscale.com/ipn/policy" "tailscale.com/log/sockstatlog" - "tailscale.com/logpolicy" "tailscale.com/net/dns" "tailscale.com/net/dnscache" "tailscale.com/net/dnsfallback" @@ -314,7 +313,12 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo } netMon := sys.NetMon.Get() - b.sockstatLogger, err = sockstatlog.NewLogger(logpolicy.LogsDir(logf), logf, logID, netMon) + // Get the stored logdir here if it exists + logDir := "" + if v, err := store.ReadState(ipn.LogDirStateKey); err == nil { + logDir = string(v) + } + b.sockstatLogger, err = sockstatlog.NewLogger(logDir, logf, logID, netMon) if err != nil { log.Printf("error setting up sockstat logger: %v", err) } diff --git a/ipn/store.go b/ipn/store.go index ee5a238a7..b9b9a904f 100644 --- a/ipn/store.go +++ b/ipn/store.go @@ -52,6 +52,9 @@ const ( // CurrentProfileStateKey is the key under which we store the current // profile. CurrentProfileStateKey = StateKey("_current-profile") + + // Log directory is the path of the directory where we store logs. + LogDirStateKey = StateKey("logdir") ) // CurrentProfileID returns the StateKey that stores the diff --git a/logpolicy/logpolicy.go b/logpolicy/logpolicy.go index 7becc934f..9bc915b3e 100644 --- a/logpolicy/logpolicy.go +++ b/logpolicy/logpolicy.go @@ -216,9 +216,9 @@ func LogsDir(logf logger.Logf) string { // as a regular user (perhaps in userspace-networking/SOCK5 mode) and we should // just use the %LocalAppData% instead. In a user context, %LocalAppData% isn't // subject to random deletions from Windows system updates. - dir := filepath.Join(os.Getenv("ProgramData"), "Tailscale") - if winProgramDataAccessible(dir) { - logf("logpolicy: using dir %v", dir) + d := filepath.Join(os.Getenv("ProgramData"), "Tailscale") + if dir, err := os.MkdirTemp(d, "tailscale-log-*"); err == nil { + logf("logpolicy: using directory in Program Data dir %v", dir) return dir } } @@ -230,8 +230,11 @@ func LogsDir(logf logger.Logf) string { // systems-d. For example, Ubuntu 18.04 (Bionic Beaver) is 237. systemdStateDir := os.Getenv("STATE_DIRECTORY") if systemdStateDir != "" { - logf("logpolicy: using $STATE_DIRECTORY, %q", systemdStateDir) - return systemdStateDir + dir, err := os.MkdirTemp(systemdStateDir, "tailscale-log-*") + if err == nil { + logf("logpolicy: using directory in $STATE_DIRECTORY, %q", dir) + return dir + } } } @@ -239,24 +242,30 @@ func LogsDir(logf logger.Logf) string { if d := paths.DefaultTailscaledStateFile(); d != "" { d = filepath.Dir(d) // directory of e.g. "/var/lib/tailscale/tailscaled.state" if err := os.MkdirAll(d, 0700); err == nil { - logf("logpolicy: using system state directory %q", d) - return d + if dir, err := os.MkdirTemp(d, "tailscale-log-*"); err == nil { + logf("logpolicy: using directory in system state directory %q", dir) + return dir + } } } cacheDir, err := os.UserCacheDir() if err == nil { d := filepath.Join(cacheDir, "Tailscale") - logf("logpolicy: using UserCacheDir, %q", d) - return d + if dir, err := os.MkdirTemp(d, "tailscale-log-*"); err == nil { + logf("logpolicy: using directory in UserCacheDir, %q", dir) + return dir + } } // Use the current working directory, unless we're being run by a // service manager that sets it to /. wd, err := os.Getwd() if err == nil && wd != "/" { - logf("logpolicy: using current directory, %q", wd) - return wd + if dir, err := os.MkdirTemp(wd, "tailscale-log-*"); err == nil { + logf("logpolicy: using directory in current directory, %q", dir) + return dir + } } // No idea where to put stuff. Try to create a temp dir. It'll