summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorValentin Rothberg <rothberg@redhat.com>2021-01-05 15:48:36 +0100
committerValentin Rothberg <rothberg@redhat.com>2021-01-05 16:37:24 +0100
commit219c69ef03dc86aa92b6134a03f22f416b954d86 (patch)
tree23b6d593c8bc76a0080c8b017ce045c205662284
parent1b9366d650200d0f2029d628fa00d1fd318631aa (diff)
downloadpodman-219c69ef03dc86aa92b6134a03f22f416b954d86.tar.gz
podman-219c69ef03dc86aa92b6134a03f22f416b954d86.tar.bz2
podman-219c69ef03dc86aa92b6134a03f22f416b954d86.zip
generate systemd: do not set `KillMode`
`KillMode=none` has been deprecated in systemd and is now throwing big warnings when being used. Users have reported the issues upstream (see #8615) and on the mailing list. This deprecation was mainly motivated by an abusive use of third-party vendors causing all kinds of undesired side-effects. For instance, busy mounts that delay reboot. After talking to the systemd team, we came up with the following plan: **Short term**: we can use TimeoutStopSec and remove KillMode=none which will default to cgroup. **Long term**: we want to change the type to sdnotify. The plumbing for Podman is done but we need it for conmon. Once sdnotify is working, we can get rid of the pidfile handling etc. and let Podman handle it. Michal Seklatar came up with a nice idea that Podman increase the time out on demand. That's a much cleaner way than hard-coding the time out in the unit as suggest in the short-term solution. This change is executing the short-term plan and sets a minimum timeout of 60 seconds. User-specified timeouts are added to that. Fixes: #8615 Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
-rw-r--r--pkg/systemd/generate/common.go5
-rw-r--r--pkg/systemd/generate/containers.go6
-rw-r--r--pkg/systemd/generate/containers_test.go43
-rw-r--r--pkg/systemd/generate/pods.go6
-rw-r--r--pkg/systemd/generate/pods_test.go15
-rw-r--r--test/e2e/generate_systemd_test.go3
6 files changed, 46 insertions, 32 deletions
diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
index 52a214883..fb921cd72 100644
--- 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
index d84125fc7..cfa02dc9d 100644
--- 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,6 +76,7 @@ After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $v
[Service]
Environment={{.EnvVariable}}=%n
Restart={{.RestartPolicy}}
+TimeoutStopSec={{.TimeoutStopSec}}
{{- if .ExecStartPre}}
ExecStartPre={{.ExecStartPre}}
{{- end}}
@@ -81,7 +84,6 @@ 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
index d27062ef3..b9fb8fee6 100644
--- 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
index 3c57b03fb..fc582e42a 100644
--- 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
index 7f1f63b7e..6d925ecd2 100644
--- 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
index 765844265..3f059300b 100644
--- 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() {