search

containers / podman

Podman: A tool for managing OCI containers and pods.
https://podman.io
Apache License 2.0
22.4k stars 2.31k forks source link

Remove cidfile annotation from remote #23155

Open inknos opened 4 days ago

inknos commented 4 days ago

Cidfile location should not be passed over on a remote connection. This way you will not mess the host configuraion on container removal.

Resolves: https://github.com/containers/podman/issues/21974

Does this PR introduce a user-facing change?

None
openshift-ci[bot] commented 4 days ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: inknos Once this PR has been reviewed and has the lgtm label, please assign jakecorrenti for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/containers/podman/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
inknos commented 4 days ago

I am still unsure that this is the right approach. @Luap99 what do you think?

mheon commented 3 days ago

Was the removal added because of Quadlet/Systemd? I feel like it might have been...

Luap99 commented 3 days ago

Was the removal added because of Quadlet/Systemd? I feel like it might have been...

Yes but we can change the units to do the removal in ExecStopPost=rm -f file. Also quadlet always assigns a name so we could drop these cidfile entirely there I guess but maybe there is a reason for them I am missing currently.

mheon commented 3 days ago

Dropping them entirely in Quadlet seems like a good idea. I think we risk regressions in unit files from generate systemd that are not regenerated on upgrade (which is really why Quadlet exists...) though.