[systemd] Ensure that podCreateArgs appear last in ExecStartPre=

When creating a new pod without the `--name` flag, e.g.: `podman pod create foobar` it will get the name `foobar` implicitly and this will be recorded as the in the `podCreateArgs`. Unfortunately, the implicit name only works if it appears as the **last** argument of the startup command. With 6e2e3a78ed1d05ee5f23f65b814e8135021961dd we started appending the pod security policy to the startCommand, resulting in the following `ExecStartPre=` line: ``` /usr/bin/podman pod create --infra-conmon-pidfile %t/pod-foobar.pid --pod-id-file %t/pod-foobar.pod-id foobar --exit-policy=stop ``` This fails to launch, as the `pod create` command expects only a single non-flag parameter, but it assumes that `exit-policy=stop` is a second and terminates immediately instead. This fixes https://github.com/containers/podman/issues/15592 Signed-off-by: Dan Čermák <dcermak@suse.com>
2025-06-21 01:19:15 +08:00 · 2022-09-13 14:43:47 +02:00
parent c64388728f
commit 0aedddd3b3
3 changed files with 62 additions and 15 deletions
--- a/pkg/systemd/generate/pods.go
+++ b/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".
-func setPodExitPolicy(cmd []string) []string {
+// Determine whether the command array includes an exit-policy setting
+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}}}}"
--- a/pkg/systemd/generate/pods_test.go
+++ b/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",
--- a/test/e2e/generate_systemd_test.go
+++ b/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"))