generate systemd: fix pod dependencies

Change the dependencies from a pod unit to its associated container units from `Requires` to `Wants` to prevent the entire pod from transitioning to a failed state. Restart policies for individual containers can be configured separately. Also make sure that the pod's RunRoot is always set. Fixes: containers#14546 Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg · Sep 13, 2022 · fe04c86 · fe04c86
1 parent 1b3e863
commit fe04c86
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 29 deletions.
diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
@@ -92,7 +92,7 @@ type podInfo struct {
 	Requires []string
 }
 
-const podTemplate = headerTemplate + `Requires={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
+const podTemplate = headerTemplate + `Wants={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
 Before={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
 {{{{- if or .Wants .After .Requires }}}}
 
@@ -252,6 +252,7 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (
 		StopTimeout:       stopTimeout,
 		GenerateTimestamp: true,
 		CreateCommand:     createCommand,
+		RunRoot:           infraCtr.Runtime().RunRoot(),
 	}
 	return &info, nil
 }

diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go
@@ -70,7 +70,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 [Service]
@@ -98,7 +98,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 [Service]
@@ -124,7 +124,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 # User-defined dependencies
@@ -152,7 +152,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 # User-defined dependencies
@@ -180,7 +180,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 # User-defined dependencies
@@ -208,7 +208,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 # User-defined dependencies
@@ -239,7 +239,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 [Service]
@@ -266,7 +266,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 [Service]
@@ -294,7 +294,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 [Service]
@@ -322,7 +322,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 [Service]
@@ -350,7 +350,7 @@ Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
 RequiresMountsFor=/var/run/containers/storage
-Requires=container-1.service container-2.service
+Wants=container-1.service container-2.service
 Before=container-1.service container-2.service
 
 [Service]

diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go
@@ -3,6 +3,7 @@ package integration
 import (
 	"io/ioutil"
 	"os"
+	"strings"
 
 	. "github.com/containers/podman/v4/test/utils"
 	. "github.com/onsi/ginkgo"
@@ -220,19 +221,20 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session).Should(Exit(0))
 
 		// Grepping the output (in addition to unit tests)
-		Expect(session.OutputToString()).To(ContainSubstring("# pod-foo.service"))
-		Expect(session.OutputToString()).To(ContainSubstring("Requires=container-foo-1.service container-foo-2.service"))
-		Expect(session.OutputToString()).To(ContainSubstring("# container-foo-1.service"))
-		Expect(session.OutputToString()).To(ContainSubstring(" start foo-1"))
-		Expect(session.OutputToString()).To(ContainSubstring("-infra")) // infra container
-		Expect(session.OutputToString()).To(ContainSubstring("# container-foo-2.service"))
-		Expect(session.OutputToString()).To(ContainSubstring(" stop -t 42 foo-2"))
-		Expect(session.OutputToString()).To(ContainSubstring("BindsTo=pod-foo.service"))
-		Expect(session.OutputToString()).To(ContainSubstring("PIDFile="))
-		Expect(session.OutputToString()).To(ContainSubstring("/userdata/conmon.pid"))
-
+		output := session.OutputToString()
+		Expect(output).To(ContainSubstring("# pod-foo.service"))
+		Expect(output).To(ContainSubstring("Wants=container-foo-1.service container-foo-2.service"))
+		Expect(output).To(ContainSubstring("# container-foo-1.service"))
+		Expect(output).To(ContainSubstring(" start foo-1"))
+		Expect(output).To(ContainSubstring("-infra")) // infra container
+		Expect(output).To(ContainSubstring("# container-foo-2.service"))
+		Expect(output).To(ContainSubstring(" stop -t 42 foo-2"))
+		Expect(output).To(ContainSubstring("BindsTo=pod-foo.service"))
+		Expect(output).To(ContainSubstring("PIDFile="))
+		Expect(output).To(ContainSubstring("/userdata/conmon.pid"))
+		Expect(strings.Count(output, "RequiresMountsFor="+podmanTest.RunRoot)).To(Equal(3))
 		// The podman commands in the unit should not contain the root flags
-		Expect(session.OutputToString()).ToNot(ContainSubstring(" --runroot"))
+		Expect(output).ToNot(ContainSubstring(" --runroot"))
 	})
 
 	It("podman generate systemd pod --name --files", func() {
@@ -468,7 +470,7 @@ var _ = Describe("Podman generate systemd", func() {
 
 		// Grepping the output (in addition to unit tests)
 		Expect(session.OutputToString()).To(ContainSubstring("# p-foo.service"))
-		Expect(session.OutputToString()).To(ContainSubstring("Requires=container-foo-1.service container-foo-2.service"))
+		Expect(session.OutputToString()).To(ContainSubstring("Wants=container-foo-1.service container-foo-2.service"))
 		Expect(session.OutputToString()).To(ContainSubstring("# container-foo-1.service"))
 		Expect(session.OutputToString()).To(ContainSubstring("BindsTo=p-foo.service"))
 	})
@@ -492,7 +494,7 @@ var _ = Describe("Podman generate systemd", func() {
 
 		// Grepping the output (in addition to unit tests)
 		Expect(session.OutputToString()).To(ContainSubstring("# p_foo.service"))
-		Expect(session.OutputToString()).To(ContainSubstring("Requires=con_foo-1.service con_foo-2.service"))
+		Expect(session.OutputToString()).To(ContainSubstring("Wants=con_foo-1.service con_foo-2.service"))
 		Expect(session.OutputToString()).To(ContainSubstring("# con_foo-1.service"))
 		Expect(session.OutputToString()).To(ContainSubstring("# con_foo-2.service"))
 		Expect(session.OutputToString()).To(ContainSubstring("BindsTo=p_foo.service"))
@@ -518,7 +520,7 @@ var _ = Describe("Podman generate systemd", func() {
 
 		// Grepping the output (in addition to unit tests)
 		Expect(session1.OutputToString()).To(ContainSubstring("# foo.service"))
-		Expect(session1.OutputToString()).To(ContainSubstring("Requires=container-foo-1.service container-foo-2.service"))
+		Expect(session1.OutputToString()).To(ContainSubstring("Wants=container-foo-1.service container-foo-2.service"))
 		Expect(session1.OutputToString()).To(ContainSubstring("# container-foo-1.service"))
 		Expect(session1.OutputToString()).To(ContainSubstring("BindsTo=foo.service"))
 
@@ -529,7 +531,7 @@ var _ = Describe("Podman generate systemd", func() {
 
 		// Grepping the output (in addition to unit tests)
 		Expect(session2.OutputToString()).To(ContainSubstring("# foo.service"))
-		Expect(session2.OutputToString()).To(ContainSubstring("Requires=foo-1.service foo-2.service"))
+		Expect(session2.OutputToString()).To(ContainSubstring("Wants=foo-1.service foo-2.service"))
 		Expect(session2.OutputToString()).To(ContainSubstring("# foo-1.service"))
 		Expect(session2.OutputToString()).To(ContainSubstring("# foo-2.service"))
 		Expect(session2.OutputToString()).To(ContainSubstring("BindsTo=foo.service"))
@@ -560,7 +562,7 @@ var _ = Describe("Podman generate systemd", func() {
 
 		// Grepping the output (in addition to unit tests)
 		Expect(session.OutputToString()).To(ContainSubstring("# pod-foo.service"))
-		Expect(session.OutputToString()).To(ContainSubstring("Requires=container-foo-1.service container-foo-2.service"))
+		Expect(session.OutputToString()).To(ContainSubstring("Wants=container-foo-1.service container-foo-2.service"))
 		Expect(session.OutputToString()).To(ContainSubstring("BindsTo=pod-foo.service"))
 		Expect(session.OutputToString()).To(ContainSubstring("pod create --infra-conmon-pidfile %t/pod-foo.pid --pod-id-file %t/pod-foo.pod-id --name foo"))
 		Expect(session.OutputToString()).To(ContainSubstring("ExecStartPre=/bin/rm -f %t/pod-foo.pid %t/pod-foo.pod-id"))