Skip to content

Use /tmp/podman-run-* for backup XDG_RUNTIME_DIR#8241

Merged
openshift-merge-robot merged 1 commit intocontainers:masterfrom
rhatdan:tmpfile
Nov 6, 2020
Merged

Use /tmp/podman-run-* for backup XDG_RUNTIME_DIR#8241
openshift-merge-robot merged 1 commit intocontainers:masterfrom
rhatdan:tmpfile

Conversation

@rhatdan
Copy link
Copy Markdown
Member

@rhatdan rhatdan commented Nov 4, 2020

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]

@openshift-ci-robot
Copy link
Copy Markdown
Collaborator

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 4, 2020
Comment thread Makefile Outdated
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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-*?

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member Author

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.

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

Weird test error, and feels a bit kludgey.

@rhatdan
Copy link
Copy Markdown
Member Author

rhatdan commented Nov 4, 2020

@TomSweeneyRedHat What feels kludgy?

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

LGTM
assuming happy tests which aren't too hip atm

Copy link
Copy Markdown
Member

@ashley-cui ashley-cui left a comment

Choose a reason for hiding this comment

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

LGTM

@mheon
Copy link
Copy Markdown
Member

mheon commented Nov 6, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2020
@openshift-merge-robot openshift-merge-robot merged commit dc58d4e into containers:master Nov 6, 2020
@github-actions github-actions Bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

run files under /tmp can be clobbered by systemd.tmpfiles

6 participants