Skip to content

Commit

Permalink
Merge pull request #11312 from vrothberg/fix-11304
Browse files Browse the repository at this point in the history
generate systemd: custom stop signal
  • Loading branch information
openshift-merge-robot authored Aug 24, 2021
2 parents eb9d731 + 70801b3 commit e20ec47
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 25 deletions.
6 changes: 3 additions & 3 deletions pkg/systemd/generate/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 22 additions & 1 deletion pkg/systemd/generate/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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}}}}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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()
Expand Down
30 changes: 15 additions & 15 deletions pkg/systemd/generate/containers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand 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
Expand 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
Expand 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
Expand 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
Expand 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
Expand 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
Expand All @@ -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
Expand 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
Expand 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
Expand 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
Expand 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
Expand 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
Expand 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
Expand 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
Expand Down
2 changes: 1 addition & 1 deletion pkg/systemd/generate/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 5 additions & 5 deletions pkg/systemd/generate/pods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions test/system/250-systemd.bats
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,17 @@ function service_cleanup() {
service_cleanup
}

@test "podman generate systemd - stop-signal" {
cname=$(random_string)
run_podman create --name $cname --stop-signal=42 $IMAGE
run_podman generate systemd --new $cname
is "$output" ".*KillSignal=42.*" "KillSignal is set"

# Regression test for #11304: systemd wants a custom stop-signal.
run_podman rm -f $cname
run_podman create --name $cname --systemd=true $IMAGE systemd
run_podman generate systemd --new $cname
is "$output" ".*KillSignal=37.*" "KillSignal is set"
}

# vim: filetype=sh

0 comments on commit e20ec47

Please sign in to comment.