Skip to content

Commit

Permalink
Remove exit and oom from persist dir
Browse files Browse the repository at this point in the history
Signed-off-by: luckyevildev <[email protected]>
  • Loading branch information
luckyevildev committed Oct 30, 2024
1 parent 19f7be8 commit 306df59
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 23 deletions.
8 changes: 1 addition & 7 deletions libpod/container_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
20 changes: 7 additions & 13 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions libpod/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
11 changes: 10 additions & 1 deletion libpod/oci_conmon_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 42 additions & 0 deletions libpod/oci_conmon_common_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
26 changes: 24 additions & 2 deletions libpod/oci_missing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 306df59

Please sign in to comment.