Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use exit-dir, get exit status from perist_dir/ctr-id/exit #23108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions libpod/container_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,18 +885,18 @@ func (c *Container) execAttachSocketPath(sessionID string) (string, error) {
return c.ociRuntime.ExecAttachSocketPath(c, sessionID)
}

// execExitFileDir gets the path to the container's exit file
func (c *Container) execExitFileDir(sessionID string) string {
return filepath.Join(c.execBundlePath(sessionID), "exit")
}

// execPersistDir gets the path to the container's persist directory
// The persist directory container the exit file and oom file (if oomkilled)
// of a container
func (c *Container) execPersistDir(sessionID string) string {
return filepath.Join(c.execBundlePath(sessionID), "persist", c.ID())
}

// execExitFileDir gets the path to the container's exit file
func (c *Container) execExitFileDir(sessionID string) string {
return filepath.Join(c.execPersistDir(sessionID), "exit")
}

// execOCILog returns the file path for the exec sessions oci log
func (c *Container) execOCILog(sessionID string) string {
if !c.ociRuntime.SupportsJSONErrors() {
Expand All @@ -918,12 +918,6 @@ func (c *Container) createExecBundle(sessionID string) (retErr error) {
}
}
}()
if err := os.MkdirAll(c.execExitFileDir(sessionID), execDirPermission); err != nil {
// The directory is allowed to exist
if !os.IsExist(err) {
return fmt.Errorf("creating OCI runtime exit file path %s: %w", c.execExitFileDir(sessionID), err)
}
}
if err := os.MkdirAll(c.execPersistDir(sessionID), execDirPermission); err != nil {
return fmt.Errorf("creating OCI runtime persist directory path %s: %w", c.execPersistDir(sessionID), err)
}
Expand Down
21 changes: 7 additions & 14 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"slices"
Expand Down Expand Up @@ -153,6 +152,10 @@ func (c *Container) oomFilePath() (string, error) {
return c.ociRuntime.OOMFilePath(c)
}

func (c *Container) persistDir() (string, error) {
return c.ociRuntime.PersistDir(c)
}

// Wait for the container's exit file to appear.
// When it does, update our state based on it.
func (c *Container) waitForExitFileAndSync() error {
Expand Down Expand Up @@ -757,22 +760,12 @@ func (c *Container) removeConmonFiles() error {
return fmt.Errorf("removing container %s winsz file: %w", c.ID(), err)
}

// Remove the exit file so we don't leak memory in tmpfs
exitFile, err := c.exitFilePath()
if err != nil {
return err
}
if err := os.Remove(exitFile); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("removing container %s exit file: %w", c.ID(), err)
}

// Remove the oom file
oomFile, err := c.oomFilePath()
persistDir, err := c.persistDir()
if err != nil {
return err
}
if err := os.Remove(oomFile); err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("removing container %s oom file: %w", c.ID(), err)
if err := os.RemoveAll(persistDir); err != nil {
return fmt.Errorf("removing container %s perist dir with exit & oom files %q: %w", c.ID(), persistDir, err)
}

return nil
Expand Down
4 changes: 4 additions & 0 deletions libpod/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ type OCIRuntime interface { //nolint:interfacebloat
// exec session in the given container.
// TODO: Probably should be made internal.
ExecAttachSocketPath(ctr *Container, sessionID string) (string, error)

// PersistDir is the path to a container's dir for oom & exit files.
PersistDir(ctr *Container) (string, error)

// ExitFilePath is the path to a container's exit file.
// All runtime implementations must create an exit file when containers
// exit, containing the exit code of the container (as a string).
Expand Down
37 changes: 23 additions & 14 deletions libpod/oci_conmon_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ type ConmonOCIRuntime struct {
conmonPath string
conmonEnv []string
tmpDir string
exitsDir string
logSizeMax int64
noPivot bool
reservePorts bool
Expand Down Expand Up @@ -143,14 +142,9 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
return nil, fmt.Errorf("no valid executable found for OCI runtime %s: %w", name, define.ErrInvalidArg)
}

runtime.exitsDir = filepath.Join(runtime.tmpDir, "exits")
// The persist-dir is where conmon writes the exit file and oom file (if oom killed), we join the container ID to this path later on
runtime.persistDir = filepath.Join(runtime.tmpDir, "persist")

// Create the exit files and attach sockets directories
if err := os.MkdirAll(runtime.exitsDir, 0750); err != nil {
return nil, fmt.Errorf("creating OCI runtime exit files directory: %w", err)
}
if err := os.MkdirAll(runtime.persistDir, 0750); err != nil {
return nil, fmt.Errorf("creating OCI runtime persist directory: %w", err)
}
Expand Down Expand Up @@ -926,18 +920,34 @@ func (r *ConmonOCIRuntime) AttachSocketPath(ctr *Container) (string, error) {
return filepath.Join(ctr.bundlePath(), "attach"), nil
}

// PersistDir returns the persit dir containing oom & exit files for containers.
func (r *ConmonOCIRuntime) PersistDir(ctr *Container) (string, error) {
if ctr == nil {
return "", fmt.Errorf("must provide a valid container to get persist dir: %w", define.ErrInvalidArg)
}

return filepath.Join(r.persistDir, ctr.ID()), nil
}

// ExitFilePath is the path to a container's exit file.
func (r *ConmonOCIRuntime) ExitFilePath(ctr *Container) (string, error) {
if ctr == nil {
return "", fmt.Errorf("must provide a valid container to get exit file path: %w", define.ErrInvalidArg)
persistDir, err := r.PersistDir(ctr)
if err != nil {
return "", err
}
return filepath.Join(r.exitsDir, ctr.ID()), nil

return filepath.Join(persistDir, "exit"), nil
}

// OOMFilePath is the path to a container's oom file.
// The oom file will only exist if the container was oom killed.
func (r *ConmonOCIRuntime) OOMFilePath(ctr *Container) (string, error) {
return filepath.Join(r.persistDir, ctr.ID(), "oom"), nil
persistDir, err := r.PersistDir(ctr)
if err != nil {
return "", err
}

return filepath.Join(persistDir, "oom"), nil
}

// RuntimeInfo provides information on the runtime.
Expand Down Expand Up @@ -1086,7 +1096,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
}

persistDir := filepath.Join(r.persistDir, ctr.ID())
args, err := r.sharedConmonArgs(ctr, ctr.ID(), ctr.bundlePath(), pidfile, ctr.LogPath(), r.exitsDir, persistDir, ociLog, ctr.LogDriver(), logTag)
args, err := r.sharedConmonArgs(ctr, ctr.ID(), ctr.bundlePath(), pidfile, ctr.LogPath(), persistDir, ociLog, ctr.LogDriver(), logTag)
if err != nil {
return 0, err
}
Expand Down Expand Up @@ -1345,8 +1355,8 @@ func (r *ConmonOCIRuntime) configureConmonEnv() ([]string, error) {
}

// sharedConmonArgs takes common arguments for exec and create/restore and formats them for the conmon CLI
// func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, pidPath, logPath, exitDir, persistDir, ociLogPath, logDriver, logTag string) ([]string, error) {
func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, pidPath, logPath, exitDir, persistDir, ociLogPath, logDriver, logTag string) ([]string, error) {
// func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, pidPath, logPath, persistDir, ociLogPath, logDriver, logTag string) ([]string, error) {
func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, pidPath, logPath, persistDir, ociLogPath, logDriver, logTag string) ([]string, error) {
// Make the persists directory for the container after the ctr ID is appended to it in the caller
// This is needed as conmon writes the exit and oom file in the given persist directory path as just "exit" and "oom"
// So creating a directory with the container ID under the persist dir will help keep track of which container the
Expand All @@ -1364,7 +1374,6 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
"-b", bundlePath,
"-p", pidPath,
"-n", ctr.Name(),
"--exit-dir", exitDir,
"--persist-dir", persistDir,
"--full-attach",
}
Expand Down
2 changes: 1 addition & 1 deletion libpod/oci_conmon_exec_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
}
defer processFile.Close()

args, err := r.sharedConmonArgs(c, sessionID, c.execBundlePath(sessionID), c.execPidPath(sessionID), c.execLogPath(sessionID), c.execExitFileDir(sessionID), c.execPersistDir(sessionID), ociLog, define.NoLogging, c.config.LogTag)
args, err := r.sharedConmonArgs(c, sessionID, c.execBundlePath(sessionID), c.execPidPath(sessionID), c.execLogPath(sessionID), c.execPersistDir(sessionID), ociLog, define.NoLogging, c.config.LogTag)
if err != nil {
return nil, nil, err
}
Expand Down
27 changes: 20 additions & 7 deletions libpod/oci_missing.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ var (
type MissingRuntime struct {
// Name is the name of the missing runtime. Will be used in errors.
name string
// exitsDir is the directory for exit files.
exitsDir string
// persistDir is the directory for exit and oom files.
persistDir string
}
Expand All @@ -53,7 +51,6 @@ func getMissingRuntime(name string, r *Runtime) OCIRuntime {

newRuntime := new(MissingRuntime)
newRuntime.name = name
newRuntime.exitsDir = filepath.Join(r.config.Engine.TmpDir, "exits")
newRuntime.persistDir = filepath.Join(r.config.Engine.TmpDir, "persist")

missingRuntimes[name] = newRuntime
Expand Down Expand Up @@ -213,22 +210,38 @@ func (r *MissingRuntime) ExecAttachSocketPath(ctr *Container, sessionID string)
return "", r.printError()
}

// PersistDir returns the persit dir containing oom & exit files for containers.
func (r *MissingRuntime) PersistDir(ctr *Container) (string, error) {
if ctr == nil {
return "", fmt.Errorf("must provide a valid container to get persist dir: %w", define.ErrInvalidArg)
}

return filepath.Join(r.persistDir, ctr.ID()), nil
}

// ExitFilePath returns the exit file path for containers.
// Here, we mimic what ConmonOCIRuntime does, because there is a chance that the
// container in question is still running happily (config file modified to
// remove a runtime, for example). We can't find the runtime to do anything to
// the container, but Conmon should still place an exit file for it.
func (r *MissingRuntime) ExitFilePath(ctr *Container) (string, error) {
if ctr == nil {
return "", fmt.Errorf("must provide a valid container to get exit file path: %w", define.ErrInvalidArg)
persistDir, err := r.PersistDir(ctr)
if err != nil {
return "", err
}
return filepath.Join(r.exitsDir, ctr.ID()), nil

return filepath.Join(persistDir, "exit"), nil
}

// OOMFilePath returns the oom file path for a container.
// The oom file will only exist if the container was oom killed.
func (r *MissingRuntime) OOMFilePath(ctr *Container) (string, error) {
return filepath.Join(r.persistDir, ctr.ID(), "oom"), nil
persistDir, err := r.PersistDir(ctr)
if err != nil {
return "", err
}

return filepath.Join(persistDir, "oom"), nil
}

// RuntimeInfo returns information on the missing runtime
Expand Down