podman generate systemd: handle --sdnotify correctly

When a container was created with `--sdnotify value` we would remove this arg instead of using it like with `--sdnotfiy=value`. Also when the arg is set to ignore we should force conmon in order to make the resulting Type=notify units work. Fixes containers#15052 Signed-off-by: Paul Holzinger <[email protected]>
Luap99 · Jul 25, 2022 · 6a9338a · 6a9338a
1 parent da1f479
commit 6a9338a
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 6 deletions.
diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
@@ -42,7 +42,7 @@ RequiresMountsFor={{{{.RunRoot}}}}
 // filterPodFlags removes --pod, --pod-id-file and --infra-conmon-pidfile from the specified command.
 // argCount is the number of last arguments which should not be filtered, e.g. the container entrypoint.
 func filterPodFlags(command []string, argCount int) []string {
-	processed := []string{}
+	processed := make([]string, 0, len(command))
 	for i := 0; i < len(command)-argCount; i++ {
 		s := command[i]
 		if s == "--pod" || s == "--pod-id-file" || s == "--infra-conmon-pidfile" {
@@ -63,15 +63,15 @@ func filterPodFlags(command []string, argCount int) []string {
 // filterCommonContainerFlags removes --sdnotify, --rm and --cgroups from the specified command.
 // argCount is the number of last arguments which should not be filtered, e.g. the container entrypoint.
 func filterCommonContainerFlags(command []string, argCount int) []string {
-	processed := []string{}
+	processed := make([]string, 0, len(command))
 	for i := 0; i < len(command)-argCount; i++ {
 		s := command[i]
 
 		switch {
 		case s == "--rm":
 			// Boolean flags support --flag and --flag={true,false}.
 			continue
-		case s == "--sdnotify", s == "--cgroups", s == "--cidfile", s == "--restart":
+		case s == "--cgroups", s == "--cidfile", s == "--restart":
 			i++
 			continue
 		case strings.HasPrefix(s, "--rm="),
@@ -111,6 +111,24 @@ func escapeSystemdArg(arg string) string {
 	return arg
 }
 
+func removeSdNotifyArg(args []string, argCount int) []string {
+	processed := make([]string, 0, len(args))
+	for i := 0; i < len(args)-argCount; i++ {
+		s := args[i]
+
+		switch {
+		case s == "--sdnotify":
+			i++
+			continue
+		case strings.HasPrefix(s, "--sdnotify="):
+			continue
+		}
+		processed = append(processed, s)
+	}
+	processed = append(processed, args[len(args)-argCount:]...)
+	return processed
+}
+
 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

diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
@@ -403,8 +403,13 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 
 		// Default to --sdnotify=conmon unless already set by the
 		// container.
-		hasSdnotifyParam := fs.Lookup("sdnotify").Changed
-		if !hasSdnotifyParam {
+		sdnotifyFlag := fs.Lookup("sdnotify")
+		if !sdnotifyFlag.Changed {
+			startCommand = append(startCommand, "--sdnotify=conmon")
+		} else if sdnotifyFlag.Value.String() == libpodDefine.SdNotifyModeIgnore {
+			// If ignore is set force conmon otherwise the unit with Type=notify will fail.
+			logrus.Infof("Forcing --sdnotify=conmon for container %s", info.ContainerNameOrID)
+			remainingCmd = removeSdNotifyArg(remainingCmd, fs.NArg())
 			startCommand = append(startCommand, "--sdnotify=conmon")
 		}
 

diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
@@ -2,6 +2,7 @@ package generate
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/containers/podman/v4/pkg/domain/entities"
@@ -313,6 +314,39 @@ ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id
 Type=notify
 NotifyAccess=all
 
+[Install]
+WantedBy=default.target
+`
+
+	goodWithNameAndSdnotifyIgnore := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network-online.target
+After=network-online.target
+RequiresMountsFor=/var/run/containers/storage
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=on-failure
+TimeoutStopSec=70
+ExecStartPre=/bin/rm -f %t/%n.ctr-id
+ExecStart=/usr/bin/podman container run \
+	--cidfile=%t/%n.ctr-id \
+	--cgroups=no-conmon \
+	--rm \
+	--sdnotify=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/%n.ctr-id
+ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id
+Type=notify
+NotifyAccess=all
+
 [Install]
 WantedBy=default.target
 `
@@ -992,7 +1026,7 @@ WantedBy=default.target
 			false,
 			false,
 		},
-		{"good with name and sdnotify",
+		{"good with name and --sdnotify=container",
 			containerInfo{
 				Executable:        "/usr/bin/podman",
 				ServiceName:       "jadda-jadda",
@@ -1011,6 +1045,63 @@ WantedBy=default.target
 			false,
 			false,
 		},
+		{"good with name and --sdnotify container",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				PIDFile:           "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       10,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "container", "run", "--sdnotify", "container", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN", "foo=arg \"with \" space"},
+				EnvVariable:       define.EnvVariable,
+				GraphRoot:         "/var/lib/containers/storage",
+				RunRoot:           "/var/run/containers/storage",
+			},
+			strings.ReplaceAll(goodWithNameAndSdnotify, "--sdnotify=container", "--sdnotify container"),
+			true,
+			false,
+			false,
+			false,
+		},
+		{"good with name and --sdnotify=ignore",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				PIDFile:           "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       10,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "container", "run", "--sdnotify=ignore", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN", "foo=arg \"with \" space"},
+				EnvVariable:       define.EnvVariable,
+				GraphRoot:         "/var/lib/containers/storage",
+				RunRoot:           "/var/run/containers/storage",
+			},
+			goodWithNameAndSdnotifyIgnore,
+			true,
+			false,
+			false,
+			false,
+		},
+		{"good with name and --sdnotify ignore",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				PIDFile:           "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       10,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "container", "run", "--sdnotify", "ignore", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN", "foo=arg \"with \" space"},
+				EnvVariable:       define.EnvVariable,
+				GraphRoot:         "/var/lib/containers/storage",
+				RunRoot:           "/var/run/containers/storage",
+			},
+			goodWithNameAndSdnotifyIgnore,
+			true,
+			false,
+			false,
+			false,
+		},
 		{"good with explicit short detach param",
 			containerInfo{
 				Executable:        "/usr/bin/podman",