From ce8813dc8d93f982634a88bb3cc54d334b5ebe69 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Tue, 11 Feb 2025 14:51:34 -0500 Subject: [PATCH] Remove persist directory when cleaning up Conmon files 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 --- libpod/container_internal.go | 14 ++++++++++---- libpod/oci.go | 8 ++++++++ libpod/oci_conmon_common.go | 5 +++++ libpod/oci_missing.go | 6 ++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index c95c9c2784..540554f3e0 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) 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 { @@ -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 diff --git a/libpod/oci.go b/libpod/oci.go index e0d7406339..f14b4acf83 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -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) diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go index 8aa103fb49..1b0f4c42da 100644 --- a/libpod/oci_conmon_common.go +++ b/libpod/oci_conmon_common.go @@ -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) diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go index 98eb91ef8d..bfdbbf228e 100644 --- a/libpod/oci_missing.go +++ b/libpod/oci_missing.go @@ -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{