generate systemd: refactor

Refactor the systemd-unit generation code and move all the logic into
`pkg/systemd/generate`.  The code was already hard to maintain but I
found it impossible to wire the `--new` logic for pods in all the chaos.

The code refactoring in this commit will make maintaining the code
easier and should make it easier to extend as well.  Further changes and
refactorings may still be needed but they will easier.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

1 files changed, 65 insertions, 44 deletions

diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
index fb0ea5cf9..f316d4452 100644
--- a/pkg/systemd/generate/containers.go
+++ b/pkg/systemd/generate/containers.go
@@ -11,22 +11,20 @@ import (
 	"text/template"
 	"time"
 
+	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/pkg/domain/entities"
 	"github.com/containers/libpod/version"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
-// EnvVariable "PODMAN_SYSTEMD_UNIT" is set in all generated systemd units and
-// is set to the unit's (unique) name.
-const EnvVariable = "PODMAN_SYSTEMD_UNIT"
-
-// ContainerInfo contains data required for generating a container's systemd
+// containerInfo contains data required for generating a container's systemd
 // unit file.
-type ContainerInfo struct {
+type containerInfo struct {
 	// ServiceName of the systemd service.
 	ServiceName string
 	// Name or ID of the container.
-	ContainerName string
+	ContainerNameOrID string
 	// StopTimeout sets the timeout Podman waits before killing the container
 	// during service stop.
 	StopTimeout uint
@@ -63,29 +61,7 @@ type ContainerInfo struct {
 	EnvVariable string
 }
 
-var restartPolicies = []string{"no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", "always"}
-
-// validateRestartPolicy checks that the user-provided policy is valid.
-func validateRestartPolicy(restart string) error {
-	for _, i := range restartPolicies {
-		if i == restart {
-			return nil
-		}
-	}
-	return errors.Errorf("%s is not a valid restart policy", restart)
-}
-
-const containerTemplate = `# {{.ServiceName}}.service
-# autogenerated by Podman {{.PodmanVersion}}
-{{- if .TimeStamp}}
-# {{.TimeStamp}}
-{{- end}}
-
-[Unit]
-Description=Podman {{.ServiceName}}.service
-Documentation=man:podman-generate-systemd(1)
-Wants=network.target
-After=network-online.target
+const containerTemplate = headerTemplate + `
 {{- if .BoundToServices}}
 RefuseManualStart=yes
 RefuseManualStop=yes
@@ -107,8 +83,8 @@ ExecStop={{.Executable}} stop --ignore --cidfile %t/%n-ctr-id {{if (ge .StopTime
 ExecStopPost={{.Executable}} rm --ignore -f --cidfile %t/%n-ctr-id
 PIDFile=%t/%n-pid
 {{- else}}
-ExecStart={{.Executable}} start {{.ContainerName}}
-ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerName}}
+ExecStart={{.Executable}} start {{.ContainerNameOrID}}
+ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}
 PIDFile={{.PIDFile}}
 {{- end}}
 KillMode=none
@@ -117,18 +93,19 @@ Type=forking
 [Install]
 WantedBy=multi-user.target default.target`
 
-// Options include different options to control the unit file generation.
-type Options struct {
-	// When set, generate service files in the current working directory and
-	// return the paths to these files instead of returning all contents in one
-	// big string.
-	Files bool
-	// New controls if a new container is created or if an existing one is started.
-	New bool
+// ContainerUnit generates a systemd unit for the specified container.  Based
+// on the options, the return value might be the entire unit or a file it has
+// been written to.
+func ContainerUnit(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, error) {
+	info, err := generateContainerInfo(ctr, options)
+	if err != nil {
+		return "", err
+	}
+	return createContainerSystemdUnit(info, options)
 }
 
-// CreateContainerSystemdUnit creates a systemd unit file for a container.
-func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, error) {
+// createContainerSystemdUnit creates a systemd unit file for a container.
+func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSystemdOptions) (string, error) {
 	if err := validateRestartPolicy(info.RestartPolicy); err != nil {
 		return "", err
 	}
@@ -152,7 +129,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro
 	// been created via a Python script, which would certainly yield an
 	// invalid `info.CreateCommand`.  Hence, we're doing a best effort unit
 	// generation and don't try aiming at completeness.
-	if opts.New {
+	if options.New {
 		// The create command must at least have three arguments:
 		// 	/usr/bin/podman run $IMAGE
 		index := 2
@@ -218,7 +195,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro
 		return "", err
 	}
 
-	if !opts.Files {
+	if !options.Files {
 		return buf.String(), nil
 	}
 
@@ -233,3 +210,47 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro
 	}
 	return path, nil
 }
+
+func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) {
+	timeout := ctr.StopTimeout()
+	if options.StopTimeout != nil {
+		timeout = *options.StopTimeout
+	}
+
+	config := ctr.Config()
+	conmonPidFile := config.ConmonPidFile
+	if conmonPidFile == "" {
+		return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag")
+	}
+
+	createCommand := []string{}
+	if config.CreateCommand != nil {
+		createCommand = config.CreateCommand
+	} else if options.New {
+		return nil, errors.Errorf("cannot use --new on container %q: no create command found", ctr.ID())
+	}
+
+	nameOrID, serviceName := containerServiceName(ctr, options)
+
+	info := containerInfo{
+		ServiceName:       serviceName,
+		ContainerNameOrID: nameOrID,
+		RestartPolicy:     options.RestartPolicy,
+		PIDFile:           conmonPidFile,
+		StopTimeout:       timeout,
+		GenerateTimestamp: true,
+		CreateCommand:     createCommand,
+	}
+	return &info, nil
+}
+
+// containerServiceName returns the nameOrID and the service name of the
+// container.
+func containerServiceName(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, string) {
+	nameOrID := ctr.ID()
+	if options.Name {
+		nameOrID = ctr.Name()
+	}
+	serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID)
+	return nameOrID, serviceName
+}