Skip to content

Commit

Permalink
Remove persist directory when cleaning up Conmon files
Browse files Browse the repository at this point in the history
This seems to have been added as part of the cleanup of our
handling of OOM files, but code was never added to remove it, so
we leaked a single directory with an exit file and OOM file per
container run. Apparently have been doing this for a while - I'd
guess since March of '23 - so I'm surprised more people didn't
notice.

Fixes #25291

Signed-off-by: Matt Heon <mheon@redhat.com>
  • Loading branch information
mheon authored and openshift-cherrypick-robot committed Feb 11, 2025
1 parent 19cb9f3 commit fd5eb1f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 4 deletions.
14 changes: 10 additions & 4 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) persistDirPath() (string, error) {
return c.ociRuntime.PersistDirectoryPath(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 @@ -766,13 +770,15 @@ func (c *Container) removeConmonFiles() error {
return fmt.Errorf("removing container %s exit file: %w", c.ID(), err)
}

// Remove the oom file
oomFile, err := c.oomFilePath()
// Remove the persist directory
persistDir, err := c.persistDirPath()
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 persistDir != "" {
if err := os.RemoveAll(persistDir); err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("removing container %s persist directory: %w", c.ID(), err)
}
}

return nil
Expand Down
8 changes: 8 additions & 0 deletions libpod/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ type OCIRuntime interface { //nolint:interfacebloat
// This is the path to that file for a given container.
OOMFilePath(ctr *Container) (string, error)

// PersistDirectoryPath is the path to a container's persist directory.
// Not all OCI runtime implementations will have a persist directory.
// If they do, it may contain files such as the exit file and the OOM
// file.
// If the directory does not exist, the empty string and no error should
// be returned.
PersistDirectoryPath(ctr *Container) (string, error)

// RuntimeInfo returns verbose information about the runtime.
RuntimeInfo() (*define.ConmonInfo, *define.OCIRuntimeInfo, error)

Expand Down
5 changes: 5 additions & 0 deletions libpod/oci_conmon_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,11 @@ func (r *ConmonOCIRuntime) OOMFilePath(ctr *Container) (string, error) {
return filepath.Join(r.persistDir, ctr.ID(), "oom"), nil
}

// PersistDirectoryPath is the path to the container's persist directory.
func (r *ConmonOCIRuntime) PersistDirectoryPath(ctr *Container) (string, error) {
return filepath.Join(r.persistDir, ctr.ID()), nil
}

// RuntimeInfo provides information on the runtime.
func (r *ConmonOCIRuntime) RuntimeInfo() (*define.ConmonInfo, *define.OCIRuntimeInfo, error) {
runtimePackage := version.Package(r.path)
Expand Down
6 changes: 6 additions & 0 deletions libpod/oci_missing.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,12 @@ func (r *MissingRuntime) OOMFilePath(ctr *Container) (string, error) {
return filepath.Join(r.persistDir, ctr.ID(), "oom"), nil
}

// PersistDirectoryPath is the path to the container's persist directory.
// It may include files like the exit file and OOM file.
func (r *MissingRuntime) PersistDirectoryPath(ctr *Container) (string, error) {
return filepath.Join(r.persistDir, ctr.ID()), nil
}

// RuntimeInfo returns information on the missing runtime
func (r *MissingRuntime) RuntimeInfo() (*define.ConmonInfo, *define.OCIRuntimeInfo, error) {
ocirt := define.OCIRuntimeInfo{
Expand Down

0 comments on commit fd5eb1f

Please sign in to comment.