Merge pull request #8889 from vrothberg/run-1138

generate systemd: do not set `KillMode`
containers · Jan 5, 2021 · 1f59276 · 1f59276
2 parents b84b7c8 + 219c69e
commit 1f59276
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 32 deletions.
diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
@@ -11,6 +11,11 @@ import (
 // is set to the unit's (unique) name.
 const EnvVariable = "PODMAN_SYSTEMD_UNIT"
 
+// 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.
+const minTimeoutStopSec = 60
+
 // RestartPolicies includes all valid restart policies to be used in a unit
 // file.
 var RestartPolicies = []string{"no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", "always"}

diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
@@ -55,6 +55,8 @@ type containerInfo struct {
 	ExecStartPre string
 	// ExecStart of the unit.
 	ExecStart string
+	// TimeoutStopSec of the unit.
+	TimeoutStopSec uint
 	// ExecStop of the unit.
 	ExecStop string
 	// ExecStopPost of the unit.
@@ -74,14 +76,14 @@ After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $v
 [Service]
 Environment={{.EnvVariable}}=%n
 Restart={{.RestartPolicy}}
+TimeoutStopSec={{.TimeoutStopSec}}
 {{- if .ExecStartPre}}
 ExecStartPre={{.ExecStartPre}}
 {{- end}}
 ExecStart={{.ExecStart}}
 ExecStop={{.ExecStop}}
 ExecStopPost={{.ExecStopPost}}
 PIDFile={{.PIDFile}}
-KillMode=none
 Type=forking
 
 [Install]
@@ -255,6 +257,8 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		info.ExecStopPost = "{{.Executable}} rm --ignore -f --cidfile {{.ContainerIDFile}}"
 	}
 
+	info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
+
 	if info.PodmanVersion == "" {
 		info.PodmanVersion = version.Version.String()
 	}

diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/containers/podman/v2/pkg/domain/entities"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestValidateRestartPolicyContainer(t *testing.T) {
@@ -48,11 +49,11 @@ After=network-online.target
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=82
 ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
-ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
-ExecStopPost=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
+ExecStop=/usr/bin/podman stop -t 22 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
+ExecStopPost=/usr/bin/podman stop -t 22 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
 PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -71,11 +72,11 @@ After=network-online.target
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=70
 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
 
 [Install]
@@ -96,11 +97,11 @@ After=a.service b.service c.service pod.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=70
 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
 
 [Install]
@@ -119,12 +120,12 @@ After=network-online.target
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=70
 ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
 ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --replace --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN "foo=arg \"with \" space"
-ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
 ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
 PIDFile=%t/jadda-jadda.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -143,12 +144,12 @@ After=network-online.target
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=70
 ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
 ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --replace -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
-ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
 ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
 PIDFile=%t/jadda-jadda.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -167,12 +168,12 @@ After=network-online.target
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=70
 ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
 ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --pod-id-file /tmp/pod-foobar.pod-id-file --replace -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
-ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
 ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
 PIDFile=%t/jadda-jadda.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -191,12 +192,12 @@ After=network-online.target
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=70
 ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
 ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --replace --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
-ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
 ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
 PIDFile=%t/jadda-jadda.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -215,12 +216,12 @@ After=network-online.target
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=70
 ExecStartPre=/bin/rm -f %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id
 ExecStart=/usr/bin/podman run --conmon-pidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id --cgroups=no-conmon -d awesome-image:latest
 ExecStop=/usr/bin/podman stop --ignore --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id -t 10
 ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id
 PIDFile=%t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -242,7 +243,7 @@ WantedBy=multi-user.target default.target
 				ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
 				RestartPolicy:     "always",
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:       10,
+				StopTimeout:       22,
 				PodmanVersion:     "CI",
 				EnvVariable:       EnvVariable,
 			},
@@ -302,7 +303,7 @@ WantedBy=multi-user.target default.target
 				ContainerNameOrID: "jadda-jadda",
 				RestartPolicy:     "always",
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:       42,
+				StopTimeout:       10,
 				PodmanVersion:     "CI",
 				CreateCommand:     []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN", "foo=arg \"with \" space"},
 				EnvVariable:       EnvVariable,
@@ -318,7 +319,7 @@ WantedBy=multi-user.target default.target
 				ContainerNameOrID: "jadda-jadda",
 				RestartPolicy:     "always",
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:       42,
+				StopTimeout:       10,
 				PodmanVersion:     "CI",
 				CreateCommand:     []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"},
 				EnvVariable:       EnvVariable,
@@ -334,7 +335,7 @@ WantedBy=multi-user.target default.target
 				ContainerNameOrID: "jadda-jadda",
 				RestartPolicy:     "always",
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:       42,
+				StopTimeout:       10,
 				PodmanVersion:     "CI",
 				CreateCommand:     []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"},
 				EnvVariable:       EnvVariable,
@@ -353,7 +354,7 @@ WantedBy=multi-user.target default.target
 				ContainerNameOrID: "jadda-jadda",
 				RestartPolicy:     "always",
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:       42,
+				StopTimeout:       10,
 				PodmanVersion:     "CI",
 				CreateCommand:     []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"},
 				EnvVariable:       EnvVariable,
@@ -390,9 +391,7 @@ WantedBy=multi-user.target default.target
 				t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr)
 				return
 			}
-			if got != test.want {
-				t.Errorf("CreateContainerSystemdUnit() = \n%v\n---------> want\n%v", got, test.want)
-			}
+			assert.Equal(t, test.want, got)
 		})
 	}
 }
diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
@@ -60,6 +60,8 @@ type podInfo struct {
 	ExecStartPre2 string
 	// ExecStart of the unit.
 	ExecStart string
+	// TimeoutStopSec of the unit.
+	TimeoutStopSec uint
 	// ExecStop of the unit.
 	ExecStop string
 	// ExecStopPost of the unit.
@@ -72,6 +74,7 @@ Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{
 [Service]
 Environment={{.EnvVariable}}=%n
 Restart={{.RestartPolicy}}
+TimeoutStopSec={{.TimeoutStopSec}}
 {{- if .ExecStartPre1}}
 ExecStartPre={{.ExecStartPre1}}
 {{- end}}
@@ -82,7 +85,6 @@ ExecStart={{.ExecStart}}
 ExecStop={{.ExecStop}}
 ExecStopPost={{.ExecStopPost}}
 PIDFile={{.PIDFile}}
-KillMode=none
 Type=forking
 
 [Install]
@@ -298,6 +300,8 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 		info.ExecStop = "{{.Executable}} pod stop --ignore --pod-id-file {{.PodIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}"
 		info.ExecStopPost = "{{.Executable}} pod rm --ignore -f --pod-id-file {{.PodIDFile}}"
 	}
+	info.TimeoutStopSec = minTimeoutStopSec + 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
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/containers/podman/v2/pkg/domain/entities"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestValidateRestartPolicyPod(t *testing.T) {
@@ -50,11 +51,11 @@ Before=container-1.service container-2.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=always
+TimeoutStopSec=102
 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
+ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra
+ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra
 PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -75,13 +76,13 @@ Before=container-1.service container-2.service
 [Service]
 Environment=PODMAN_SYSTEMD_UNIT=%n
 Restart=on-failure
+TimeoutStopSec=70
 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
 ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10
 ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id
 PIDFile=%t/pod-123abc.pid
-KillMode=none
 Type=forking
 
 [Install]
@@ -102,7 +103,7 @@ WantedBy=multi-user.target default.target
 				InfraNameOrID:    "jadda-jadda-infra",
 				RestartPolicy:    "always",
 				PIDFile:          "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:      10,
+				StopTimeout:      42,
 				PodmanVersion:    "CI",
 				RequiredServices: []string{"container-1", "container-2"},
 			},
@@ -139,9 +140,7 @@ WantedBy=multi-user.target default.target
 				t.Errorf("CreatePodSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr)
 				return
 			}
-			if got != test.want {
-				t.Errorf("CreatePodSystemdUnit() = \n%v\n---------> want\n%v", got, test.want)
-			}
+			assert.Equal(t, test.want, got)
 		})
 	}
 }
diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go
@@ -74,6 +74,9 @@ var _ = Describe("Podman generate systemd", func() {
 
 		found, _ := session.GrepString(" stop -t 1234 ")
 		Expect(found).To(BeTrue())
+
+		found, _ = session.GrepString("TimeoutStopSec=1294")
+		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd", func() {