generate systemd: ExecStopPost for all units

Add an `ExecStopPost` run even for units generated without `--new`. Although it may seem redundant to run `container/pod stop` twice at first glance, we really need the post run. If the main PID (i.e., conmon) is killed, systemd will not execute `ExecStop` but only the post one. We made this obeservation in a customer issue and could reproduce the behavior consistently. Hence, the post run is needed to properly clean up when conmon is killed and it's pretty much a NOP in all other cases. Credits to Ulrich Obergfell for throrough and detailed analyses, which ultimately lead to this fix. Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
containers · Jun 17, 2020 · e5c3432 · e5c3432
1 parent f293606
commit e5c3432
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 4 deletions.
diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
@@ -83,9 +83,7 @@ ExecStartPre={{.ExecStartPre}}
 {{- end}}
 ExecStart={{.ExecStart}}
 ExecStop={{.ExecStop}}
-{{- if .ExecStopPost}}
 ExecStopPost={{.ExecStopPost}}
-{{- end}}
 PIDFile={{.PIDFile}}
 KillMode=none
 Type=forking
@@ -170,6 +168,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 	info.EnvVariable = EnvVariable
 	info.ExecStart = "{{.Executable}} start {{.ContainerNameOrID}}"
 	info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}"
+	info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}"
 
 	// Assemble the ExecStart command when creating a new container.
 	//

diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
@@ -50,6 +50,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
 ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
 ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
+ExecStopPost=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
 PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 KillMode=none
 Type=forking
@@ -71,6 +72,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
 ExecStart=/usr/bin/podman start foobar
 ExecStop=/usr/bin/podman stop -t 10 foobar
+ExecStopPost=/usr/bin/podman stop -t 10 foobar
 PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 KillMode=none
 Type=forking
@@ -96,6 +98,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
 ExecStart=/usr/bin/podman start foobar
 ExecStop=/usr/bin/podman stop -t 10 foobar
+ExecStopPost=/usr/bin/podman stop -t 10 foobar
 PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 KillMode=none
 Type=forking

diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
@@ -82,9 +82,7 @@ ExecStartPre={{.ExecStartPre2}}
 {{- end}}
 ExecStart={{.ExecStart}}
 ExecStop={{.ExecStop}}
-{{- if .ExecStopPost}}
 ExecStopPost={{.ExecStopPost}}
-{{- end}}
 PIDFile={{.PIDFile}}
 KillMode=none
 Type=forking
@@ -236,6 +234,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 	info.EnvVariable = EnvVariable
 	info.ExecStart = "{{.Executable}} start {{.InfraNameOrID}}"
 	info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}"
+	info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}"
 
 	// Assemble the ExecStart command when creating a new pod.
 	//

diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go
@@ -52,6 +52,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
 ExecStart=/usr/bin/podman start jadda-jadda-infra
 ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra
+ExecStopPost=/usr/bin/podman stop -t 10 jadda-jadda-infra
 PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 KillMode=none
 Type=forking