Skip to content
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.

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

container/commit: Auto-clean /var/tmp, /tmp, /run #367

Merged
merged 3 commits into from
Sep 15, 2022

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Sep 13, 2022

The original command here was just scoped to /var, but we also
don't want content in /run. Extend the tooling to handle that
and the other two temporary directories.

Also, let's be a bit nicer here and auto-clean empty directories
in /var.

I was testing out the
https://github.com/coreos/coreos-layering-examples/blob/main/tailscale/Dockerfile
example and today we have this:

drwxr-xr-x root/root         0 2022-09-13 18:53 run/
drwxr-xr-x root/root         0 2022-09-13 18:51 run/rpm-ostree/
drwxr-xr-x root/root         0 2022-09-13 18:53 run/rpm-ostree/lock/
drwxr-xr-x root/root         0 2022-09-13 18:51 run/systemd/
drwxr-xr-x root/root         0 2022-09-13 18:51 run/systemd/resolve/
-rwx------ root/root         0 2022-09-13 18:51 run/systemd/resolve/stub-resolv.conf
...
drwxr-xr-x root/root         0 2022-09-13 18:53 var/
drwxr-xr-x root/root         0 2022-09-13 18:53 var/cache/
drwx------ root/root         0 2022-09-13 18:53 var/cache/ldconfig/
-rw------- root/root     22000 2022-09-13 18:53 var/cache/ldconfig/aux-cache
drwxr-xr-x root/root         0 2022-09-08 23:10 var/cache/tailscale/
drwxr-xr-x root/root         0 2022-09-13 18:53 var/tmp/

In this set, we can auto-clean the leftover locking directories
rpm-ostree (erroneously) leaves in /run.

I am tempted to auto-cleanup the stub-resolv.conf thing and the
ldconfig/aux-cache.


@cgwalters
Copy link
Member Author

This pairs with #366

@lucab
Copy link
Member

lucab commented Sep 14, 2022

I do like the empty directories /var autocleaning, it's a clear UX improvement. It assumes there is something re-creating those on boot (e.g. tmpfiles.d fragment or the software itself), but I think that's fine. And we can always implement auto-translation to tmpfiles.d here in the future if we want.

I'm less convinced about inspecting /run. IMHO it makes sense to leave that one alone (i.e. ignore any content landing there), basically treating it similarly to /tmp as an ephemeral hierarchy.

@cgwalters cgwalters changed the title container/commit: Autoclean directories, also handle /run container/commit: Auto-clean /var/tmp, /tmp, /run Sep 14, 2022
@cgwalters
Copy link
Member Author

I'm less convinced about inspecting /run. IMHO it makes sense to leave that one alone (i.e. ignore any content landing there), basically treating it similarly to /tmp as an ephemeral hierarchy.

Good feedback! Fixed. For example, we no longer warn about anything in /var/tmp but just silently clean it, which definitely seems friendlier to me.

@cgwalters
Copy link
Member Author

Tangentially related, cc containers/buildah#4242 for a podman bug we may also want to work around in this code.

The original command here was just scoped to `/var`, but we also
don't want content in `/run`.  Extend the tooling to handle that
and the other two temporary directories.

Also, let's be a bit nicer here and auto-clean empty directories
in `/var`.

I was testing out the
https://github.com/coreos/coreos-layering-examples/blob/main/tailscale/Dockerfile
example and today we have this:

```
drwxr-xr-x root/root         0 2022-09-13 18:53 run/
drwxr-xr-x root/root         0 2022-09-13 18:51 run/rpm-ostree/
drwxr-xr-x root/root         0 2022-09-13 18:53 run/rpm-ostree/lock/
drwxr-xr-x root/root         0 2022-09-13 18:51 run/systemd/
drwxr-xr-x root/root         0 2022-09-13 18:51 run/systemd/resolve/
-rwx------ root/root         0 2022-09-13 18:51 run/systemd/resolve/stub-resolv.conf
...
drwxr-xr-x root/root         0 2022-09-13 18:53 var/
drwxr-xr-x root/root         0 2022-09-13 18:53 var/cache/
drwx------ root/root         0 2022-09-13 18:53 var/cache/ldconfig/
-rw------- root/root     22000 2022-09-13 18:53 var/cache/ldconfig/aux-cache
drwxr-xr-x root/root         0 2022-09-08 23:10 var/cache/tailscale/
drwxr-xr-x root/root         0 2022-09-13 18:53 var/tmp/
```

In this set, we can auto-clean the leftover locking directories
rpm-ostree (erroneously) leaves in `/run`, as well as
`/var/cache/ldconfig`.
I think it makes sense for us to use this in rpm-ostree directly
too at build time for example.
Copy link
Member

@lucab lucab left a comment

Choose a reason for hiding this comment

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

Code LGTM, but CI needs some tweaking.

@cgwalters cgwalters force-pushed the container-commit-autoclean branch 2 times, most recently from 721a79e to 5351c00 Compare September 15, 2022 18:50
To verify our changes there too.
@cgwalters
Copy link
Member Author

Got the new CI green 🎉

@cgwalters cgwalters merged commit 66095a8 into ostreedev:main Sep 15, 2022
cgwalters added a commit to cgwalters/coreos-layering-examples that referenced this pull request Sep 26, 2022
This command just deletes content from `/var/cache`, but
that's no longer needed since it's part of
ostreedev/ostree-rs-ext#367
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants