From 306df592abaa461628e8672e8f3389862d07d785 Mon Sep 17 00:00:00 2001 From: Stanislav Petrov Date: Mon, 28 Oct 2024 17:18:47 +0300 Subject: [PATCH] Remove exit and oom from persist dir Signed-off-by: luckyevildev --- libpod/container_exec.go | 8 +----- libpod/container_internal.go | 20 ++++++--------- libpod/oci.go | 3 +++ libpod/oci_conmon_common.go | 11 ++++++++- libpod/oci_conmon_common_test.go | 42 ++++++++++++++++++++++++++++++++ libpod/oci_missing.go | 26 ++++++++++++++++++-- 6 files changed, 87 insertions(+), 23 deletions(-) create mode 100644 libpod/oci_conmon_common_test.go diff --git a/libpod/container_exec.go b/libpod/container_exec.go index 42f6eae9e4..ea90533d19 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -887,7 +887,7 @@ func (c *Container) execAttachSocketPath(sessionID string) (string, error) { // execExitFileDir gets the path to the container's exit file func (c *Container) execExitFileDir(sessionID string) string { - return filepath.Join(c.execBundlePath(sessionID), "exit") + return filepath.Join(c.execPersistDir(sessionID), "exit") } // execPersistDir gets the path to the container's persist directory @@ -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) } diff --git a/libpod/container_internal.go b/libpod/container_internal.go index c7efd18e4b..17094b8604 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -153,6 +153,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 { @@ -757,22 +761,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 && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("removing container %s persist dir with exit & oom files: %w", c.ID(), err) } return nil diff --git a/libpod/oci.go b/libpod/oci.go index e0d7406339..d3661e9443 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -141,6 +141,9 @@ 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). diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go index 8aa103fb49..ca7995a048 100644 --- a/libpod/oci_conmon_common.go +++ b/libpod/oci_conmon_common.go @@ -848,12 +848,21 @@ 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) } - return filepath.Join(r.exitsDir, ctr.ID()), nil + return filepath.Join(r.persistDir, ctr.ID()), nil } // OOMFilePath is the path to a container's oom file. diff --git a/libpod/oci_conmon_common_test.go b/libpod/oci_conmon_common_test.go new file mode 100644 index 0000000000..999f5009dd --- /dev/null +++ b/libpod/oci_conmon_common_test.go @@ -0,0 +1,42 @@ +//go:build !remote + +package libpod + +import ( + "github.com/containers/podman/v5/libpod/lock" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestPersistDir(t *testing.T) { + runtime := &ConmonOCIRuntime{ + persistDir: "/tmp/persist", + } + + manager, err := lock.NewInMemoryManager(16) + + if err != nil { + t.Fatalf("Error setting up locks: %v", err) + } + + ctr, err := getTestCtr1(manager) + assert.NoError(t, err) + + dir, err := runtime.PersistDir(ctr) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + expectedDir := "/tmp/persist/" + ctr.config.ID + + if dir != expectedDir { + t.Fatalf("expected %s, got %s", expectedDir, dir) + } + + _, err = runtime.PersistDir(nil) + + if err == nil { + t.Fatalf("expected error, got nil") + } +} diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go index 98eb91ef8d..69a72150d2 100644 --- a/libpod/oci_missing.go +++ b/libpod/oci_missing.go @@ -208,6 +208,15 @@ 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 @@ -217,13 +226,26 @@ 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) } - return filepath.Join(r.exitsDir, ctr.ID()), nil + + persistDir, err := r.PersistDir(ctr) + + if err != nil { + return "", err + } + + 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