Remove cidfile annotation from remote #23155
Conversation
Cidfile location should not be passed over on a remote connection. This way you will not mess the host configuraion on container removal. Resolves: containers#21974 Signed-off-by: Nicola Sella <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: inknos The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I am still unsure that this is the right approach. @Luap99 what do you think? |
There was a problem hiding this comment.
This fixes the remote issue but reintroduces the ones this was supposes to fix 3fee351
Overall I see no elegant fix, I think the easiest would be to drop the annotation on the service side in pkg/api/handlers/libpod/containers_create.go as this will not chnage the local code path
I wonder if removing the cid files is really the correct thing, compare #23106 It seems we no longer match docker because of that.
So maybe the easiest is to fix that issue and not deal with removing at all? But then again we need to consider if this would be a breaking change or not.
@mheon WDYT?
|
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. |
|
Dropping them entirely in Quadlet seems like a good idea. I think we risk regressions in unit files from |
|
A friendly reminder that this PR had no activity for 30 days. |
Cidfile location should not be passed over on a remote connection. This way you will not mess the host configuraion on container removal.
Resolves: #21974
Does this PR introduce a user-facing change?