From 9227e822c258084729fb98d7c61e88a629c6651e Mon Sep 17 00:00:00 2001 From: Ismael Luceno Date: Sat, 15 Aug 2026 04:00:00 +0100 Subject: [PATCH 2/5] Report container setup failures instead of discarding them Three places turned a diagnosable failure into a bare "exit status 1": - EnsureFakeRoot() left Stdout/Stderr unset on the detached child, sending everything the setup wrote to /dev/null, and returned success even when the child had already died. - RunDetached()'s watchdog called os.Exit(0) once the child was gone, so a container that failed to start was indistinguishable from one that ran successfully. Close the log pipes instead and let cmd.Wait() report the real status, along with the tail of the container log. - GetSubIDRanges() propagated getsubids' exit status verbatim and then sliced its output unconditionally. Report why it failed, include its stderr, and say how to add the missing subordinate ID ranges. Upstream-Status: Submitted [https://github.com/89luca89/lilipod/pull/50] Signed-off-by: Ismael Luceno --- pkg/procutils/proc_utils.go | 168 +++++++++++++++++++++++++++++++++--- 1 file changed, 155 insertions(+), 13 deletions(-) diff --git a/pkg/procutils/proc_utils.go b/pkg/procutils/proc_utils.go index 1236709fef6c..3075b8cb58b0 100644 --- a/pkg/procutils/proc_utils.go +++ b/pkg/procutils/proc_utils.go @@ -3,6 +3,7 @@ package procutils import ( "bufio" + "errors" "fmt" "io" "os" @@ -80,6 +81,19 @@ func EnsureFakeRoot(interactive bool) (bool, error) { Setsid: true, } + // Without this the detached child's output goes to /dev/null, so the + // reason a container failed to set up is lost and the caller only ever + // sees "exit status 1". + startLog, logErr := os.Create(GetStartLogPath()) + if logErr != nil { + logging.LogDebug("could not create start log: %v", logErr) + } else { + defer startLog.Close() + + cmd.Stdout = startLog + cmd.Stderr = startLog + } + logging.LogDebug("tty not specified, using cmd.Start") err := cmd.Start() @@ -92,6 +106,18 @@ func EnsureFakeRoot(interactive bool) (bool, error) { logging.LogDebug("tty not specified, waiting for child to start") time.Sleep(time.Millisecond * 250) + // A child that is already gone died during setup. Report what it wrote + // instead of returning success and leaving a stopped container behind. + if !IsPidRunning(cmd.Process.Pid) { + _ = cmd.Wait() + + if reason := strings.TrimSpace(ReadStartLog()); reason != "" { + return false, errors.New("container setup failed:\n" + reason) + } + + return false, fmt.Errorf("container setup failed, see %s", GetStartLogPath()) + } + logging.LogDebug("tty not specified, releasing child") err = cmd.Process.Release() @@ -104,6 +130,37 @@ func EnsureFakeRoot(interactive bool) (bool, error) { return true, nil } +// GetStartLogPath returns the file a detached container start writes its +// output to, so setup failures can be reported instead of discarded. +func GetStartLogPath() string { + dir := os.Getenv("XDG_DATA_HOME") + if dir == "" { + if home, err := os.UserHomeDir(); err == nil { + dir = filepath.Join(home, ".local", "share") + } + } + + if dir == "" { + dir = os.TempDir() + } else { + dir = filepath.Join(dir, "lilipod") + } + + _ = os.MkdirAll(dir, 0o755) + + return filepath.Join(dir, "last-start.log") +} + +// ReadStartLog returns the contents of the last detached start log. +func ReadStartLog() string { + out, err := os.ReadFile(GetStartLogPath()) + if err != nil { + return "" + } + + return string(out) +} + // GetUIDGID will return a couple of uid/gid integers for input user. // Input user can be in the form of username:group, or uid:gid or a mix of that. func GetUIDGID(username string) (int, int) { @@ -178,6 +235,53 @@ func GetUIDGID(username string) (int, int) { return 0, 0 } +// subIDHelp is the actionable part of the error shown when a user has no +// subordinate ID ranges. Rootless containers cannot work without them. +const subIDHelp = "rootless containers need subordinate UID/GID ranges for user %q.\n" + + "Add them with:\n" + + " sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 %s\n" + + "or create /etc/subuid and /etc/subgid containing:\n" + + " %s:100000:65536" + +// runGetSubIDs runs getsubids and returns the first range found for username. +// Any failure is reported with the reason and how to fix it, because a bare +// exit status here is indistinguishable from every other startup failure. +func runGetSubIDs(username string, args ...string) ([]string, error) { + out, err := exec.Command("getsubids", args...).Output() + if err != nil { + var execErr *exec.Error + if errors.As(err, &execErr) { + return nil, fmt.Errorf( + "cannot run getsubids: %w\n"+ + "It is part of shadow-utils built with subordinate ID support, "+ + "and is required to set up a rootless container", + err) + } + + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + reason := strings.TrimSpace(string(exitErr.Stderr)) + if reason == "" { + reason = "no ranges configured" + } + + return nil, fmt.Errorf("getsubids %s failed: %s\n"+subIDHelp, + strings.Join(args, " "), reason, username, username, username) + } + + return nil, err + } + + // expected output: "0: " + fields := strings.Fields(string(out)) + if len(fields) < 4 { + return nil, fmt.Errorf("unexpected getsubids output %q\n"+subIDHelp, + string(out), username, username, username) + } + + return fields[2:4], nil +} + // GetSubIDRanges will return a slice of subUIDs and subGIDs for // running user. // This function will use the "getsubids" program to discover them. @@ -189,28 +293,20 @@ func GetSubIDRanges() ([]string, []string, error) { return nil, nil, err } - subUIDout, err := exec.Command("getsubids", user.Username).Output() + subUIDSlice, err := runGetSubIDs(user.Username, user.Username) if err != nil { logging.LogError("%v", err) return nil, nil, err } - subUIDSlice := strings.Split( - strings.Trim(string(subUIDout), "\n"), - " ")[2:] - - subGIDout, err := exec.Command("getsubids", "-g", user.Username).Output() + subGIDSlice, err := runGetSubIDs(user.Username, "-g", user.Username) if err != nil { logging.LogError("%v", err) return nil, nil, err } - subGIDSlice := strings.Split( - strings.Trim(string(subGIDout), "\n"), - " ")[2:] - subUIDSlice = append([]string{user.Uid}, subUIDSlice...) subGIDSlice = append([]string{user.Gid}, subGIDSlice...) @@ -353,6 +449,35 @@ func RunInteractive(cmd *exec.Cmd) error { return cmd.Wait() } +// tailLog returns the last max lines of a container log file, with the +// ":out:"/":err:" prefixes stripped, so a failure can be +// reported with the container's own output instead of just an exit status. +func tailLog(logfile string, max int) string { + content, err := os.ReadFile(logfile) + if err != nil { + return "" + } + + lines := []string{} + + for _, line := range strings.Split(strings.TrimRight(string(content), "\n"), "\n") { + if parts := strings.SplitN(line, ":", 3); len(parts) == 3 && + (parts[1] == "out" || parts[1] == "err") { + line = parts[2] + } + + if strings.TrimSpace(line) != "" { + lines = append(lines, line) + } + } + + if len(lines) > max { + lines = lines[len(lines)-max:] + } + + return strings.Join(lines, "\n") +} + // RunDetached will run input cmd and redurect all outputs to logfile. // No stdin is set up. func RunDetached(cmd *exec.Cmd, logfile string) error { @@ -435,7 +560,9 @@ func RunDetached(cmd *exec.Cmd, logfile string) error { } }() - // keep an eye on the child process, and exit if dead + // Keep an eye on the child process: when it is gone, close the log pipes + // so the readers below can finish. Exiting 0 here instead would discard + // the child's exit status and make a failed start look like a success. go func() { defer wg.Done() @@ -443,7 +570,13 @@ func RunDetached(cmd *exec.Cmd, logfile string) error { time.Sleep(time.Second * 5) if !IsPidRunning(cmd.Process.Pid) { - os.Exit(0) + // let the output copiers drain before tearing the pipes down + time.Sleep(time.Second) + + _ = outW.Close() + _ = errW.Close() + + return } } }() @@ -457,5 +590,14 @@ func RunDetached(cmd *exec.Cmd, logfile string) error { wg.Wait() - return cmd.Wait() + err = cmd.Wait() + if err != nil { + if out := tailLog(logfile, 20); out != "" { + return fmt.Errorf("container exited: %w\n%s\n(full log: %s)", err, out, logfile) + } + + return fmt.Errorf("container exited: %w (output logged to %s)", err, logfile) + } + + return nil }