podman icon indicating copy to clipboard operation
podman copied to clipboard

Stopping Quadlet - ExecStop overwrites custom settings

Open thomascastelein opened this issue 1 year ago • 8 comments
trafficstars

Issue Description

Been trying to migrate from custom start scripts with systemd-generate to quadlets and is working quite wel. The only thing I noticed is when I start a container it automatically adds the --rm option even when I set it to --rm=false in the [container] section. This means every time we stop the service with "systemctl stop app1" it automaticly removes the container. We would like to have the container in a exited/stopped state.

I tried to add the ExecStop and ExecStopPost to /usr/bin/podman stop app1 in [Service] but gets overwritten with the default: ExecStopPost=/usr/bin/podman rm -v -f -I app1 << Visible in systemctl status app1

Funny enough if I change it to podman stop app1-doesnotexist then it stops the container but does not remove it. Also the default ExecStopPost=/usr/bin/podman rm -v -f -I is not visible. For us this is what we want but means we would have to stop a non existing container revery time we want to just stop the container.

Steps to reproduce the issue

Steps to reproduce the issue

  1. Use ExecStopPost=/usr/bin/podman stop app1 (With container config --rm=false)
  2. systemctl start app1
  3. systemctl stop app1
  4. Container is stopped, container removed.

Systemctl status app1


● app1.service - Podman app1.service
   Loaded: loaded (/etc/containers/systemd/app1.container; generated)
   Active: failed (Result: exit-code) since Fri 2024-07-26 12:19:52 CEST; 3s ago
  Process: 2033681 ExecStopPost=/usr/bin/podman stop app1 (code=exited, status=125)
  Process: 2033665 ExecStop=/usr/bin/podman rm -v -f -i --cidfile=/run/app1.cid (code=exited, status=0/SUCCESS)
  Process: 2033405 ExecStop=/usr/bin/podman stop app1 (code=exited, status=0/SUCCESS)
  Process: 2033001 ExecStart=/usr/bin/podman run --name=app1 --cidfile=/run/app1.cid --replace --rm --cgroups=split --network=${OMGEVING} --sdnotify=conmon -d
 Main PID: 2033001 (code=exited, status=137)

  1. Use ExecStopPost=/usr/bin/podman stop app1-does-not-exist (With container config --rm=false)
  2. systemctl start app1
  3. systemctl stop app1
  4. Container is stopped, container is not removed.

Systemctl status app1


● app1.service - Podman app1.service
   Loaded: loaded (/etc/containers/systemd/app1.container; generated)
   Active: failed (Result: exit-code) since Fri 2024-07-26 12:22:23 CEST; 10s ago
  Process: 2035480 ExecStopPost=/usr/bin/podman stop app1-app1 (code=exited, status=125)
  Process: 2035229 ExecStop=/usr/bin/podman stop app1-app1 (code=exited, status=125)
  Process: 2034826 ExecStart=/usr/bin/podman run --name=${OMGEVING}-app1 --cidfile=/run/app1.cid --replace --rm --cgroups=split --network=${OMGEVING} --sdnotify=conmon -d
 Main PID: 2034826 (code=exited, status=0/SUCCESS)

Describe the results you received

As mentioned above, I would not expect the default ExecStop to be podman --rm as this would normally also not be default when using the systemd generate for podman. This would only be achieved with the --new option.

The app1.container conf is the following (Removed all private settings)

[Install]
WantedBy=default.target

[Unit]
Description=Podman app1.service
Wants=network-online.target
After=network-online.target

[Service]
EnvironmentFile=/opt/config/params.env
Restart=on-failure
TimeoutStopSec=70
ExecStopPost=/usr/bin/podman stop app1
ExecStop=/usr/bin/podman stop app1

[Container]
ContainerName=app1
Image=XXXX/:${VERSION}
IP=XXXX
Network=${ENVROIMENT}
PodmanArgs=--rm false

Describe the results you expected

Container gets deleted with default configuration.

podman info output

Client:       Podman Engine
Version:      4.9.4-rhel
API Version:  4.9.4-rhel
Go Version:   go1.21.7 (Red Hat 1.21.7-2.module+el8.10.0+21638+b01be198)
Built:        Tue Jun 18 11:34:14 2024
OS/Arch:      linux/amd64

Running on 
REDHAT_BUGZILLA_PRODUCT="Red Hat Enterprise Linux 8"
REDHAT_BUGZILLA_PRODUCT_VERSION=8.10

Podman in a container

Yes

Privileged Or Rootless

Privileged

Upstream Latest Release

Yes

Additional environment details

No response

Additional information

No response

thomascastelein avatar Jul 26 '24 10:07 thomascastelein

@ygalblum @vrothberg @alexlarsson I believe this has been discussed in the past.

rhatdan avatar Jul 26 '24 12:07 rhatdan

When running the following

/usr/lib/systemd/system-generators/podman-system-generator {--user} --dryrun

I can see what the podman generate makes of my settings. The bits bellow seem to alway be generated. Is this by default? Because I have tried everything to prevent this from happening but stil applies.

Environment=PODMAN_SYSTEMD_UNIT=%n
KillMode=mixed
ExecStop=/usr/bin/podman rm -v -f -i --cidfile=%t/%N.cid
ExecStopPost=-/usr/bin/podman rm -v -f -i --cidfile=%t/%N.cid
Delegate=yes
Type=notify
NotifyAccess=all
SyslogIdentifier=%N
ExecStart=/usr/bin/podman run --name=app1 --cidfile=%t/%N.cid --replace --rm --cgroups=split 

thomascastelein avatar Jul 26 '24 13:07 thomascastelein

The problem is we would need to convert quadlets to a start/stop mode rather then just use of run.

rhatdan avatar Jul 26 '24 14:07 rhatdan

Seems to me this is hard coded in. Should this not be checking if a ExecStop/ExecStopPost already exists? We really would like a way to stop the containers from deleting it self after a shutdown. We host around 1000+ containers running on simple generated .service files and the migration to quadlets would mean we would have even less configurations that we used for each server.

Stopping containers meant that most monitoring tools could easily read out the status of containers. Removing this feature means its pretty useless to be using the podman.socket/API. as its only known to systemd.

Also, I am addressing this just to comply on new updates from podman because I love to join in on any new features. I've been using podman for the past 5 years and really like the new improvements. This is the first time i find something that struck me as a weird change.

pkg/systemd/quadlet/quadlet.go

	// If conmon exited uncleanly it may not have removed the container, so
	// force it, -i makes it ignore non-existing files.
	serviceStopCmd := createBasePodmanCommand(container, ContainerGroup)
	serviceStopCmd.add("rm", "-v", "-f", "-i", "--cidfile=%t/%N.cid")
	service.AddCmdline(ServiceGroup, "ExecStop", serviceStopCmd.Args)
	// The ExecStopPost is needed when the main PID (i.e., conmon) gets killed.
	// In that case, ExecStop is not executed but *Post only.  If both are
	// fired in sequence, *Post will exit when detecting that the --cidfile
	// has already been removed by the previous `rm`..
	serviceStopCmd.Args[0] = fmt.Sprintf("-%s", serviceStopCmd.Args[0])
	service.AddCmdline(ServiceGroup, "ExecStopPost", serviceStopCmd.Args)

	podman := createBasePodmanCommand(container, ContainerGroup)

	podman.add("run")

	podman.addf("--name=%s", containerName)

	podman.add(
		// We store the container id so we can clean it up in case of failure
		"--cidfile=%t/%N.cid",

		// And replace any previous container with the same name, not fail
		"--replace",

		// On clean shutdown, remove container
		"--rm",
	)

thomascastelein avatar Jul 26 '24 17:07 thomascastelein

You could open a PR to make the suggested changes. But not adding an ExecStop and using the one you define, means that the container would not be destroyed, which means the next time the unit starts the ExecStart would execute a podman run ... command which will blow up with the container already exists error.

Which means you would have to override the ExecStart command as well. It would need to catch failure on podman run and do a podman start, I would guess.

rhatdan avatar Jul 27 '24 10:07 rhatdan

I could do that, although i'm no coding expert.. The thing that just does not make sense to me is that the old generator was easy to fire up once and then just rely on the service it created, as in using the stop and start commands. Quadlets don't do anything like that.

Sure, we could just create our own easily writable services, but it would be nice to have the option in quadlets nevertheless.

To make an example of how we use podman in our production servers, this is the .sh script we fire up once at server creation and every time we update the app1 version.

systemctl stop app1
systemctl disable app1
podman stop app1
podman rm app1
podman run -d --name app1 #Lots of other stuf, like mounts, add-hosts, envroimentals ect.
privaterepo/app1:$VERSIE
podman generate systemd app1 > /etc/systemd/system/app1.service
podman stop app1
systemctl --now enable app1

thomascastelein avatar Jul 27 '24 11:07 thomascastelein

Hi, @rhatdan yes, we have discussed this issue in the past. I think one of the discussions was around podmansh.

I understand the requirement and I see the reasons for it. We had some technical issues we didn't resolve in the previous discussions. But, this doesn't mean we can't discuss them some more.

One idea was to change Quadlet's output and instead of using run in ExecStart, use create in ExecStartPre and start in ExecStart. However, using create poses an issue.

Currently, create does not have an ignore flag. As a result, once the container exists, the call to create will fail. Adding such a flag to podman should not be an issue. The issue, however, is what happens when a change is actually needed. By disregarding an error or by supporting an ignore flag, changes will not be applied.

Note than we have done this in .volume. Volumes are created once and in order to apply changes, the user must manually remove the volume. We could add a note about it and because this will not be the default behavior, advanced users will have to be more careful.

WDYT?

ygalblum avatar Jul 29 '24 06:07 ygalblum

The problem is that quadlet depends on certain container options, if we add a ignore flag it means the container can be created outside the unit without the proper options such as --sdnotify=conmon breaking the resulting unit in weird ways. Sure we can call this a user error but this will be super confusing and it will make it impossible for quadlet to ever update any of the generated options that would make it incompatible with previous options (which AFAIK was a big reason for a systemd generator in the first place)

The next problem would be how do you deal with cidfiles? We would need to teach a new hypothetical --ignore option to still write cidfiles which is extra work. Unless we get rid of cidfiles in quadlet which might be a good idea regardless?

It will also make podman auto-update non functional as this expects a restart of the unit to recreate the container with the name image.

Luap99 avatar Jul 29 '24 08:07 Luap99

Hi,

podman generate systemd delivers output for a recommended systemd .service file, which is adjustable to my needs, but has caveats, if we don't understand each switch exactly.

quadlet has the demand to make "better settings for container under systemd" #15686 . But maybe we loose some flexibility here.


However, podman generate systemd delivered such lines:

...
ExecStart=/usr/bin/podman run \
        --cidfile=%t/%n.ctr-id \
        --cgroups=no-conmon \
        --rm \
        --sdnotify=conmon \
        -d \
        --replace \
...
ExecStop=/usr/bin/podman stop \
        --ignore -t 10 \
        --cidfile=%t/%n.ctr-id
ExecStopPost=/usr/bin/podman rm \
        -f \
        --ignore -t 10 \
        --cidfile=%t/%n.ctr-id
...

So, for ExecStop it sets podman stop to stop the container. If the container was run with --rm and --replace, the container will be removed after stop or replaced at start, if there exists a container with the same name. At 'ExecStopPost' it makes a container removal for the case, that the container crashed instead of correctly stopped.

Quadlet generates such lines:

...
ExecStart=/usr/bin/podman run --name=systemd-%N --cidfile=%t/%N.cid --replace --rm --cgroups=split ...
...
ExecStop=/usr/bin/podman rm -f -i --cidfile=%t/%N.cid
ExecStopPost=-/usr/bin/podman rm -f -i --cidfile=%t/%N.cid
...

  1. Why does quadlet sets podman rm for ExecStop instead of podman stop. Isn't it kind of disruptive, if it deletes the container files before stopping the container?
  2. Quadlet seems to use the default timeout of 10 seconds. But it seems, that I can't set this timeout within a quadlet .container file. But why?
  3. Why does quadlet uses the short named parameters like -i instead of the full named parameters like --ignore to make the generated files more understandable like podman generate systemd already does?

I have a container with such settings from above for ExecStop and ExecStopPost generated by quadlet. If I stop the container with systemctl stop container_name, it always throws such exit code status=2/INVALIDARGUMENT:

Aug 07 10:55:21 machine podman[122654]: 2024-08-07 10:55:21.164891453 +0200 CEST m=+0.071511852 container died ffdc707c5d5d07b24d80d25ac2a1c3651c787533abb1e7ef8f79f779>
Aug 07 10:55:21 machine podman[122654]: 2024-08-07 10:55:21.405877902 +0200 CEST m=+0.312497830 container remove ffdc707c5d5d07b24d80d25ac2a1c3651c787533abb1e7ef8f79f7>
Aug 07 10:55:21 machine container_name[122654]: ffdc707c5d5d07b24d80d25ac2a1c3651c787533abb1e7ef8f79f779f0a571f0
Aug 07 10:55:21 machine systemd[1]: container_name.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
Aug 07 10:55:21 machine systemd[1]: container_name.service: Failed with result 'exit-code'.
  1. How can I fix that "stopping" issue?

Greetings and thanks for any help =)

ScrambledBrain avatar Aug 07 '24 09:08 ScrambledBrain

A friendly reminder that this issue had no activity for 30 days.

github-actions[bot] avatar Sep 07 '24 00:09 github-actions[bot]

Is there any way to revive this issue? I would really like to see some possibilities to being able to stop containers without removing them. So most monitoring software and general overview of containers remain an easy task.

It's a real pain to go from something that's super easy to use, to a quadlet that force deletes my containers on a simple systemctl stop command. Im stil not convinced to make the big change to quadlets. using podman with the systemd generate has been an absolute blessing and would love to have the same functionality.

To put it in some perspective, Lets say I log into a server, a container failed because of a memory issue and it got killed. I would have to look into the systemd folder and compare it with 40 other running containers to figure out what container just died. Yes I could also check the journal for the kill logs, still. It's different then just using podman ps -an and check for the stopped container.

thomascastelein avatar Oct 29 '24 17:10 thomascastelein

To put it in some perspective, Lets say I log into a server, a container failed because of a memory issue and it got killed. I would have to look into the systemd folder and compare it with 40 other running containers to figure out what container just died. Yes I could also check the journal for the kill logs, still. It's different then just using podman ps -an and check for the stopped container.

I think podman events --filter=event=died --until=0h --stream=false might go into the direction you want. (Yes the ergonomics of podman events leave a lot to be desired, but it's start i guess)

Histalek avatar Oct 29 '24 20:10 Histalek

So just trying to figure out what would be posible, so sorry for sounding recless,

We did our own sort of cleanup each time we starten a cotnainer.sh script. That simply did the following:

systemctl stop app1
systemctl disable app1
podman stop app1
podman rm app1
podman run -d --name app1 privaterepo/app1:$VERSIE
podman generate systemd app1 > /etc/systemd/system/app1.service
podman stop app1
systemctl --now enable app1

Could we not just simply do the same as a start check? If %t/%N.cid exists, just do the cleanup then and not after a systemctl stop like so: ExecStartPre=/bin/sh -c '[ -f %t/%N.cid ] && /usr/bin/podman rm -f -i --cidfile=%t/%N.cid'

Only assuming we do this for the cid file...

thomascastelein avatar Oct 30 '24 08:10 thomascastelein

IIUC, this proposal solution specifically addresses the requirement you raised in the comment before it. That is, systemctl stop will not remove the container, but, systemctl start will still start a new one. Right? If so, maybe add a key in the [Container] section, something like KeepContainer which will result in the proposed implementation. In any case, I would keep the current behaviour is the default one.

ygalblum avatar Oct 31 '24 12:10 ygalblum

Yes, im totally fine letting it be the default behavior. Our main goal is to keep the functionality to simply start and stop a container, but without it being removed. The only times we would need a container to be deleted and started is if we would require changes in the service file which does not happen often.

And if we update our image, it would be recreated anyway.

A KeepContainer as an extra option would be a good start!

thomascastelein avatar Oct 31 '24 15:10 thomascastelein

I understand. But, keep in mind that with your suggestion of setting ExecStartPre=/bin/sh -c '[ -f %t/%N.cid ] && /usr/bin/podman rm -f -i --cidfile=%t/%N.cid', the container is recreated on every start. It's just that it is not removed upon stop, only on the next start.

ygalblum avatar Oct 31 '24 18:10 ygalblum

To be honest, that would be perfect.

thomascastelein avatar Oct 31 '24 19:10 thomascastelein

@ygalblum I was looking thru my older comment and saw that we already replace/remove an existing container with the --replace when starting a quadlet service by default so im guessing this would not matter. This would only be a problem if I would rename my quadlet and start it. The old pid would remain, but only until a reboot of the host so I would not see a problem.

// And replace any previous container with the same name, not fail
		"--replace",

So as far as I can see what we do at stop is only delete the pid. If no-one else sees any problems with some of us to keep containers when we stop them. What would be my route te accomplish something like this? Would I need to create my own PR? Id probably have to ask our dev team for that.

thomascastelein avatar Nov 01 '24 14:11 thomascastelein

You are correct about the --replace flag. So, it is mainly about what happens during stop (and maybe also about the --rm flag of the ExecStart command).

As for how, yes you will need to make the code change, document the new key in the man page and add a test (since the change is in the behaviour of systemd, you'll need to implement a system test in https://github.com/containers/podman/blob/main/test/system/252-quadlet.bats). Once you have all these, you can open a PR.

ygalblum avatar Nov 01 '24 18:11 ygalblum

Hi, I was wondering if there has been any updates on the current status of this. I'd also like the option to stop the container without removing it too. :)

tyrone-wu avatar Oct 27 '25 17:10 tyrone-wu

@tyrone-wu care to open a PR?

ygalblum avatar Oct 28 '25 13:10 ygalblum

For context, I'm using quadlets in my homelab so it's not as big of deal for this to get upstream since i'm rolling with my own forked experimental/cursed version anyway. 🫠

One idea was to change Quadlet's output and instead of using run in ExecStart, use create in ExecStartPre and start in ExecStart.

I basically implemented this idea, prefixed podman create with -, and added a KeepOnStop= to adjust whether to stop or remove the container.

It's a bandaid workaround when create fails when container exists instead of implementing a proper ignore flag.

The issue, however, is what happens when a change is actually needed. By disregarding an error or by supporting an ignore flag, changes will not be applied.

only issue, as you mentioned earlier, is handling changes in the quadlet file (and maybe auto-update?).

(auto-update only updates images, applies to running containers, and restarts the unit, so changes in the quadlet file don't apply until unit is re-generated and restarted.)

2 options come to mind for going about this:

  1. Note than we have done this in .volume.

    as you mentioned earlier, leave it up to the user to manually remove the container.

  2. Add a new flag in the create command to replace the container if config is different.

    In this case, daemon-reload to update the unit file (in turn update the create), and restart (or stop, `start) to re-create the container.

    Comparing whether the container configuration (the ones that matter) has changed could be difficult. maybe compare CreateCommand?, however that's under inspect.

    Also if container name is changed, then this would end up just creating a new container instead of replacing. Will probably need to add a warning/note for this.

That being said, im using this for homelab so it's not critical for me to have this upstream. These were my thoughts while i was implementing a messy option 1. option 2 might be better overall.

Someone with a business usecase could probably chime in on their day 2 operation for a more formal take on how their workflow goes.

tyrone-wu avatar Nov 20 '25 22:11 tyrone-wu