Fix handler and systemd activation errors

On panic from handler: log warning and stack trace, report
InternalServerError to client

When using `podman system service` make determining the listening endpoint deterministic.

  // When determining _*THE*_ listening endpoint --
  // 1) User input wins always
  // 2) systemd socket activation
  // 3) rootless honors XDG_RUNTIME_DIR
  // 4) if varlink -- adapter.DefaultVarlinkAddress
  // 5) lastly adapter.DefaultAPIAddress

Fixes #5150
Fixes #5151

Signed-off-by: Jhon Honce <jhonce@redhat.com>

2 files changed, 0 insertions, 447 deletions

diff --git a/pkg/systemdgen/systemdgen.go b/pkg/systemdgen/systemdgen.go
deleted file mode 100644
index 26b3b3756..000000000
--- a/pkg/systemdgen/systemdgen.go
+++ /dev/null
@@ -1,206 +0,0 @@
-package systemdgen
-
-import (
-	"bytes"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"sort"
-	"strings"
-	"text/template"
-	"time"
-
-	"github.com/containers/libpod/version"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
-)
-
-// ContainerInfo contains data required for generating a container's systemd
-// unit file.
-type ContainerInfo struct {
-	// ServiceName of the systemd service.
-	ServiceName string
-	// Name or ID of the container.
-	ContainerName string
-	// InfraContainer of the pod.
-	InfraContainer string
-	// StopTimeout sets the timeout Podman waits before killing the container
-	// during service stop.
-	StopTimeout int
-	// RestartPolicy of the systemd unit (e.g., no, on-failure, always).
-	RestartPolicy string
-	// PIDFile of the service. Required for forking services. Must point to the
-	// PID of the associated conmon process.
-	PIDFile string
-	// GenerateTimestamp, if set the generated unit file has a time stamp.
-	GenerateTimestamp bool
-	// BoundToServices are the services this service binds to.  Note that this
-	// service runs after them.
-	BoundToServices []string
-	// RequiredServices are services this service requires. Note that this
-	// service runs before them.
-	RequiredServices []string
-	// PodmanVersion for the header. Will be set internally. Will be auto-filled
-	// if left empty.
-	PodmanVersion string
-	// Executable is the path to the podman executable. Will be auto-filled if
-	// left empty.
-	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"}
-
-// 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)
-{{- if .BoundToServices}}
-RefuseManualStart=yes
-RefuseManualStop=yes
-BindsTo={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
-After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
-{{- end}}
-{{- if .RequiredServices}}
-Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
-Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
-{{- end}}
-
-[Service]
-Restart={{.RestartPolicy}}
-{{- if .New}}
-ExecStartPre=/usr/bin/rm -f /%t/%n-pid /%t/%n-cid
-ExecStart={{.RunCommand}}
-ExecStop={{.Executable}} stop --ignore --cidfile /%t/%n-cid {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}
-ExecStopPost={{.Executable}} rm --ignore -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
-
-[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, opts Options) (string, error) {
-	if err := validateRestartPolicy(info.RestartPolicy); err != nil {
-		return "", err
-	}
-
-	// Make sure the executable is set.
-	if info.Executable == "" {
-		executable, err := os.Executable()
-		if err != nil {
-			executable = "/usr/bin/podman"
-			logrus.Warnf("Could not obtain podman executable location, using default %s", executable)
-		}
-		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 five arguments and append the
-		// CreateCommand with a stripped command and subcomand.
-		command := []string{
-			info.Executable,
-			"run",
-			"--conmon-pidfile", "/%t/%n-pid",
-			"--cidfile", "/%t/%n-cid",
-			"--cgroups=no-conmon",
-		}
-		command = append(command, info.CreateCommand[index:]...)
-		info.RunCommand = strings.Join(command, " ")
-		info.New = true
-	}
-
-	if info.PodmanVersion == "" {
-		info.PodmanVersion = version.Version
-	}
-	if info.GenerateTimestamp {
-		info.TimeStamp = fmt.Sprintf("%v", time.Now().Format(time.UnixDate))
-	}
-
-	// Sort the slices to assure a deterministic output.
-	sort.Strings(info.RequiredServices)
-	sort.Strings(info.BoundToServices)
-
-	// Generate the template and compile it.
-	templ, err := template.New("systemd_service_file").Parse(containerTemplate)
-	if err != nil {
-		return "", errors.Wrap(err, "error parsing systemd service template")
-	}
-
-	var buf bytes.Buffer
-	if err := templ.Execute(&buf, info); err != nil {
-		return "", err
-	}
-
-	if !opts.Files {
-		return buf.String(), nil
-	}
-
-	buf.WriteByte('\n')
-	cwd, err := os.Getwd()
-	if err != nil {
-		return "", errors.Wrap(err, "error getting current working directory")
-	}
-	path := filepath.Join(cwd, fmt.Sprintf("%s.service", info.ServiceName))
-	if err := ioutil.WriteFile(path, buf.Bytes(), 0644); err != nil {
-		return "", errors.Wrap(err, "error generating systemd unit")
-	}
-	return path, nil
-}
diff --git a/pkg/systemdgen/systemdgen_test.go b/pkg/systemdgen/systemdgen_test.go
deleted file mode 100644
index ee2429407..000000000
--- a/pkg/systemdgen/systemdgen_test.go
+++ /dev/null
@@ -1,241 +0,0 @@
-package systemdgen
-
-import (
-	"testing"
-)
-
-func TestValidateRestartPolicy(t *testing.T) {
-	type ContainerInfo struct {
-		restart string
-	}
-	tests := []struct {
-		name          string
-		ContainerInfo ContainerInfo
-		wantErr       bool
-	}{
-		{"good-on", ContainerInfo{restart: "no"}, false},
-		{"good-on-success", ContainerInfo{restart: "on-success"}, false},
-		{"good-on-failure", ContainerInfo{restart: "on-failure"}, false},
-		{"good-on-abnormal", ContainerInfo{restart: "on-abnormal"}, false},
-		{"good-on-watchdog", ContainerInfo{restart: "on-watchdog"}, false},
-		{"good-on-abort", ContainerInfo{restart: "on-abort"}, false},
-		{"good-always", ContainerInfo{restart: "always"}, false},
-		{"fail", ContainerInfo{restart: "foobar"}, true},
-		{"failblank", ContainerInfo{restart: ""}, true},
-	}
-	for _, tt := range tests {
-		test := tt
-		t.Run(tt.name, func(t *testing.T) {
-			if err := validateRestartPolicy(test.ContainerInfo.restart); (err != nil) != test.wantErr {
-				t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr)
-			}
-		})
-	}
-}
-
-func TestCreateContainerSystemdUnit(t *testing.T) {
-	goodID := `# container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service
-# autogenerated by Podman CI
-
-[Unit]
-Description=Podman container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.service
-Documentation=man:podman-generate-systemd(1)
-
-[Service]
-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
-
-[Install]
-WantedBy=multi-user.target`
-
-	goodName := `# container-foobar.service
-# autogenerated by Podman CI
-
-[Unit]
-Description=Podman container-foobar.service
-Documentation=man:podman-generate-systemd(1)
-
-[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
-
-[Install]
-WantedBy=multi-user.target`
-
-	goodNameBoundTo := `# container-foobar.service
-# autogenerated by Podman CI
-
-[Unit]
-Description=Podman container-foobar.service
-Documentation=man:podman-generate-systemd(1)
-RefuseManualStart=yes
-RefuseManualStop=yes
-BindsTo=a.service b.service c.service pod.service
-After=a.service b.service c.service pod.service
-
-[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
-
-[Install]
-WantedBy=multi-user.target`
-
-	podGoodName := `# pod-123abc.service
-# autogenerated by Podman CI
-
-[Unit]
-Description=Podman pod-123abc.service
-Documentation=man:podman-generate-systemd(1)
-Requires=container-1.service container-2.service
-Before=container-1.service container-2.service
-
-[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 --cgroups=no-conmon --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN
-ExecStop=/usr/bin/podman stop --ignore --cidfile /%t/%n-cid -t 42
-ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile /%t/%n-cid
-PIDFile=/%t/%n-pid
-KillMode=none
-Type=forking
-
-[Install]
-WantedBy=multi-user.target`
-
-	tests := []struct {
-		name    string
-		info    ContainerInfo
-		want    string
-		wantErr bool
-	}{
-
-		{"good with id",
-			ContainerInfo{
-				Executable:    "/usr/bin/podman",
-				ServiceName:   "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
-				ContainerName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
-				RestartPolicy: "always",
-				PIDFile:       "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:   10,
-				PodmanVersion: "CI",
-			},
-			goodID,
-			false,
-		},
-		{"good with name",
-			ContainerInfo{
-				Executable:    "/usr/bin/podman",
-				ServiceName:   "container-foobar",
-				ContainerName: "foobar",
-				RestartPolicy: "always",
-				PIDFile:       "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:   10,
-				PodmanVersion: "CI",
-			},
-			goodName,
-			false,
-		},
-		{"good with name and bound to",
-			ContainerInfo{
-				Executable:      "/usr/bin/podman",
-				ServiceName:     "container-foobar",
-				ContainerName:   "foobar",
-				RestartPolicy:   "always",
-				PIDFile:         "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:     10,
-				PodmanVersion:   "CI",
-				BoundToServices: []string{"pod", "a", "b", "c"},
-			},
-			goodNameBoundTo,
-			false,
-		},
-		{"pod",
-			ContainerInfo{
-				Executable:       "/usr/bin/podman",
-				ServiceName:      "pod-123abc",
-				ContainerName:    "jadda-jadda-infra",
-				RestartPolicy:    "always",
-				PIDFile:          "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:      10,
-				PodmanVersion:    "CI",
-				RequiredServices: []string{"container-1", "container-2"},
-			},
-			podGoodName,
-			false,
-		},
-		{"bad restart policy",
-			ContainerInfo{
-				Executable:    "/usr/bin/podman",
-				ServiceName:   "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
-				RestartPolicy: "never",
-				PIDFile:       "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
-				StopTimeout:   10,
-				PodmanVersion: "CI",
-			},
-			"",
-			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 {
-		test := tt
-		t.Run(tt.name, func(t *testing.T) {
-			opts := Options{
-				Files: false,
-				New:   test.info.New,
-			}
-			got, err := CreateContainerSystemdUnit(&test.info, opts)
-			if (err != nil) != test.wantErr {
-				t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr)
-				return
-			}
-			if got != test.want {
-				t.Errorf("CreateContainerSystemdUnit() = \n%v\n---------> want\n%v", got, test.want)
-			}
-		})
-	}
-}