podman-generate-systemd --new

Add a --new flag to podman-generate-systemd to create a new container
via podman-run instead of starting an existing container.

Creating a new container presents the challenge to find a reverse
mapping from a container to the CLI flags it can be created with.  We
are doing this via `(Container).Config.CreateCommand` field, which
includes a copy of the process' command from procFS at creating time.
This field may not be useful when the container was not created via the
Podman CLI (e.g., via a Python script).  Hence, we do not guarantee the
correctness of the generated files.

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

cmd/podman/cliconfig/config.go

@@ -163,6 +163,7 @@ type GenerateKubeValues struct {
 type GenerateSystemdValues struct {
 	PodmanCommand
 	Name          bool
+	New           bool
 	Files         bool
 	RestartPolicy string
 	StopTimeout   int

cmd/podman/generate_systemd.go

@@ -45,6 +45,7 @@ func init() {
 	}
 	flags.IntVarP(&containerSystemdCommand.StopTimeout, "timeout", "t", -1, "stop timeout override")
 	flags.StringVar(&containerSystemdCommand.RestartPolicy, "restart-policy", "on-failure", "applicable systemd restart-policy")
+	flags.BoolVarP(&containerSystemdCommand.New, "new", "", false, "create a new container instead of starting an existing one")
 }
 
 func generateSystemdCmd(c *cliconfig.GenerateSystemdValues) error {

docs/source/markdown/podman-generate-systemd.1.md

@@ -22,11 +22,16 @@ Generate files instead of printing to stdout.  The generated files are named {co
 
 Use the name of the container for the start, stop, and description in the unit file
 
+**--new**
+
+Create a new container via podman-run instead of starting an existing one.  This option relies on container configuration files, which may not map directly to podman CLI flags; please review the generated output carefully before placing in production.
+
 **--timeout**, **-t**=*value*
 
 Override the default stop timeout for the container with the given value.
 
 **--restart-policy**=*policy*
+
 Set the systemd restart policy.  The restart-policy must be one of: "no", "on-success", "on-failure", "on-abnormal",
 "on-watchdog", "on-abort", or "always".  The default policy is *on-failure*.
 

pkg/adapter/containers.go

@@ -1230,6 +1230,7 @@ func (r *LocalRuntime) generateSystemdgenContainerInfo(c *cliconfig.GenerateSyst
 		PIDFile:           conmonPidFile,
 		StopTimeout:       timeout,
 		GenerateTimestamp: true,
+		CreateCommand:     config.CreateCommand,
 	}
 
 	return info, true, nil
@@ -1237,11 +1238,21 @@ func (r *LocalRuntime) generateSystemdgenContainerInfo(c *cliconfig.GenerateSyst
 
 // GenerateSystemd creates a unit file for a container or pod.
 func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (string, error) {
+	opts := systemdgen.Options{
+		Files: c.Files,
+		New:   c.New,
+	}
+
 	// First assume it's a container.
 	if info, found, err := r.generateSystemdgenContainerInfo(c, c.InputArgs[0], nil); found && err != nil {
 		return "", err
 	} else if found && err == nil {
-		return systemdgen.CreateContainerSystemdUnit(info, c.Files)
+		return systemdgen.CreateContainerSystemdUnit(info, opts)
+	}
+
+	// --new does not support pods.
+	if c.New {
+		return "", errors.Errorf("error generating systemd unit files: cannot generate generic files for a pod")
 	}
 
 	// We're either having a pod or garbage.
@@ -1312,7 +1323,7 @@ func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (stri
 		if i > 0 {
 			builder.WriteByte('\n')
 		}
-		out, err := systemdgen.CreateContainerSystemdUnit(info, c.Files)
+		out, err := systemdgen.CreateContainerSystemdUnit(info, opts)
 		if err != nil {
 			return "", err
 		}

pkg/systemdgen/systemdgen.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"strings"
 	"text/template"
 	"time"
 
@@ -48,6 +49,14 @@ type ContainerInfo struct {
 	Executable string
 	// TimeStamp at the time of creating the unit file. Will be set internally.
 	TimeStamp string
+	// New controls if a new container is created or if an existing one is started.
+	New bool
+	// CreateCommand is the full command plus arguments of the process the
+	// container has been created with.
+	CreateCommand []string
+	// RunCommand is a post-processed variant of CreateCommand and used for
+	// the ExecStart field in generic unit files.
+	RunCommand string
 }
 
 var restartPolicies = []string{"no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", "always"}
@@ -84,17 +93,35 @@ Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{
 
 [Service]
 Restart={{.RestartPolicy}}
+{{- if .New}}
+ExecStartPre=/usr/bin/rm -f /%t/%n-pid /%t/%n-cid
+ExecStart={{.RunCommand}}
+ExecStop={{.Executable}} stop --cidfile /%t/%n-cid {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}
+ExecStopPost={{.Executable}} rm -f --cidfile /%t/%n-cid
+PIDFile=/%t/%n-pid
+{{- else}}
 ExecStart={{.Executable}} start {{.ContainerName}}
 ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerName}}
+PIDFile={{.PIDFile}}
+{{- end}}
 KillMode=none
 Type=forking
-PIDFile={{.PIDFile}}
 
 [Install]
 WantedBy=multi-user.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
+}
+
 // CreateContainerSystemdUnit creates a systemd unit file for a container.
-func CreateContainerSystemdUnit(info *ContainerInfo, generateFiles bool) (string, error) {
+func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, error) {
 	if err := validateRestartPolicy(info.RestartPolicy); err != nil {
 		return "", err
 	}
@@ -109,6 +136,36 @@ func CreateContainerSystemdUnit(info *ContainerInfo, generateFiles bool) (string
 		info.Executable = executable
 	}
 
+	// Assemble the ExecStart command when creating a new container.
+	//
+	// Note that we cannot catch all corner cases here such that users
+	// *must* manually check the generated files.  A container might have
+	// 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 {
+		// The create command must at least have three arguments:
+		// 	/usr/bin/podman run $IMAGE
+		index := 2
+		if info.CreateCommand[1] == "container" {
+			index = 3
+		}
+		if len(info.CreateCommand) < index+1 {
+			return "", errors.Errorf("container's create command is too short or invalid: %v", info.CreateCommand)
+		}
+		// We're hard-coding the first four arguments and append the
+		// CreatCommand with a stripped command and subcomand.
+		command := []string{
+			info.Executable,
+			"run",
+			"--conmon-pidfile", "/%t/%n-pid",
+			"--cidfile", "/%t/%n-cid",
+		}
+		command = append(command, info.CreateCommand[index:]...)
+		info.RunCommand = strings.Join(command, " ")
+		info.New = true
+	}
+
 	if info.PodmanVersion == "" {
 		info.PodmanVersion = version.Version
 	}
@@ -131,7 +188,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, generateFiles bool) (string
 		return "", err
 	}
 
-	if !generateFiles {
+	if !opts.Files {
 		return buf.String(), nil
 	}
 

pkg/systemdgen/systemdgen_test.go

@@ -44,9 +44,9 @@ Documentation=man:podman-generate-systemd(1)
 Restart=always
 ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
 ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
+PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 KillMode=none
 Type=forking
-PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 
 [Install]
 WantedBy=multi-user.target`
@@ -62,9 +62,9 @@ Documentation=man:podman-generate-systemd(1)
 Restart=always
 ExecStart=/usr/bin/podman start foobar
 ExecStop=/usr/bin/podman stop -t 10 foobar
+PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 KillMode=none
 Type=forking
-PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 
 [Install]
 WantedBy=multi-user.target`
@@ -84,9 +84,9 @@ After=a.service b.service c.service pod.service
 Restart=always
 ExecStart=/usr/bin/podman start foobar
 ExecStop=/usr/bin/podman stop -t 10 foobar
+PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 KillMode=none
 Type=forking
-PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 
 [Install]
 WantedBy=multi-user.target`
@@ -104,9 +104,29 @@ Before=container-1.service container-2.service
 Restart=always
 ExecStart=/usr/bin/podman start jadda-jadda-infra
 ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra
+PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
+KillMode=none
+Type=forking
+
+[Install]
+WantedBy=multi-user.target`
+
+	goodNameNew := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+
+[Service]
+Restart=always
+ExecStartPre=/usr/bin/rm -f /%t/%n-pid /%t/%n-cid
+ExecStart=/usr/bin/podman run --conmon-pidfile /%t/%n-pid --cidfile /%t/%n-cid --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
+ExecStop=/usr/bin/podman stop --cidfile /%t/%n-cid -t 42
+ExecStopPost=/usr/bin/podman rm -f --cidfile /%t/%n-cid
+PIDFile=/%t/%n-pid
 KillMode=none
 Type=forking
-PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
 
 [Install]
 WantedBy=multi-user.target`
@@ -184,16 +204,35 @@ WantedBy=multi-user.target`
 			"",
 			true,
 		},
+		{"good with name and generic",
+			ContainerInfo{
+				Executable:    "/usr/bin/podman",
+				ServiceName:   "jadda-jadda",
+				ContainerName: "jadda-jadda",
+				RestartPolicy: "always",
+				PIDFile:       "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:   42,
+				PodmanVersion: "CI",
+				New:           true,
+				CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"},
+			},
+			goodNameNew,
+			false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := CreateContainerSystemdUnit(&tt.info, false)
+			opts := Options{
+				Files: false,
+				New:   tt.info.New,
+			}
+			got, err := CreateContainerSystemdUnit(&tt.info, opts)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, tt.wantErr)
 				return
 			}
 			if got != tt.want {
-				t.Errorf("CreateContainerSystemdUnit() = \n%v, want \n%v", got, tt.want)
+				t.Errorf("CreateContainerSystemdUnit() = \n%v\n---------> want\n%v", got, tt.want)
 			}
 		})
 	}

test/e2e/generate_systemd_test.go

@@ -177,4 +177,32 @@ var _ = Describe("Podman generate systemd", func() {
 		found, _ = session.GrepString("/container-foo-1.service")
 		Expect(found).To(BeTrue())
 	})
+
+	It("podman generate systemd --new", func() {
+		n := podmanTest.Podman([]string{"create", "--name", "foo", "alpine", "top"})
+		n.WaitWithDefaultTimeout()
+		Expect(n.ExitCode()).To(Equal(0))
+
+		session := podmanTest.Podman([]string{"generate", "systemd", "--timeout", "42", "--name", "--new", "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		// Grepping the output (in addition to unit tests)
+		found, _ := session.GrepString("# container-foo.service")
+		Expect(found).To(BeTrue())
+
+		found, _ = session.GrepString("stop --cidfile /%t/%n-cid -t 42")
+		Expect(found).To(BeTrue())
+	})
+
+	It("podman generate systemd --new pod", func() {
+		n := podmanTest.Podman([]string{"pod", "create", "--name", "foo"})
+		n.WaitWithDefaultTimeout()
+		Expect(n.ExitCode()).To(Equal(0))
+
+		session := podmanTest.Podman([]string{"generate", "systemd", "--timeout", "42", "--name", "--new", "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+	})
+
 })