Merge pull request #15771 from dcermak/pod-systemd-exit-policy

[systemd] Ensure that podCreateArgs appear last in ExecStartPre=

pkg/systemd/generate/pods.go

@@ -256,14 +256,14 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (
 	return &info, nil
 }
 
-// Unless already specified, the pod's exit policy to "stop".
+// Determine whether the command array includes an exit-policy setting
-func setPodExitPolicy(cmd []string) []string {
+func hasPodExitPolicy(cmd []string) bool {
 	for _, arg := range cmd {
 		if strings.HasPrefix(arg, "--exit-policy=") || arg == "--exit-policy" {
-			return cmd
+			return true
 		}
 	}
-	return append(cmd, "--exit-policy=stop")
+	return false
 }
 
 // executePodTemplate executes the pod template on the specified podInfo.  Note
@@ -364,8 +364,10 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 			podCreateArgs = append(podCreateArgs, "--replace")
 		}
 
+		if !hasPodExitPolicy(append(startCommand, podCreateArgs...)) {
+			startCommand = append(startCommand, "--exit-policy=stop")
+		}
 		startCommand = append(startCommand, podCreateArgs...)
-		startCommand = setPodExitPolicy(startCommand)
 		startCommand = escapeSystemdArguments(startCommand)
 
 		info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}"

pkg/systemd/generate/pods_test.go

@@ -7,25 +7,26 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestSetPodExitPolicy(t *testing.T) {
+func TestHasPodExitPolicy(t *testing.T) {
 	tests := []struct {
-		input, expected []string
+		input    []string
+		expected bool
 	}{
 		{
 			[]string{"podman", "pod", "create"},
-			[]string{"podman", "pod", "create", "--exit-policy=stop"},
+			false,
 		},
 		{
 			[]string{"podman", "pod", "create", "--exit-policy=continue"},
-			[]string{"podman", "pod", "create", "--exit-policy=continue"},
+			true,
 		},
 		{
 			[]string{"podman", "pod", "create", "--exit-policy", "continue"},
-			[]string{"podman", "pod", "create", "--exit-policy", "continue"},
+			true,
 		},
 	}
 	for _, test := range tests {
-		assert.Equalf(t, test.expected, setPodExitPolicy(test.input), "%v", test.input)
+		assert.Equalf(t, test.expected, hasPodExitPolicy(test.input), "%v", test.input)
 	}
 }
 
@@ -226,6 +227,33 @@ ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra
 PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 Type=forking
 
+[Install]
+WantedBy=default.target
+`
+	podNoExplicitName := `# pod-123abc.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman pod-123abc.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network-online.target
+After=network-online.target
+RequiresMountsFor=/var/run/containers/storage
+Requires=
+Before=
+
+[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 --exit-policy=stop foo
+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
+Type=forking
+
 [Install]
 WantedBy=default.target
 `
@@ -274,7 +302,7 @@ 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 --exit-policy=stop
+ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --exit-policy=stop --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
@@ -302,7 +330,7 @@ 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 --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 --exit-policy=stop
+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 --exit-policy=stop --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
 ExecStop=/usr/bin/podman --events-backend none --runroot /root pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10
 ExecStopPost=/usr/bin/podman --events-backend none --runroot /root pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id
@@ -330,7 +358,7 @@ 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 --replace --exit-policy=stop
+ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --exit-policy=stop --name foo --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
@@ -483,6 +511,23 @@ WantedBy=default.target
 			false,
 			false,
 		},
+		{"pod without --name",
+			podInfo{
+				Executable:    "/usr/bin/podman",
+				ServiceName:   "pod-123abc",
+				InfraNameOrID: "jadda-jadda-infra",
+				PIDFile:       "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:   10,
+				PodmanVersion: "CI",
+				GraphRoot:     "/var/lib/containers/storage",
+				RunRoot:       "/var/run/containers/storage",
+				CreateCommand: []string{"podman", "pod", "create", "foo"},
+			},
+			podNoExplicitName,
+			true,
+			false,
+			false,
+		},
 		{"pod restartSec",
 			podInfo{
 				Executable:       "/usr/bin/podman",

test/e2e/generate_systemd_test.go

@@ -562,7 +562,7 @@ var _ = Describe("Podman generate systemd", func() {
 		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("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("pod create --infra-conmon-pidfile %t/pod-foo.pid --pod-id-file %t/pod-foo.pod-id --exit-policy=stop --name foo"))
 		Expect(session.OutputToString()).To(ContainSubstring("ExecStartPre=/bin/rm -f %t/pod-foo.pid %t/pod-foo.pod-id"))
 		Expect(session.OutputToString()).To(ContainSubstring("pod stop --ignore --pod-id-file %t/pod-foo.pod-id -t 10"))
 		Expect(session.OutputToString()).To(ContainSubstring("pod rm --ignore -f --pod-id-file %t/pod-foo.pod-id"))