Make podman generate systemd --new flag parsing more robust

First, use the pflag library to parse the flags. With this we can
handle all corner cases such as -td or --detach=false.

Second, preserve the root args with --new. They are used for all podman
commands in the unit file. (e.g. podman --root /tmp run alpine)

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>

pkg/systemd/generate/common.go

@@ -71,3 +71,30 @@ func quoteArguments(command []string) []string {
 	}
 	return command
 }
+
+func removeDetachArg(args []string, argCount int) []string {
+	// "--detach=false" could also be in the container entrypoint
+	// split them off so we do not remove it there
+	realArgs := args[len(args)-argCount:]
+	flagArgs := removeArg("-d=false", args[:len(args)-argCount])
+	flagArgs = removeArg("--detach=false", flagArgs)
+	return append(flagArgs, realArgs...)
+}
+
+func removeReplaceArg(args []string, argCount int) []string {
+	// "--replace=false" could also be in the container entrypoint
+	// split them off so we do not remove it there
+	realArgs := args[len(args)-argCount:]
+	flagArgs := removeArg("--replace=false", args[:len(args)-argCount])
+	return append(flagArgs, realArgs...)
+}
+
+func removeArg(arg string, args []string) []string {
+	newArgs := []string{}
+	for _, a := range args {
+		if a != arg {
+			newArgs = append(newArgs, a)
+		}
+	}
+	return newArgs
+}

pkg/systemd/generate/containers.go

@@ -14,6 +14,7 @@ import (
 	"github.com/containers/podman/v2/version"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/pflag"
 )
 
 // containerInfo contains data required for generating a container's systemd
@@ -44,6 +45,9 @@ type containerInfo struct {
 	// Executable is the path to the podman executable. Will be auto-filled if
 	// left empty.
 	Executable string
+	// RootFlags contains the root flags which were used to create the container
+	// Only used with --new
+	RootFlags string
 	// TimeStamp at the time of creating the unit file. Will be set internally.
 	TimeStamp string
 	// CreateCommand is the full command plus arguments of the process the
@@ -185,22 +189,30 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		info.ContainerIDFile = "%t/" + info.ServiceName + ".ctr-id"
 		// The create command must at least have three arguments:
 		// 	/usr/bin/podman run $IMAGE
-		index := 2
+		index := 0
-		if info.CreateCommand[1] == "container" {
+		for i, arg := range info.CreateCommand {
-			index = 3
+			if arg == "run" || arg == "create" {
+				index = i + 1
+				break
+			}
 		}
-		if len(info.CreateCommand) < index+1 {
+		if index == 0 {
 			return "", errors.Errorf("container's create command is too short or invalid: %v", info.CreateCommand)
 		}
 		// We're hard-coding the first five arguments and append the
 		// CreateCommand with a stripped command and subcommand.
-		startCommand := []string{
+		startCommand := []string{info.Executable}
-			info.Executable,
+		if index > 2 {
+			// include root flags
+			info.RootFlags = strings.Join(quoteArguments(info.CreateCommand[1:index-1]), " ")
+			startCommand = append(startCommand, info.CreateCommand[1:index-1]...)
+		}
+		startCommand = append(startCommand,
 			"run",
 			"--conmon-pidfile", "{{.PIDFile}}",
 			"--cidfile", "{{.ContainerIDFile}}",
 			"--cgroups=no-conmon",
-		}
+		)
 		// If the container is in a pod, make sure that the
 		// --pod-id-file is set correctly.
 		if info.pod != nil {
@@ -210,22 +222,26 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		}
 
 		// Presence check for certain flags/options.
-		hasDetachParam := false
+		fs := pflag.NewFlagSet("args", pflag.ContinueOnError)
-		hasNameParam := false
+		fs.ParseErrorsWhitelist.UnknownFlags = true
-		hasReplaceParam := false
+		fs.Usage = func() {}
-		for _, p := range info.CreateCommand[index:] {
+		fs.SetInterspersed(false)
-			switch p {
+		fs.BoolP("detach", "d", false, "")
-			case "--detach", "-d":
+		fs.String("name", "", "")
-				hasDetachParam = true
+		fs.Bool("replace", false, "")
-			case "--name":
+		fs.Parse(info.CreateCommand[index:])
-				hasNameParam = true
+
-			case "--replace":
+		hasDetachParam, err := fs.GetBool("detach")
-				hasReplaceParam = true
+		if err != nil {
-			}
+			return "", err
-			if strings.HasPrefix(p, "--name=") {
-				hasNameParam = true
-			}
 		}
+		hasNameParam := fs.Lookup("name").Changed
+		hasReplaceParam, err := fs.GetBool("replace")
+		if err != nil {
+			return "", err
+		}
+
+		remainingCmd := info.CreateCommand[index:]
 
 		if !hasDetachParam {
 			// Enforce detaching
@@ -240,6 +256,13 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 			// will wait the `podman run` command exit until failed
 			// with timeout error.
 			startCommand = append(startCommand, "-d")
+
+			if fs.Changed("detach") {
+				// this can only happen if --detach=false is set
+				// in that case we need to remove it otherwise we
+				// would overwrite the previous detach arg to false
+				remainingCmd = removeDetachArg(remainingCmd, fs.NArg())
+			}
 		}
 		if hasNameParam && !hasReplaceParam {
 			// Enforce --replace for named containers.  This will
@@ -247,14 +270,21 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 			// start after system crashes (see
 			// github.com/containers/podman/issues/5485).
 			startCommand = append(startCommand, "--replace")
+
+			if fs.Changed("replace") {
+				// this can only happen if --replace=false is set
+				// in that case we need to remove it otherwise we
+				// would overwrite the previous replace arg to false
+				remainingCmd = removeReplaceArg(remainingCmd, fs.NArg())
+			}
 		}
-		startCommand = append(startCommand, info.CreateCommand[index:]...)
+		startCommand = append(startCommand, remainingCmd...)
 		startCommand = quoteArguments(startCommand)
 
 		info.ExecStartPre = "/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}"
 		info.ExecStart = strings.Join(startCommand, " ")
-		info.ExecStop = "{{.Executable}} stop --ignore --cidfile {{.ContainerIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}"
+		info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}stop --ignore --cidfile {{.ContainerIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}"
-		info.ExecStopPost = "{{.Executable}} rm --ignore -f --cidfile {{.ContainerIDFile}}"
+		info.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}rm --ignore -f --cidfile {{.ContainerIDFile}}"
 	}
 
 	info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout

pkg/systemd/generate/containers_test.go

@@ -122,9 +122,9 @@ 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"
+ExecStart=/usr/bin/podman container 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 10
+ExecStop=/usr/bin/podman container stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
-ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+ExecStopPost=/usr/bin/podman container rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
 PIDFile=%t/jadda-jadda.pid
 Type=forking
 
@@ -224,6 +224,107 @@ ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/container-639c53578af4d
 PIDFile=%t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid
 Type=forking
 
+[Install]
+WantedBy=multi-user.target default.target
+`
+
+	genGoodNewDetach := func(detachparam string) string {
+		goodNewDetach := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=always
+TimeoutStopSec=102
+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 ` +
+			detachparam +
+			` awesome-image:latest
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42
+ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+PIDFile=%t/jadda-jadda.pid
+Type=forking
+
+[Install]
+WantedBy=multi-user.target default.target
+`
+		return goodNewDetach
+	}
+
+	goodNameNewDetachFalseWithCmd := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=always
+TimeoutStopSec=102
+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 test -p 80:80 awesome-image:latest somecmd --detach=false
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42
+ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+PIDFile=%t/jadda-jadda.pid
+Type=forking
+
+[Install]
+WantedBy=multi-user.target default.target
+`
+
+	goodNewRootFlags := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=always
+TimeoutStopSec=102
+ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
+ExecStart=/usr/bin/podman --events-backend none --runroot /root run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d awesome-image:latest
+ExecStop=/usr/bin/podman --events-backend none --runroot /root stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42
+ExecStopPost=/usr/bin/podman --events-backend none --runroot /root rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+PIDFile=%t/jadda-jadda.pid
+Type=forking
+
+[Install]
+WantedBy=multi-user.target default.target
+`
+
+	goodContainerCreate := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+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 container run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d awesome-image:latest
+ExecStop=/usr/bin/podman container stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
+ExecStopPost=/usr/bin/podman container rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+PIDFile=%t/jadda-jadda.pid
+Type=forking
+
 [Install]
 WantedBy=multi-user.target default.target
 `
@@ -321,7 +422,7 @@ WantedBy=multi-user.target default.target
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
 				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"},
+				CreateCommand:     []string{"I'll get stripped", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"},
 				EnvVariable:       EnvVariable,
 			},
 			goodWithExplicitShortDetachParam,
@@ -337,7 +438,7 @@ WantedBy=multi-user.target default.target
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
 				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"},
+				CreateCommand:     []string{"I'll get stripped", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"},
 				EnvVariable:       EnvVariable,
 				pod: &podInfo{
 					PodIDFile: "/tmp/pod-foobar.pod-id-file",
@@ -356,7 +457,7 @@ WantedBy=multi-user.target default.target
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
 				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"},
+				CreateCommand:     []string{"I'll get stripped", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"},
 				EnvVariable:       EnvVariable,
 			},
 			goodNameNewDetach,
@@ -372,13 +473,141 @@ WantedBy=multi-user.target default.target
 				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
 				StopTimeout:       10,
 				PodmanVersion:     "CI",
-				CreateCommand:     []string{"I'll get stripped", "container", "run", "awesome-image:latest"},
+				CreateCommand:     []string{"I'll get stripped", "run", "awesome-image:latest"},
 				EnvVariable:       EnvVariable,
 			},
 			goodIDNew,
 			true,
 			false,
 		},
+		{"good with explicit detach=true param",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       42,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "run", "--detach=true", "awesome-image:latest"},
+				EnvVariable:       EnvVariable,
+			},
+			genGoodNewDetach("--detach=true"),
+			true,
+			false,
+		},
+		{"good with explicit detach=false param",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       42,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "run", "--detach=false", "awesome-image:latest"},
+				EnvVariable:       EnvVariable,
+			},
+			genGoodNewDetach("-d"),
+			true,
+			false,
+		},
+		{"good with explicit detach=false param",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       42,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "run", "--name", "test", "-p", "80:80", "--detach=false", "awesome-image:latest", "somecmd", "--detach=false"},
+				EnvVariable:       EnvVariable,
+			},
+			goodNameNewDetachFalseWithCmd,
+			true,
+			false,
+		},
+		{"good with multiple detach=false params",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       42,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "run", "--name", "test", "-p", "80:80", "--detach=false", "--detach=false", "awesome-image:latest", "somecmd", "--detach=false"},
+				EnvVariable:       EnvVariable,
+			},
+			goodNameNewDetachFalseWithCmd,
+			true,
+			false,
+		},
+		{"good with multiple shorthand params detach first",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       42,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "run", "-dti", "awesome-image:latest"},
+				EnvVariable:       EnvVariable,
+			},
+			genGoodNewDetach("-dti"),
+			true,
+			false,
+		},
+		{"good with multiple shorthand params detach last",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       42,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "run", "-tid", "awesome-image:latest"},
+				EnvVariable:       EnvVariable,
+			},
+			genGoodNewDetach("-tid"),
+			true,
+			false,
+		},
+		{"good with root flags",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       42,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "--events-backend", "none", "--runroot", "/root", "run", "awesome-image:latest"},
+				EnvVariable:       EnvVariable,
+			},
+			goodNewRootFlags,
+			true,
+			false,
+		},
+		{"good with container create",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       10,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "container", "create", "awesome-image:latest"},
+				EnvVariable:       EnvVariable,
+			},
+			goodContainerCreate,
+			true,
+			false,
+		},
 	}
 	for _, tt := range tests {
 		test := tt

pkg/systemd/generate/pods.go

@@ -14,6 +14,7 @@ import (
 	"github.com/containers/podman/v2/version"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/pflag"
 )
 
 // podInfo contains data required for generating a pod's systemd
@@ -44,6 +45,9 @@ type podInfo struct {
 	// Executable is the path to the podman executable. Will be auto-filled if
 	// left empty.
 	Executable string
+	// RootFlags contains the root flags which were used to create the container
+	// Only used with --new
+	RootFlags string
 	// TimeStamp at the time of creating the unit file. Will be set internally.
 	TimeStamp string
 	// CreateCommand is the full command plus arguments of the process the
@@ -264,7 +268,8 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 			if podCreateIndex == 0 {
 				return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand)
 			}
-			podRootArgs = info.CreateCommand[0 : podCreateIndex-2]
+			podRootArgs = info.CreateCommand[1 : podCreateIndex-1]
+			info.RootFlags = strings.Join(quoteArguments(podRootArgs), " ")
 			podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:])
 		}
 		// We're hard-coding the first five arguments and append the
@@ -277,17 +282,26 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 				"--pod-id-file", "{{.PodIDFile}}"}...)
 
 		// Presence check for certain flags/options.
-		hasNameParam := false
+		fs := pflag.NewFlagSet("args", pflag.ContinueOnError)
-		hasReplaceParam := false
+		fs.ParseErrorsWhitelist.UnknownFlags = true
-		for _, p := range podCreateArgs {
+		fs.Usage = func() {}
-			switch p {
+		fs.SetInterspersed(false)
-			case "--name":
+		fs.String("name", "", "")
-				hasNameParam = true
+		fs.Bool("replace", false, "")
-			case "--replace":
+		fs.Parse(podCreateArgs)
-				hasReplaceParam = true
+
-			}
+		hasNameParam := fs.Lookup("name").Changed
+		hasReplaceParam, err := fs.GetBool("replace")
+		if err != nil {
+			return "", err
 		}
 		if hasNameParam && !hasReplaceParam {
+			if fs.Changed("replace") {
+				// this can only happen if --replace=false is set
+				// in that case we need to remove it otherwise we
+				// would overwrite the previous replace arg to false
+				podCreateArgs = removeReplaceArg(podCreateArgs, fs.NArg())
+			}
 			podCreateArgs = append(podCreateArgs, "--replace")
 		}
 
@@ -296,9 +310,9 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 
 		info.ExecStartPre1 = "/bin/rm -f {{.PIDFile}} {{.PodIDFile}}"
 		info.ExecStartPre2 = strings.Join(startCommand, " ")
-		info.ExecStart = "{{.Executable}} pod start --pod-id-file {{.PodIDFile}}"
+		info.ExecStart = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod start --pod-id-file {{.PodIDFile}}"
-		info.ExecStop = "{{.Executable}} pod stop --ignore --pod-id-file {{.PodIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}"
+		info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}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.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod rm --ignore -f --pod-id-file {{.PodIDFile}}"
 	}
 	info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
 

pkg/systemd/generate/pods_test.go

@@ -85,6 +85,60 @@ ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-
 PIDFile=%t/pod-123abc.pid
 Type=forking
 
+[Install]
+WantedBy=multi-user.target default.target
+`
+
+	podGoodNamedNewWithRootArgs := `# pod-123abc.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman pod-123abc.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+Requires=container-1.service container-2.service
+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 --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
+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
+PIDFile=%t/pod-123abc.pid
+Type=forking
+
+[Install]
+WantedBy=multi-user.target default.target
+`
+
+	podGoodNamedNewWithReplaceFalse := `# pod-123abc.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman pod-123abc.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+Requires=container-1.service container-2.service
+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 --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
+Type=forking
+
 [Install]
 WantedBy=multi-user.target default.target
 `
@@ -106,6 +160,23 @@ WantedBy=multi-user.target default.target
 				StopTimeout:      42,
 				PodmanVersion:    "CI",
 				RequiredServices: []string{"container-1", "container-2"},
+				CreateCommand:    []string{"podman", "pod", "create", "--name", "foo", "bar=arg with space"},
+			},
+			podGood,
+			false,
+			false,
+		},
+		{"pod with root args",
+			podInfo{
+				Executable:       "/usr/bin/podman",
+				ServiceName:      "pod-123abc",
+				InfraNameOrID:    "jadda-jadda-infra",
+				RestartPolicy:    "always",
+				PIDFile:          "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:      42,
+				PodmanVersion:    "CI",
+				RequiredServices: []string{"container-1", "container-2"},
+				CreateCommand:    []string{"podman", "--events-backend", "none", "--runroot", "/root", "pod", "create", "--name", "foo", "bar=arg with space"},
 			},
 			podGood,
 			false,
@@ -127,6 +198,38 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 		},
+		{"pod --new with root args",
+			podInfo{
+				Executable:       "/usr/bin/podman",
+				ServiceName:      "pod-123abc",
+				InfraNameOrID:    "jadda-jadda-infra",
+				RestartPolicy:    "on-failure",
+				PIDFile:          "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:      10,
+				PodmanVersion:    "CI",
+				RequiredServices: []string{"container-1", "container-2"},
+				CreateCommand:    []string{"podman", "--events-backend", "none", "--runroot", "/root", "pod", "create", "--name", "foo", "bar=arg with space"},
+			},
+			podGoodNamedNewWithRootArgs,
+			true,
+			false,
+		},
+		{"pod --new with --replace=false",
+			podInfo{
+				Executable:       "/usr/bin/podman",
+				ServiceName:      "pod-123abc",
+				InfraNameOrID:    "jadda-jadda-infra",
+				RestartPolicy:    "on-failure",
+				PIDFile:          "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:      10,
+				PodmanVersion:    "CI",
+				RequiredServices: []string{"container-1", "container-2"},
+				CreateCommand:    []string{"podman", "pod", "create", "--name", "foo", "--replace=false"},
+			},
+			podGoodNamedNewWithReplaceFalse,
+			true,
+			false,
+		},
 	}
 
 	for _, tt := range tests {

test/e2e/generate_systemd_test.go

@@ -59,8 +59,7 @@ var _ = Describe("Podman generate systemd", func() {
 		session = podmanTest.Podman([]string{"generate", "systemd", "--restart-policy", "bogus", "foobar"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).To(ExitWithError())
-		found, _ := session.ErrorGrepString("bogus is not a valid restart policy")
+		Expect(session.ErrorToString()).To(ContainSubstring("bogus is not a valid restart policy"))
-		Expect(found).Should(BeTrue())
 	})
 
 	It("podman generate systemd good timeout value", func() {
@@ -71,12 +70,8 @@ var _ = Describe("Podman generate systemd", func() {
 		session = podmanTest.Podman([]string{"generate", "systemd", "--time", "1234", "foobar"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
-
+		Expect(session.OutputToString()).To(ContainSubstring("TimeoutStopSec=1294"))
-		found, _ := session.GrepString(" stop -t 1234 ")
+		Expect(session.OutputToString()).To(ContainSubstring(" stop -t 1234 "))
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("TimeoutStopSec=1294")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd", func() {
@@ -87,6 +82,9 @@ var _ = Describe("Podman generate systemd", func() {
 		session := podmanTest.Podman([]string{"generate", "systemd", "nginx"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
+
+		// The podman commands in the unit should not contain the root flags
+		Expect(session.OutputToString()).ToNot(ContainSubstring(" --runroot"))
 	})
 
 	It("podman generate systemd --files --name", func() {
@@ -101,9 +99,7 @@ var _ = Describe("Podman generate systemd", func() {
 		for _, file := range session.OutputToStringArray() {
 			os.Remove(file)
 		}
-
+		Expect(session.OutputToString()).To(ContainSubstring("/container-nginx.service"))
-		found, _ := session.GrepString("/container-nginx.service")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd with timeout", func() {
@@ -114,9 +110,7 @@ var _ = Describe("Podman generate systemd", func() {
 		session := podmanTest.Podman([]string{"generate", "systemd", "--time", "5", "nginx"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
-
+		Expect(session.OutputToString()).To(ContainSubstring("podman stop -t 5"))
-		found, _ := session.GrepString("podman stop -t 5")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd pod --name", func() {
@@ -137,35 +131,19 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# pod-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# pod-foo.service"))
-		Expect(found).To(BeTrue())
+		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"))
 
-		found, _ = session.GrepString("Requires=container-foo-1.service container-foo-2.service")
+		// The podman commands in the unit should not contain the root flags
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).ToNot(ContainSubstring(" --runroot"))
-
-		found, _ = session.GrepString("# container-foo-1.service")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString(" start foo-1")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("-infra") // infra container
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("# container-foo-2.service")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString(" stop -t 42 foo-2")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("BindsTo=pod-foo.service")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("PIDFile=")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("/userdata/conmon.pid")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd pod --name --files", func() {
@@ -185,11 +163,8 @@ var _ = Describe("Podman generate systemd", func() {
 			os.Remove(file)
 		}
 
-		found, _ := session.GrepString("/pod-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("/pod-foo.service"))
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).To(ContainSubstring("/container-foo-1.service"))
-
-		found, _ = session.GrepString("/container-foo-1.service")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd --new --name foo", func() {
@@ -202,14 +177,13 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# container-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# container-foo.service"))
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).To(ContainSubstring(" --replace "))
-
+		Expect(session.OutputToString()).To(ContainSubstring(" stop --ignore --cidfile %t/container-foo.ctr-id -t 42"))
-		found, _ = session.GrepString(" --replace ")
+		if !IsRemote() {
-		Expect(found).To(BeTrue())
+			// The podman commands in the unit should contain the root flags if generate systemd --new is used
-
+			Expect(session.OutputToString()).To(ContainSubstring(" --runroot"))
-		found, _ = session.GrepString("stop --ignore --cidfile %t/container-foo.ctr-id -t 42")
+		}
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd --new --name=foo", func() {
@@ -222,14 +196,9 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# container-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# container-foo.service"))
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).To(ContainSubstring(" --replace "))
-
+		Expect(session.OutputToString()).To(ContainSubstring(" stop --ignore --cidfile %t/container-foo.ctr-id -t 42"))
-		found, _ = session.GrepString(" --replace ")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("stop --ignore --cidfile %t/container-foo.ctr-id -t 42")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd --new without explicit detaching param", func() {
@@ -242,8 +211,7 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("--cgroups=no-conmon -d")
+		Expect(session.OutputToString()).To(ContainSubstring("--cgroups=no-conmon -d"))
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd --new with explicit detaching param in middle", func() {
@@ -256,8 +224,7 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("--name foo alpine top")
+		Expect(session.OutputToString()).To(ContainSubstring("--name foo alpine top"))
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd --new pod", func() {
@@ -280,8 +247,8 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# con-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# con-foo.service"))
-		Expect(found).To(BeTrue())
+
 	})
 
 	It("podman generate systemd --separator _", func() {
@@ -294,8 +261,7 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# container_foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# container_foo.service"))
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd pod --pod-prefix p", func() {
@@ -316,17 +282,10 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# p-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# p-foo.service"))
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).To(ContainSubstring("Requires=container-foo-1.service container-foo-2.service"))
-
+		Expect(session.OutputToString()).To(ContainSubstring("# container-foo-1.service"))
-		found, _ = session.GrepString("Requires=container-foo-1.service container-foo-2.service")
+		Expect(session.OutputToString()).To(ContainSubstring("BindsTo=p-foo.service"))
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("# container-foo-1.service")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("BindsTo=p-foo.service")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd pod --pod-prefix p --container-prefix con --separator _ change all prefixes/separator", func() {
@@ -347,20 +306,11 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# p_foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# p_foo.service"))
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).To(ContainSubstring("Requires=con_foo-1.service con_foo-2.service"))
-
+		Expect(session.OutputToString()).To(ContainSubstring("# con_foo-1.service"))
-		found, _ = session.GrepString("Requires=con_foo-1.service con_foo-2.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# con_foo-2.service"))
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).To(ContainSubstring("BindsTo=p_foo.service"))
-
-		found, _ = session.GrepString("# con_foo-1.service")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("# con_foo-2.service")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("BindsTo=p_foo.service")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd pod with containers --new", func() {
@@ -386,26 +336,13 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// Grepping the output (in addition to unit tests)
-		found, _ := session.GrepString("# pod-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("# pod-foo.service"))
-		Expect(found).To(BeTrue())
+		Expect(session.OutputToString()).To(ContainSubstring("Requires=container-foo-1.service container-foo-2.service"))
-
+		Expect(session.OutputToString()).To(ContainSubstring("BindsTo=pod-foo.service"))
-		found, _ = session.GrepString("Requires=container-foo-1.service container-foo-2.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(found).To(BeTrue())
+		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"))
-		found, _ = session.GrepString("BindsTo=pod-foo.service")
+		Expect(session.OutputToString()).To(ContainSubstring("pod rm --ignore -f --pod-id-file %t/pod-foo.pod-id"))
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("pod create --infra-conmon-pidfile %t/pod-foo.pid --pod-id-file %t/pod-foo.pod-id --name foo")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("ExecStartPre=/bin/rm -f %t/pod-foo.pid %t/pod-foo.pod-id")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("pod stop --ignore --pod-id-file %t/pod-foo.pod-id -t 10")
-		Expect(found).To(BeTrue())
-
-		found, _ = session.GrepString("pod rm --ignore -f --pod-id-file %t/pod-foo.pod-id")
-		Expect(found).To(BeTrue())
 	})
 
 	It("podman generate systemd --format json", func() {