Merge pull request #11076 from boaz0/closes_10275

Support template unit files in podman generate systemd

4 files changed, 174 insertions, 6 deletions

diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
index 3515bb3b7..24c85a27e 100644
--- a/pkg/systemd/generate/common.go
+++ b/pkg/systemd/generate/common.go
@@ -23,7 +23,7 @@ func validateRestartPolicy(restart string) error {
 	return errors.Errorf("%s is not a valid restart policy", restart)
 }
 
-const headerTemplate = `# {{{{.ServiceName}}}}.service
+const headerTemplate = `# {{{{.ServiceName}}}}{{{{- if (eq .IdentifySpecifier true) }}}}@{{{{- end}}}}.service
 {{{{- if (eq .GenerateNoHeader false) }}}}
 # autogenerated by Podman {{{{.PodmanVersion}}}}
 {{{{- if .TimeStamp}}}}
@@ -32,7 +32,7 @@ const headerTemplate = `# {{{{.ServiceName}}}}.service
 {{{{- end}}}}
 
 [Unit]
-Description=Podman {{{{.ServiceName}}}}.service
+Description=Podman {{{{.ServiceName}}}}.service{{{{- if (eq .IdentifySpecifier true) }}}} for %I{{{{- end}}}}
 Documentation=man:podman-generate-systemd(1)
 Wants=network-online.target
 After=network-online.target
diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
index 037652a6d..95ff13371 100644
--- a/pkg/systemd/generate/containers.go
+++ b/pkg/systemd/generate/containers.go
@@ -90,6 +90,8 @@ type containerInfo struct {
 	// Location of the RunRoot for the container.  Required for ensuring the tmpfs
 	// or volume exists and is mounted when coming online at boot.
 	RunRoot string
+	// Add %i and %I to description and execute parts
+	IdentifySpecifier bool
 }
 
 const containerTemplate = headerTemplate + `
@@ -99,7 +101,7 @@ After={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{
 {{{{- end}}}}
 
 [Service]
-Environment={{{{.EnvVariable}}}}=%n
+Environment={{{{.EnvVariable}}}}=%n{{{{- if (eq .IdentifySpecifier true) }}}}-%i{{{{- end}}}}
 {{{{- if .ExtraEnvs}}}}
 Environment={{{{- range $index, $value := .ExtraEnvs -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}{{{{end}}}}
 {{{{- end}}}}
@@ -204,6 +206,46 @@ func containerServiceName(ctr *libpod.Container, options entities.GenerateSystem
 	return nameOrID, serviceName
 }
 
+// setContainerNameForTemplate updates startCommand to contain the name argument with
+// a value that includes the identify specifier.
+// In case startCommand doesn't contain that argument it's added after "run" and its
+// value will be set to info.ServiceName concated with the identify specifier %i.
+func setContainerNameForTemplate(startCommand []string, info *containerInfo) ([]string, error) {
+	// find the index of "--name" in the command slice
+	nameIx := -1
+	for argIx, arg := range startCommand {
+		if arg == "--name" {
+			nameIx = argIx + 1
+			break
+		}
+		if strings.HasPrefix(arg, "--name=") {
+			nameIx = argIx
+			break
+		}
+	}
+	switch {
+	case nameIx == -1:
+		// if not found, add --name argument in the command slice before the "run" argument.
+		// it's assumed that the command slice contains this argument.
+		runIx := -1
+		for argIx, arg := range startCommand {
+			if arg == "run" {
+				runIx = argIx
+				break
+			}
+		}
+		if runIx == -1 {
+			return startCommand, fmt.Errorf("\"run\" is missing in the command arguments")
+		}
+		startCommand = append(startCommand[:runIx+1], startCommand[runIx:]...)
+		startCommand[runIx+1] = fmt.Sprintf("--name=%s-%%i", info.ServiceName)
+	default:
+		// append the identity specifier (%i) to the end of the --name value
+		startCommand[nameIx] = fmt.Sprintf("%s-%%i", startCommand[nameIx])
+	}
+	return startCommand, nil
+}
+
 // executeContainerTemplate executes the container template on the specified
 // containerInfo.  Note that the containerInfo is also post processed and
 // completed, which allows for an easier unit testing.
@@ -273,7 +315,6 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 			"--rm",
 		)
 		remainingCmd := info.CreateCommand[index:]
-
 		// Presence check for certain flags/options.
 		fs := pflag.NewFlagSet("args", pflag.ContinueOnError)
 		fs.ParseErrorsWhitelist.UnknownFlags = true
@@ -389,6 +430,13 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 
 		startCommand = append(startCommand, remainingCmd...)
 		startCommand = escapeSystemdArguments(startCommand)
+		if options.TemplateUnitFile {
+			info.IdentifySpecifier = true
+			startCommand, err = setContainerNameForTemplate(startCommand, info)
+			if err != nil {
+				return "", err
+			}
+		}
 		info.ExecStart = strings.Join(startCommand, " ")
 	}
 
diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
index f46513459..eab2c2e67 100644
--- a/pkg/systemd/generate/containers_test.go
+++ b/pkg/systemd/generate/containers_test.go
@@ -1,6 +1,7 @@
 package generate
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/containers/podman/v3/pkg/domain/entities"
@@ -522,6 +523,32 @@ NotifyAccess=all
 [Install]
 WantedBy=multi-user.target default.target
 `
+
+	templateGood := `# container-foo@.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman container-foo.service for %I
+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-%i
+Restart=on-failure
+StartLimitBurst=42
+TimeoutStopSec=70
+ExecStartPre=/bin/rm -f %t/%n.ctr-id
+ExecStart=/usr/bin/podman run --name=container-foo-%i --cidfile=%t/%n.ctr-id --cgroups=no-conmon --rm --sdnotify=conmon -d awesome-image:latest
+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=multi-user.target default.target
+`
 	tests := []struct {
 		name     string
 		info     containerInfo
@@ -529,6 +556,7 @@ WantedBy=multi-user.target default.target
 		new      bool
 		noHeader bool
 		wantErr  bool
+		template bool
 	}{
 
 		{"good with id",
@@ -547,6 +575,7 @@ WantedBy=multi-user.target default.target
 			false,
 			false,
 			false,
+			false,
 		},
 		{"good with noHeader",
 			containerInfo{
@@ -564,6 +593,7 @@ WantedBy=multi-user.target default.target
 			false,
 			true,
 			false,
+			false,
 		},
 		{"good with name",
 			containerInfo{
@@ -581,6 +611,7 @@ WantedBy=multi-user.target default.target
 			false,
 			false,
 			false,
+			false,
 		},
 		{"good with name and bound to",
 			containerInfo{
@@ -599,6 +630,7 @@ WantedBy=multi-user.target default.target
 			false,
 			false,
 			false,
+			false,
 		},
 		{"good with name and generic",
 			containerInfo{
@@ -617,6 +649,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with name and sdnotify",
 			containerInfo{
@@ -635,6 +668,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with explicit short detach param",
 			containerInfo{
@@ -653,6 +687,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with explicit short detach param and podInfo",
 			containerInfo{
@@ -674,6 +709,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with explicit full detach param",
 			containerInfo{
@@ -692,6 +728,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with id and no param",
 			containerInfo{
@@ -710,6 +747,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with explicit detach=true param",
 			containerInfo{
@@ -728,6 +766,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with explicit detach=false param",
 			containerInfo{
@@ -746,6 +785,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with explicit detach=false param",
 			containerInfo{
@@ -764,6 +804,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with multiple detach=false params",
 			containerInfo{
@@ -782,6 +823,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with multiple shorthand params detach first",
 			containerInfo{
@@ -800,6 +842,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with multiple shorthand params detach last",
 			containerInfo{
@@ -818,6 +861,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with container create",
 			containerInfo{
@@ -836,6 +880,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with journald log tag (see #9034)",
 			containerInfo{
@@ -854,6 +899,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with special chars",
 			containerInfo{
@@ -872,6 +918,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with ID files",
 			containerInfo{
@@ -890,6 +937,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with pod ID files",
 			containerInfo{
@@ -911,6 +959,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with environment variables",
 			containerInfo{
@@ -930,6 +979,7 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
 		},
 		{"good with restart policy",
 			containerInfo{
@@ -948,14 +998,34 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 			false,
+			false,
+		},
+		{"good template",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "container-foo",
+				ContainerNameOrID: "foo",
+				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{"I'll get stripped", "create", "--restart", "on-failure:42", "awesome-image:latest"},
+			},
+			templateGood,
+			true,
+			false,
+			false,
+			true,
 		},
 	}
 	for _, tt := range tests {
 		test := tt
 		t.Run(tt.name, func(t *testing.T) {
 			opts := entities.GenerateSystemdOptions{
-				New:      test.new,
-				NoHeader: test.noHeader,
+				New:              test.new,
+				NoHeader:         test.noHeader,
+				TemplateUnitFile: test.template,
 			}
 			test.info.RestartPolicy = define.DefaultRestartPolicy
 			got, err := executeContainerTemplate(&test.info, opts)
@@ -967,3 +1037,48 @@ WantedBy=multi-user.target default.target
 		})
 	}
 }
+
+func TestSetContainerNameForTemplate(t *testing.T) {
+	tt := []struct {
+		name         string
+		startCommand []string
+		info         *containerInfo
+		expected     []string
+		err          error
+	}{
+		{
+			name:         "no name argument is set",
+			startCommand: []string{"/usr/bin/podman", "run", "busybox", "top"},
+			info:         &containerInfo{ServiceName: "container-122"},
+			expected:     []string{"/usr/bin/podman", "run", "--name=container-122-%i", "busybox", "top"},
+			err:          nil,
+		},
+		{
+			name:         "--name=value is used in arguments",
+			startCommand: []string{"/usr/bin/podman", "run", "--name=lovely_james", "busybox", "top"},
+			info:         &containerInfo{},
+			expected:     []string{"/usr/bin/podman", "run", "--name=lovely_james-%i", "busybox", "top"},
+			err:          nil,
+		},
+		{
+			name:         "--name value is used in arguments",
+			startCommand: []string{"/usr/bin/podman", "run", "--name", "lovely_james", "busybox", "top"},
+			info:         &containerInfo{},
+			expected:     []string{"/usr/bin/podman", "run", "--name", "lovely_james-%i", "busybox", "top"},
+			err:          nil,
+		},
+		{
+			name:         "--name value is used in arguments",
+			startCommand: []string{"/usr/bin/podman", "create", "busybox", "top"},
+			info:         &containerInfo{},
+			expected:     []string{"/usr/bin/podman", "create", "busybox", "top"},
+			err:          fmt.Errorf("\"run\" is missing in the command arguments"),
+		},
+	}
+
+	for _, te := range tt {
+		res, err := setContainerNameForTemplate(te.startCommand, te.info)
+		assert.Equal(t, te.err, err)
+		assert.Equal(t, te.expected, res)
+	}
+}
diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
index e755b8eea..38f7e8e3e 100644
--- a/pkg/systemd/generate/pods.go
+++ b/pkg/systemd/generate/pods.go
@@ -79,6 +79,8 @@ type podInfo struct {
 	// Location of the RunRoot for the pod.  Required for ensuring the tmpfs
 	// or volume exists and is mounted when coming online at boot.
 	RunRoot string
+	// Add %i and %I to description and execute parts - this should not be used
+	IdentifySpecifier bool
 }
 
 const podTemplate = headerTemplate + `Requires={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
@@ -108,6 +110,9 @@ WantedBy=multi-user.target default.target
 // Based on the options, the return value might be the content of all units or
 // the files they been written to.
 func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (map[string]string, error) {
+	if options.TemplateUnitFile {
+		return nil, errors.New("--template is not supported for pods")
+	}
 	// Error out if the pod has no infra container, which we require to be the
 	// main service.
 	if !pod.HasInfraContainer() {