-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Use /tmp/podman-run-* for backup XDG_RUNTIME_DIR #8241
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -493,6 +493,8 @@ install.bin-nobuild: | |||
install ${SELINUXOPT} -d -m 755 $(DESTDIR)$(BINDIR) | |||
install ${SELINUXOPT} -m 755 bin/podman $(DESTDIR)$(BINDIR)/podman | |||
test -z "${SELINUXOPT}" || chcon --verbose --reference=$(DESTDIR)$(BINDIR)/podman bin/podman | |||
install ${SELINUXOPT} -m 755 -d ${DESTDIR}${TMPFILESDIR} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For historical purposes, could you add a comment here explaining this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You want a comment in the man page explaining why we are installing podman.conf, or do you want a comment in podman.conf telling you what the line in their is for? Telling systemd to not prune /tmp/podman-run-*?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking of just a comment here explaining the change with a pointer to the issue. If you think a man page change would serve better, and/or a comment in /tmp/podman-run-*
I'm fine with either or both. Just so that two years from now someone doesn't remove it without realizing it's need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am just thinking the message should go into the podman.conf file. Not here. If I am removing content, I am likely to read the file. Added explanation to podman.conf file.
Weird test error, and feels a bit kludgey. |
@TomSweeneyRedHat What feels kludgy? |
Just dropping a file in a dir to keep it from being deleted feels kludgy to me. Yeah, not our code doing that and I don't have a better suggestion. |
We need to block systemd from cleaning up this directory by dropping a /usr/lib/tmpfiles.d/podman.conf file in place. Signed-off-by: Daniel J Walsh <[email protected]>
# /tmp/podman-run-* directory can contain content for Podman containers that have run | ||
# for many days. This following line prevents systemd from removing this content. | ||
x /tmp/podman-run-.* | ||
d /run/podman 0700 root root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/lgtm |
We need to block systemd from cleaning up this directory
by dropping a /usr/lib/tmpfiles.d/podman.conf file in place.
Fixes: #7852
Signed-off-by: Daniel J Walsh [email protected]