Skip to content

[ci:docs] Clean up /var/tmp/ when creating containers from oci-archive tarballs #19201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jdoss wants to merge 1 commit into from
Closed

[ci:docs] Clean up /var/tmp/ when creating containers from oci-archive tarballs #19201

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions contrib/tmpfile/podman.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ x /tmp/run-*/libpod
D! /var/lib/containers/storage/tmp 0700 root root
D! /run/podman 0700 root root
D! /var/lib/cni/networks
# Remove /var/tmp/oci* and /var/tmp/storage* podman temporary directories on each
# boot which are created when creating containers from oci-archive tarballs
R! /var/tmp/oci*
R! /var/tmp/storage*
Copy link
Member

@edsantiago edsantiago Jul 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit alarming: storage is a generic word, it is easy to imagine regular users saving /var/tmp/storage-archive.tgz and being surprised to find it gone on reboot. (Let's leave aside discussion of the merits of expecting anything in a tmpdir to survive)

Suggestion: instead of *, try [0-9]+$ (first making sure that those patterns are valid in this context)

[EDIT: this applies to both added patterns, both oci and storage]

Copy link
Member

@edsantiago edsantiago Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better than that, a better solution would be for @containers/podman-maintainers to use better namespaces (podman-unpack-*) and to find (and plug) the leaks.

Copy link
Contributor Author

@jdoss jdoss Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought of this too so that is why I did on reboot vs using https://www.freedesktop.org/software/systemd/man/tmpfiles.d.html#e to have it be removed when systemd-tmpfiles-clean.service fires off it's timer. At least in this case it can stick around until reboot. I know you want to put this aside but I also don't expect anything in tmpdirs to survive a reboot which is why I went this direction. It a pretty well known that if tmpdirs are temporary.

Looking into your suggestion

-rw-r--r--. 1 jdoss jdoss    0 Jul 11 20:04 oci1234.tar
drwxr-xr-x. 1 jdoss jdoss    0 Jul 11 20:03 oci1333441593
drwxr-xr-x. 1 jdoss jdoss    0 Jul 11 20:04 oci2343867263
drwxr-xr-x. 1 jdoss jdoss    0 Jul 11 20:04 storage1255676731
drwx------. 1 jdoss jdoss    2 Jun 23 14:56 storage162146237
drwx------. 1 jdoss jdoss    0 Jun 23 14:47 storage1628300548
-rw-r--r--. 1 jdoss jdoss    0 Jul 11 20:04 storage.tar

This shell-style globbing seems to work

$ ls storage*[0-9]
storage1255676731:

storage162146237:
1

storage1628300548:

$ ls oci*[0-9]
oci1333441593:

oci2343867263:

But I am hesitant to to do anything more than what my PR provides because again, if something is in /var/tmp I don't expect it to survive a reboot. Heck, someone could create a directory like this and it will get caught.

$ mkdir storage123
[jdoss@sw-0608 tmp]$ ls storage*[0-9]
storage123:

storage1255676731:

storage162146237:
1

storage1628300548:

So even the shell-style globbing doesn't catch this edge case.

It would be handy if these directories were to have a prefix of podman- to make it more error proof when removing them with systemd-tmpfiles.d but this is a quick fix to prevent systems from running out of disk space. Ideally we should change to a podman- prefix on these directories and adjust this file later so they are cleaned up with systemd-tmpfiles-clean.service

Copy link
Contributor Author

@jdoss jdoss Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better than that, a better solution would be for @containers/podman-maintainers to use better namespaces (podman-unpack-*) and to find (and plug) the leaks.

I missed this reply before I sent mine. I agree wholeheartedly.