diff --git a/internal/processmanager/README.md b/internal/processmanager/README.md index 9b5e59e..feac8c9 100644 --- a/internal/processmanager/README.md +++ b/internal/processmanager/README.md @@ -45,6 +45,41 @@ unreachable container runtime — is reported as undetermined rather than as a stopped server. The loop leaves such a server alone: treating an unreadable probe as "down" would restart a healthy server. +## How the start command's program is found + +A start command names its program relative to the directory the game server +process runs in — `./srcds_run` on Linux, `srcds.exe` on Windows. That is the +server directory joined with the configured `work_dir` (see the root README), +and the server directory itself when no `work_dir` is set. The supervisor that +launches the command does not run from there. systemd expands `ExecStart=` before it +applies `WorkingDirectory=`, and Windows resolves a program that contains a path +separator against the directory of the process that asked for the start, never +against the one the service is given. A command written as `.\server.exe` or +`bin\srcds.exe` therefore names a path that does not exist, and the launch fails +with nothing but the supervisor's own "file not found" to explain it. + +`systemd`, `shawl` and `winsw` all resolve the program before the command is +written into a unit or registered as a service. That process working directory is +searched first, PATH second, so `powershell` or `java` stays reachable while a +binary shipped with the server always wins over a same-named one elsewhere on the +host. What is registered is the absolute path whenever the program was found, +which means the same file for every supervisor. + +The daemon's own working directory is not searched at all, and only absolute PATH +entries are searched. Windows looks in the calling process's directory before +PATH, which would let a file dropped next to the daemon binary stand in for the +interpreter a game server asked for. + +The two Windows managers keep an unresolved command as it stands instead of +refusing to register the service: shawl searches its own `--cwd` for a name that +carries no path separator, and a game server whose files are downloaded on the +first start would otherwise never get to run. `systemd` fails instead, because a +unit is written once and a wrong `ExecStart=` would keep failing silently. + +Because the registered command line changes from a relative path to an absolute +one, Windows services registered by an older daemon differ from the ones the +config now describes and are registered again on the next start. + ## Metrics support The `Metrics(ctx, server)` method returns Prometheus-style samples (see @@ -167,6 +202,12 @@ service control manager accepts the request, and tails the shawl log when a serv immediately. Because shawl restarts the game process itself, a running service proves the supervisor came up, not that the game stayed up. +All shawl writes about a program it could not launch is `program not found`, which names +neither the file it wanted nor the directories it searched. When a service stops immediately +and the start command's program is in neither the working directory nor PATH, the daemon says +so before the log tail, naming both — the difference between an archive that unpacked into a +subdirectory and a game that crashed on startup, which are fixed in entirely different places. + Metrics are liveness-only; see the table above. ### Changing the restart policy @@ -239,6 +280,13 @@ with that relative path, e.g. `/server/GroundBranch/Binaries/Linux`. In the star command `{dir}` expands to `docker_workdir` and `{work_dir}` to that container working directory, not to the host paths. +A configured home directory (`home_dir`, `home_dir_linux`, `home_dir_windows`, +`home_dir_macos`; see the root README) is resolved the same way: `HOME` is set to +`docker_workdir` joined with that relative path, so a `home_dir` of `.` gives +`HOME=/server`. During installation the server directory is mounted at +`/mnt/server` instead, and `HOME` follows it there. Without a configured +`home_dir` the container keeps whatever `HOME` its image sets. + #### Installation Configuration | Key | Description | Example | Default | @@ -442,7 +490,9 @@ Podman uses the same metadata keys as Docker for compatibility: As with Docker, the server directory is mounted at `docker_workdir`, which is the container working directory by default; a configured process work directory (`work_dir*` keys, see the root README) is joined onto it. `{dir}` and `{work_dir}` -in the start command expand to those container paths. +in the start command expand to those container paths. A configured `home_dir*` +is joined onto `docker_workdir` as well and exported as `HOME`, and installation +uses the `/mnt/server` mount for both. ### Socket Configuration diff --git a/internal/processmanager/executable.go b/internal/processmanager/executable.go new file mode 100644 index 0000000..8cd3be0 --- /dev/null +++ b/internal/processmanager/executable.go @@ -0,0 +1,97 @@ +package processmanager + +import ( + "os" + "os/exec" + "path/filepath" + + "github.com/pkg/errors" +) + +// resolveCommandExecutable returns the absolute path of the program a start command begins +// with, looking for it in the directory the game server process runs in and then in PATH. +// +// Supervisors do not resolve a relative program against the working directory they are told to +// use. systemd expands ExecStart= before it applies WorkingDirectory=, and Windows resolves a +// program that carries a path separator against the directory of the process that requested the +// start, never against the one the service is given. A start command spelled `.\server.exe` or +// `bin\srcds.exe` — the spelling every Linux entry in the games catalogue uses — therefore names +// a path that does not exist, and the game server fails to launch with nothing but the +// supervisor's own "file not found" to go on. An absolute path means the same file for every +// supervisor, whatever directory it happens to start from. +// +// A bare name that the server directory does not hold falls back to PATH, so an interpreter +// such as powershell or java stays reachable. +func resolveCommandExecutable(cmd, processWorkDir string) (string, error) { + if filepath.IsAbs(cmd) { + path, err := lookPathAbs(cmd) + if err != nil { + return "", errors.WithMessagef(err, "failed to find command %q", cmd) + } + + return path, nil + } + + path, workDirErr := lookPathAbs(filepath.Join(processWorkDir, cmd)) + if workDirErr == nil { + return path, nil + } + + path, pathErr := lookPathInPATH(cmd) + if pathErr == nil { + return path, nil + } + + return "", errors.WithMessagef( + workDirErr, "failed to find command %q in %q and in PATH", cmd, processWorkDir, + ) +} + +// lookPathAbs is exec.LookPath with a result that is always absolute. +// +// A hit reported as exec.ErrDot is refused rather than used. LookPath returns that sentinel +// when the name resolved inside the calling process's own working directory, which on Windows +// is searched implicitly and before PATH. That directory holds the daemon binary and has +// nothing to do with the game server, so a file dropped next to the daemon must never become +// the program a service is registered with. +func lookPathAbs(name string) (string, error) { + path, err := exec.LookPath(name) + if err != nil { + return "", errors.Wrapf(err, "failed to look up %q", name) + } + + abs, err := filepath.Abs(path) + if err != nil { + return "", errors.Wrapf(err, "failed to make path %q absolute", path) + } + + return abs, nil +} + +// lookPathInPATH searches PATH alone for a command name. +// +// exec.LookPath cannot do this on Windows: it looks in the calling process's working directory +// first and returns that hit instead of going on to PATH, so refusing the hit afterwards would +// also lose the interpreter that PATH really does provide. Each PATH entry is therefore joined +// with the name and checked on its own, which keeps LookPath's PATHEXT handling while leaving +// the implicit search of the daemon's own directory out of it. +// +// Entries that are not absolute are skipped. The resolved path is written into a unit or a +// service that a supervisor starts from some other directory, where a path that only means +// something relative to the daemon's working directory would point somewhere else or nowhere. +func lookPathInPATH(cmd string) (string, error) { + for _, dir := range filepath.SplitList(os.Getenv("PATH")) { + if !filepath.IsAbs(dir) { + continue + } + + path, err := exec.LookPath(filepath.Join(dir, cmd)) + if err != nil { + continue + } + + return path, nil + } + + return "", errors.Errorf("failed to find %q in PATH", cmd) +} diff --git a/internal/processmanager/executable_test.go b/internal/processmanager/executable_test.go new file mode 100644 index 0000000..8687dd6 --- /dev/null +++ b/internal/processmanager/executable_test.go @@ -0,0 +1,176 @@ +package processmanager + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// executableName is the file name a start command would name on the running OS, together with +// the token a games catalogue entry spells it with. +func executableName() (fileName, bareToken string) { + if runtime.GOOS == "windows" { + return "server.exe", "server.exe" + } + + return "server", "server" +} + +func writeExecutable(t *testing.T, dir, name string) string { + t.Helper() + + path := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(path, []byte("#!/bin/sh\n"), 0o755)) + + return path +} + +// A command with a path separator is the case supervisors get wrong: they resolve it against +// their own working directory rather than the one the game server is given. +func TestResolveCommandExecutable_RelativePathWithSeparator(t *testing.T) { + workDir := t.TempDir() + subDir := filepath.Join(workDir, "bin") + require.NoError(t, os.Mkdir(subDir, 0o755)) + + fileName, _ := executableName() + want := writeExecutable(t, subDir, fileName) + + // "." is spelled out rather than joined: filepath.Join cleans it away, and the leading + // separator is the whole point of the case. + tests := map[string]struct { + cmd string + dir string + }{ + "dot_prefixed": {cmd: "." + string(filepath.Separator) + fileName, dir: subDir}, + "subdirectory": {cmd: filepath.Join("bin", fileName), dir: workDir}, + "parent_walked": {cmd: filepath.Join("..", "bin", fileName), dir: subDir}, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := resolveCommandExecutable(tt.cmd, tt.dir) + + require.NoError(t, err) + assert.Equal(t, want, got) + }) + } +} + +func TestResolveCommandExecutable_BareNameInWorkDir(t *testing.T) { + workDir := t.TempDir() + fileName, bareToken := executableName() + want := writeExecutable(t, workDir, fileName) + + got, err := resolveCommandExecutable(bareToken, workDir) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +func TestResolveCommandExecutable_AbsolutePath(t *testing.T) { + workDir := t.TempDir() + fileName, _ := executableName() + want := writeExecutable(t, workDir, fileName) + + got, err := resolveCommandExecutable(want, t.TempDir()) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// An interpreter the server directory does not hold has to stay reachable, or a start command +// such as `powershell -File run.ps1` would stop working. +func TestResolveCommandExecutable_FallsBackToPath(t *testing.T) { + binDir := t.TempDir() + fileName, bareToken := executableName() + want := writeExecutable(t, binDir, fileName) + + t.Setenv("PATH", binDir) + if runtime.GOOS == "windows" { + t.Setenv("PATHEXT", ".EXE") + } + + got, err := resolveCommandExecutable(bareToken, t.TempDir()) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// The server directory wins over PATH, so a server never runs a same-named binary from +// somewhere else on the host. +func TestResolveCommandExecutable_WorkDirWinsOverPath(t *testing.T) { + workDir := t.TempDir() + binDir := t.TempDir() + fileName, bareToken := executableName() + + want := writeExecutable(t, workDir, fileName) + writeExecutable(t, binDir, fileName) + + t.Setenv("PATH", binDir) + if runtime.GOOS == "windows" { + t.Setenv("PATHEXT", ".EXE") + } + + got, err := resolveCommandExecutable(bareToken, workDir) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// Windows searches the calling process's own working directory before PATH, so without an +// explicit PATH-only lookup a file sitting next to the daemon binary would be registered as the +// game server's interpreter instead of the real one. +func TestResolveCommandExecutable_DaemonDirDoesNotShadowPath(t *testing.T) { + daemonDir := t.TempDir() + binDir := t.TempDir() + fileName, bareToken := executableName() + + writeExecutable(t, daemonDir, fileName) + want := writeExecutable(t, binDir, fileName) + + t.Chdir(daemonDir) + t.Setenv("PATH", binDir) + if runtime.GOOS == "windows" { + t.Setenv("PATHEXT", ".EXE") + } + + got, err := resolveCommandExecutable(bareToken, t.TempDir()) + + require.NoError(t, err) + assert.Equal(t, want, got) +} + +// A command that exists only next to the daemon is not found at all: the daemon's working +// directory is not a place a game server's program may come from. +func TestResolveCommandExecutable_DaemonDirIsNotSearched(t *testing.T) { + daemonDir := t.TempDir() + fileName, bareToken := executableName() + + writeExecutable(t, daemonDir, fileName) + + t.Chdir(daemonDir) + t.Setenv("PATH", t.TempDir()) + if runtime.GOOS == "windows" { + t.Setenv("PATHEXT", ".EXE") + } + + _, err := resolveCommandExecutable(bareToken, t.TempDir()) + + require.Error(t, err) + assert.Contains(t, err.Error(), bareToken) +} + +func TestResolveCommandExecutable_NotFound(t *testing.T) { + workDir := t.TempDir() + t.Setenv("PATH", t.TempDir()) + + _, err := resolveCommandExecutable("definitely-not-here", workDir) + + require.Error(t, err) + assert.Contains(t, err.Error(), "definitely-not-here") + assert.Contains(t, err.Error(), workDir) +} diff --git a/internal/processmanager/shawl_windows.go b/internal/processmanager/shawl_windows.go index 2e8ed6e..1be3fb1 100644 --- a/internal/processmanager/shawl_windows.go +++ b/internal/processmanager/shawl_windows.go @@ -275,6 +275,20 @@ func (pm *Shawl) buildServicePlan(server *domain.Server) (shawlServicePlan, erro return shawlServicePlan{}, errors.WithMessage(err, "failed to resolve server process work directory") } + // Windows resolves a program that carries a path separator against the working directory of + // the process asking for the start, so the service control manager would look for a command + // written as `.\server.exe` next to the daemon rather than in the server directory. The + // command is anchored here, while the daemon still knows where that directory is. + // + // A command that cannot be found is registered as it stands rather than refused: shawl + // searches the working directory itself for a name that carries no separator, and a game + // server whose files arrive on the first start would otherwise never get to run. + if len(cmdArr) > 0 { + if executable, resolveErr := resolveCommandExecutable(cmdArr[0], processWorkDir); resolveErr == nil { + cmdArr[0] = executable + } + } + envVars, err := server.EnvironmentVars(pm.cfg) if err != nil { return shawlServicePlan{}, errors.WithMessage(err, "failed to build server environment") @@ -645,6 +659,11 @@ func (pm *Shawl) waitForServiceRunning(ctx context.Context, server *domain.Serve if err != nil { if errors.Is(err, ErrServiceStoppedOnStart) { _, _ = out.Write([]byte("Service " + serviceName + " stopped immediately after start\n")) + + if hint := pm.missingExecutableHint(server); hint != "" { + _, _ = out.Write([]byte(hint)) + } + pm.writeLogTail(out, server) } @@ -656,6 +675,33 @@ func (pm *Shawl) waitForServiceRunning(ctx context.Context, server *domain.Serve return nil } +// missingExecutableHint names the program a start command begins with when that program is +// nowhere the service could have found it, and returns an empty string otherwise. +// +// All shawl has to say about it is "program not found", which names neither the file it looked +// for nor the directories it looked in. That one line is the difference between a game whose +// archive unpacked into a subdirectory and a game that crashed on startup, and the two are +// fixed in entirely different places. +func (pm *Shawl) missingExecutableHint(server *domain.Server) string { + cmdArr, err := domain.BuildCommandArgs(pm.cfg, server, pm.cfg.Scripts.Start, server.StartCommand()) + if err != nil || len(cmdArr) == 0 { + return "" + } + + processWorkDir, err := server.ProcessWorkDir(pm.cfg) + if err != nil { + return "" + } + + if _, err := resolveCommandExecutable(cmdArr[0], processWorkDir); err == nil { + return "" + } + + return "The start command begins with " + quoteName(cmdArr[0]) + + ", which is neither in " + quoteName(processWorkDir) + " nor in PATH. " + + "Check that the game server files are installed and that the start command points at them.\n" +} + // seekLogTail positions f at the last shawlOutputSizeLimit bytes of the log. It reports whether // the file was long enough to be cut, in which case the read starts inside an entry whose // beginning is gone and the caller has to discard the fragment before the first newline. diff --git a/internal/processmanager/systemd.go b/internal/processmanager/systemd.go index 4c6b971..f419b7a 100644 --- a/internal/processmanager/systemd.go +++ b/internal/processmanager/systemd.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "os" - "os/exec" "os/user" "path/filepath" "sort" @@ -838,28 +837,16 @@ func (pm *SystemD) makeStartCommand(server *domain.Server) (string, error) { return "", ErrEmptyCommand } - cmd := args[0] - processWorkDir, err := server.ProcessWorkDir(pm.cfg) if err != nil { return "", errors.WithMessage(err, "failed to resolve server process work directory") } - var foundPath string - - if filepath.IsAbs(cmd) { - foundPath, err = exec.LookPath(cmd) - if err != nil { - return "", errors.WithMessagef(err, "failed to find command '%s'", cmd) - } - } else { - foundPath, err = exec.LookPath(filepath.Join(processWorkDir, cmd)) - if err != nil { - foundPath, err = exec.LookPath(cmd) - if err != nil { - return "", errors.WithMessagef(err, "failed to find command '%s'", cmd) - } - } + // systemd expands ExecStart= before it applies WorkingDirectory=, so the unit carries the + // absolute path rather than the relative one the games catalogue is written with. + foundPath, err := resolveCommandExecutable(args[0], processWorkDir) + if err != nil { + return "", err } args[0] = foundPath diff --git a/internal/processmanager/systemd_internal_test.go b/internal/processmanager/systemd_internal_test.go index 9c245da..76829d9 100644 --- a/internal/processmanager/systemd_internal_test.go +++ b/internal/processmanager/systemd_internal_test.go @@ -128,14 +128,16 @@ func Test_makeCommand(t *testing.T) { server: func() *domain.Server { return makeServerWithStartCommandAndDir("invalid", tempDir) }, - expectedError: `failed to find command 'invalid'`, + // The directory that was searched is part of the message: a command that is + // missing and a command that is somewhere else look identical without it. + expectedError: `failed to find command "invalid" in "` + tempDir + `" and in PATH`, }, { name: "error invalid global command", server: func() *domain.Server { return makeServerWithStartCommandAndDir("/usr/bin/invalid", tempDir) }, - expectedError: `failed to find command '/usr/bin/invalid'`, + expectedError: `failed to find command "/usr/bin/invalid"`, }, } diff --git a/internal/processmanager/winsw_windows.go b/internal/processmanager/winsw_windows.go index e0aabec..2b3f78d 100644 --- a/internal/processmanager/winsw_windows.go +++ b/internal/processmanager/winsw_windows.go @@ -395,6 +395,19 @@ func (pm *WinSW) buildServiceConfig(server *domain.Server) (string, error) { return "", ErrEmptyCommand } + processWorkDir, err := server.ProcessWorkDir(pm.cfg) + if err != nil { + return "", errors.WithMessage(err, "failed to resolve server process work directory") + } + + // WinSW starts the executable from its own directory, so a command written as `.\server.exe` + // has to be anchored to the server directory before it reaches the service configuration. + // One that cannot be found is left alone, so a game server whose files arrive on the first + // start is not blocked from running. + if executable, resolveErr := resolveCommandExecutable(cmdArr[0], processWorkDir); resolveErr == nil { + cmdArr[0] = executable + } + executable := cmdArr[0] argArr := make([]string, 0, len(cmdArr)+1) @@ -412,11 +425,6 @@ func (pm *WinSW) buildServiceConfig(server *domain.Server) (string, error) { arguments = shellquote.WindowsJoin(argArr...) } - processWorkDir, err := server.ProcessWorkDir(pm.cfg) - if err != nil { - return "", errors.WithMessage(err, "failed to resolve server process work directory") - } - serviceName := pm.serviceName(server) serviceConfig := WinSWServiceConfig{ ID: serviceName,