Merge pull request #11312 from vrothberg/fix-11304

generate systemd: custom stop signal

5 files changed, 46 insertions, 25 deletions

diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
index 45e12014a..f2092c0ae 100644
--- a/pkg/systemd/generate/common.go
+++ b/pkg/systemd/generate/common.go
@@ -8,9 +8,9 @@ import (
 	"github.com/pkg/errors"
 )
 
-// minTimeoutStopSec is the minimal stop timeout for generated systemd units.
-// Once exceeded, processes of the services are killed and the cgroup(s) are
-// cleaned up.
+// minTimeoutStopSec is the minimal stop timeout for generated systemd units
+// without --new.  Once exceeded, processes of the services are killed and the
+// cgroup(s) are cleaned up.
 const minTimeoutStopSec = 60
 
 // validateRestartPolicy checks that the user-provided policy is valid.
diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
index 78b81b54b..66cf5ca44 100644
--- a/pkg/systemd/generate/containers.go
+++ b/pkg/systemd/generate/containers.go
@@ -16,6 +16,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
+	"golang.org/x/sys/unix"
 )
 
 // containerInfo contains data required for generating a container's systemd
@@ -32,6 +33,8 @@ type containerInfo struct {
 	// StopTimeout sets the timeout Podman waits before killing the container
 	// during service stop.
 	StopTimeout uint
+	// KillSignal of the container.
+	KillSignal string
 	// RestartPolicy of the systemd unit (e.g., no, on-failure, always).
 	RestartPolicy string
 	// PIDFile of the service. Required for forking services. Must point to the
@@ -102,6 +105,9 @@ Environment={{{{- range $index, $value := .ExtraEnvs -}}}}{{{{if $index}}}} {{{{
 {{{{- end}}}}
 Restart={{{{.RestartPolicy}}}}
 TimeoutStopSec={{{{.TimeoutStopSec}}}}
+{{{{- if .KillSignal}}}}
+KillSignal={{{{.KillSignal}}}}
+{{{{- end}}}}
 {{{{- if .ExecStartPre}}}}
 ExecStartPre={{{{.ExecStartPre}}}}
 {{{{- end}}}}
@@ -184,6 +190,13 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste
 		containerEnv:      envs,
 	}
 
+	// Set a custom kill signal for non SIGTERM (already default in
+	// systemd) signals.
+	stopSignal := ctr.StopSignal()
+	if stopSignal != uint(unix.SIGTERM) {
+		info.KillSignal = fmt.Sprintf("%d", stopSignal)
+	}
+
 	return &info, nil
 }
 
@@ -359,7 +372,15 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		info.ExecStart = strings.Join(startCommand, " ")
 	}
 
-	info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
+	info.TimeoutStopSec = info.StopTimeout
+
+	// For units without --new add an additional 60 seconds to the stop
+	// timeout to make sure that Podman stop has enough time to properly
+	// shutdown and cleanup the container before systemd starts to nuke
+	// everything in the cgroup.
+	if !options.New {
+		info.TimeoutStopSec += minTimeoutStopSec
+	}
 
 	if info.PodmanVersion == "" {
 		info.PodmanVersion = version.Version.String()
diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
index 6141950d0..ef3bbb5ae 100644
--- a/pkg/systemd/generate/containers_test.go
+++ b/pkg/systemd/generate/containers_test.go
@@ -129,7 +129,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman container run --cgroups=no-conmon --rm --sdnotify=conmon -d --replace --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN "foo=arg \"with \" space"
 Type=notify
 NotifyAccess=all
@@ -151,7 +151,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman container run --cgroups=no-conmon --rm -d --replace --sdnotify=container --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN "foo=arg \"with \" space"
 Type=notify
 NotifyAccess=all
@@ -173,7 +173,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon --replace -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
 Type=notify
 NotifyAccess=all
@@ -195,7 +195,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --pod-id-file %t/pod-foobar.pod-id-file --sdnotify=conmon --replace -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
 Type=notify
 NotifyAccess=all
@@ -217,7 +217,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon --replace --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
 Type=notify
 NotifyAccess=all
@@ -239,7 +239,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon -d awesome-image:latest
 Type=notify
 NotifyAccess=all
@@ -262,7 +262,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=102
+TimeoutStopSec=42
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon ` +
 			detachparam +
 			` awesome-image:latest
@@ -288,7 +288,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=102
+TimeoutStopSec=42
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon -d --replace --name test -p 80:80 awesome-image:latest somecmd --detach=false
 Type=notify
 NotifyAccess=all
@@ -310,7 +310,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=102
+TimeoutStopSec=42
 ExecStart=/usr/bin/podman --events-backend none --runroot /root run --cgroups=no-conmon --rm --sdnotify=conmon -d awesome-image:latest
 Type=notify
 NotifyAccess=all
@@ -332,7 +332,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman container run --cgroups=no-conmon --rm --sdnotify=conmon -d awesome-image:latest
 Type=notify
 NotifyAccess=all
@@ -354,7 +354,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon -d --replace --name test --log-driver=journald --log-opt=tag={{.Name}} awesome-image:latest
 Type=notify
 NotifyAccess=all
@@ -376,7 +376,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon -d --replace --name test awesome-image:latest sh -c "kill $$$$ && echo %%\\"
 Type=notify
 NotifyAccess=all
@@ -398,7 +398,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon -d --conmon-pidfile=foo --cidfile=foo awesome-image:latest podman run --cgroups=foo --conmon-pidfile=foo --cidfile=foo alpine
 Type=notify
 NotifyAccess=all
@@ -420,7 +420,7 @@ RequiresMountsFor=/var/run/containers/storage
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --pod-id-file %t/pod-foobar.pod-id-file --sdnotify=conmon -d --conmon-pidfile=foo --cidfile=foo awesome-image:latest podman run --cgroups=foo --conmon-pidfile=foo --cidfile=foo --pod-id-file /tmp/pod-foobar.pod-id-file alpine
 Type=notify
 NotifyAccess=all
@@ -443,7 +443,7 @@ RequiresMountsFor=/var/run/containers/storage
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Environment=FOO=abc "BAR=my test" USER=%%a
 Restart=always
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStart=/usr/bin/podman run --cgroups=no-conmon --rm --sdnotify=conmon -d --env FOO --env=BAR --env=MYENV=2 -e USER awesome-image:latest
 Type=notify
 NotifyAccess=all
diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
index 1b92649e8..d49828884 100644
--- a/pkg/systemd/generate/pods.go
+++ b/pkg/systemd/generate/pods.go
@@ -323,7 +323,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 		info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod stop --ignore --pod-id-file {{{{.PodIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}"
 		info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod rm --ignore -f --pod-id-file {{{{.PodIDFile}}}}"
 	}
-	info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
+	info.TimeoutStopSec = info.StopTimeout
 
 	if info.PodmanVersion == "" {
 		info.PodmanVersion = version.Version.String()
diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go
index 4b8a9ffd5..49a4ed23f 100644
--- a/pkg/systemd/generate/pods_test.go
+++ b/pkg/systemd/generate/pods_test.go
@@ -54,7 +54,7 @@ Before=container-1.service container-2.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
-TimeoutStopSec=102
+TimeoutStopSec=42
 ExecStart=/usr/bin/podman start jadda-jadda-infra
 ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra
 ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra
@@ -82,7 +82,7 @@ Before=container-1.service container-2.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=on-failure
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id
 ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo "bar=arg with space" --replace
 ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id
@@ -110,7 +110,7 @@ Before=container-1.service container-2.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=on-failure
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id
 ExecStartPre=/usr/bin/podman --events-backend none --runroot /root pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo "bar=arg with space" --replace
 ExecStart=/usr/bin/podman --events-backend none --runroot /root pod start --pod-id-file %t/pod-123abc.pod-id
@@ -138,7 +138,7 @@ Before=container-1.service container-2.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=on-failure
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id
 ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo --replace
 ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id
@@ -166,7 +166,7 @@ Before=container-1.service container-2.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=on-failure
-TimeoutStopSec=70
+TimeoutStopSec=10
 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id
 ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo --label key={{someval}} --replace
 ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id