tailscale / tailscale/tailscale

Log file corruption when more than one instance of tailscale or tailscaled runs concurrently

Open
#8,476 0 comments 0 reactions 0 assignees View on GitHub
bug P1 Nuisance
Dominant language
Go
Stars
36.5k
Forks
3.2k
Avg merge
2d 3h
Merged PRs (30d)
123

Description

It's possible to run multiple tailscaled clients or tailscale commands on the same machine but if they are invoked in a similar manner, they will use the same log files as other clients, resulting in corrupting each other's logs.

The log directory is chosen based on a system-wide policy and the UID of the process, or the TS_LOGS_DIR variable. The log files are named with a prefix that is one of "tailscale", "tailscaled", or "sockstats". This leads to multiple binaries writing logs to the same file.

A few cases:

1. Common case: a single tailscaled process running in kernel mode. Users will expect the logs to be in the usual system log location.
2. Two kernel mode tailscaled processes. Each will be passed its own different state/statedir but currently they will open the same log files.
3. Two userspace-networking mode tailscaled processes. Again, will have different state/statedir but will open the same log files.

The tailscale command suffers from the same problem, though it is less common since it isn't a long-running process.

I propose that we add some unique element to the log file pathname and store it in the client configuration in state/statedir. The public logID is one option; however it is currently only stored in the log config file in the log dir.

Some details:

Log files are created/opened in in three different locations:

1. tsnet.StartLogger(): combines s.rootPath and calls logpolicy.NewConfig(), then logtail.NewLogger()
2. tailscaled.run() calls logger.New() which uses logpolicy.LogsDir() to construct the directory using the system-wide policy and the UID, then uses th
3. ipnlocal.NewLocalBackend calls logpolicy.LogsDir() and passes that directory and the public logID of the caller to sockstatlog.NewLogger(), which constructs the filename using the dir and "sockstats", then calls logtail.NewLogger() with the constructed logtail.Config.

A few difficulties:

Loading the configuration vs. starting the logger

Since the log location currently does not depend on any configuration and the logger needs to start as early as possible, in case 2 the logger starts before the state file(s) are loaded. Storing the log file location in the state file would require moving the configuration load to before the log is started.

Using the public log ID as part of the log file pathname

The public log ID is unique and persistent along with the log files, so it makes an attractive unique pathname element. However it is currently stored in the log config... which is stored in the log directory. To use it, we would need to do the following steps:

1. Load the state file and look for the log dir in it
2. If no log dir, allocate the public ID via NewConfig()
3. Construct the log directory path using the public ID
4. Pass the log directory path to NewLoggerWithConfig()
5. Store the log dir path in the state file

This is all fine to do, it just doesn't fit in the existing public interfaces.

Moving existing log files

We already have code for moving log files or their configurations from old locations to new ones in at least two parts of the code. Instead of adding a third set of log file moving code, maybe we should combine it all together. The existing locations are:

logpolicy.tryFixLogStateLocation()
logpolicy.NewWithConfigPath()

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.