[v5.4-rhel] 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

Fixes: https://issues.redhat.com/browse/RHEL-86544, https://issues.redhat.com/browse/RHEL-86550

Signed-off-by: Matt Heon <[email protected]>
Signed-off-by: tomsweeneyredhat <[email protected]>

Author: mheon

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

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)
 

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)

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{