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>

cmd/podman/service.go

@@ -17,6 +17,7 @@ import (
 	"github.com/containers/libpod/pkg/adapter"
 	api "github.com/containers/libpod/pkg/api/server"
 	"github.com/containers/libpod/pkg/rootless"
+	"github.com/containers/libpod/pkg/systemd"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/containers/libpod/pkg/varlinkapi"
 	"github.com/containers/libpod/version"
@@ -55,13 +56,20 @@ func init() {
 }
 
 func serviceCmd(c *cliconfig.ServiceValues) error {
-	// For V2, default to the REST socket
-	apiURI := adapter.DefaultAPIAddress
-	if c.Varlink {
-		apiURI = adapter.DefaultVarlinkAddress
-	}
+	var apiURI string
+
+	// 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
 
-	if rootless.IsRootless() {
+	if len(c.InputArgs) > 0 {
+		apiURI = c.InputArgs[0]
+	} else if _, ok := systemd.SocketActivated(); ok {
+		apiURI = ""
+	} else if rootless.IsRootless() {
 		xdg, err := util.GetRuntimeDir()
 		if err != nil {
 			return err
@@ -80,15 +88,20 @@ func serviceCmd(c *cliconfig.ServiceValues) error {
 				return err
 			}
 		}
-		apiURI = fmt.Sprintf("unix:%s", socketDir)
+		apiURI = "unix:" + socketDir
+	} else if c.Varlink {
+		apiURI = adapter.DefaultVarlinkAddress
+	} else {
+		// For V2, default to the REST socket
+		apiURI = adapter.DefaultAPIAddress
 	}
 
-	if len(c.InputArgs) > 0 {
-		apiURI = c.InputArgs[0]
+	if "" == apiURI {
+		logrus.Info("using systemd socket activation to determine API endpoint")
+	} else {
+		logrus.Infof("using API endpoint: %s", apiURI)
 	}
 
-	logrus.Infof("using API endpoint: %s", apiURI)
-
 	// Create a single runtime api consumption
 	runtime, err := libpodruntime.GetRuntimeDisableFDs(getContext(), &c.PodmanCommand)
 	if err != nil {
@@ -106,17 +119,21 @@ func serviceCmd(c *cliconfig.ServiceValues) error {
 func runREST(r *libpod.Runtime, uri string, timeout time.Duration) error {
 	logrus.Warn("This function is EXPERIMENTAL")
 	fmt.Println("This function is EXPERIMENTAL.")
-	fields := strings.Split(uri, ":")
-	if len(fields) == 1 {
-		return errors.Errorf("%s is an invalid socket destination", uri)
-	}
-	address := strings.Join(fields[1:], ":")
-	l, err := net.Listen(fields[0], address)
-	if err != nil {
-		return errors.Wrapf(err, "unable to create socket %s", uri)
+
+	var l *net.Listener
+	if uri != "" {
+		fields := strings.Split(uri, ":")
+		if len(fields) == 1 {
+			return errors.Errorf("%s is an invalid socket destination", uri)
+		}
+		address := strings.Join(fields[1:], ":")
+		l, err := net.Listen(fields[0], address)
+		if err != nil {
+			return errors.Wrapf(err, "unable to create socket %s", uri)
+		}
+		defer l.Close()
 	}
-	defer l.Close()
-	server, err := api.NewServerWithSettings(r, timeout, &l)
+	server, err := api.NewServerWithSettings(r, timeout, l)
 	if err != nil {
 		return err
 	}

pkg/adapter/containers.go

@@ -26,7 +26,7 @@ import (
 	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/libpod/logs"
 	"github.com/containers/libpod/pkg/adapter/shortcuts"
-	"github.com/containers/libpod/pkg/systemdgen"
+	"github.com/containers/libpod/pkg/systemd"
 	"github.com/containers/storage"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -1142,7 +1142,7 @@ func generateServiceName(c *cliconfig.GenerateSystemdValues, ctr *libpod.Contain
 
 // generateSystemdgenContainerInfo is a helper to generate a
 // systemdgen.ContainerInfo for `GenerateSystemd`.
-func (r *LocalRuntime) generateSystemdgenContainerInfo(c *cliconfig.GenerateSystemdValues, nameOrID string, pod *libpod.Pod) (*systemdgen.ContainerInfo, bool, error) {
+func (r *LocalRuntime) generateSystemdgenContainerInfo(c *cliconfig.GenerateSystemdValues, nameOrID string, pod *libpod.Pod) (*systemd.ContainerInfo, bool, error) {
 	ctr, err := r.Runtime.LookupContainer(nameOrID)
 	if err != nil {
 		return nil, false, err
@@ -1160,7 +1160,7 @@ func (r *LocalRuntime) generateSystemdgenContainerInfo(c *cliconfig.GenerateSyst
 	}
 
 	name, serviceName := generateServiceName(c, ctr, pod)
-	info := &systemdgen.ContainerInfo{
+	info := &systemd.ContainerInfo{
 		ServiceName:       serviceName,
 		ContainerName:     name,
 		RestartPolicy:     c.RestartPolicy,
@@ -1175,7 +1175,7 @@ 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{
+	opts := systemd.Options{
 		Files: c.Files,
 		New:   c.New,
 	}
@@ -1184,7 +1184,7 @@ func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (stri
 	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, opts)
+		return systemd.CreateContainerSystemdUnit(info, opts)
 	}
 
 	// --new does not support pods.
@@ -1230,7 +1230,7 @@ func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (stri
 
 	// Traverse the dependency graph and create systemdgen.ContainerInfo's for
 	// each container.
-	containerInfos := []*systemdgen.ContainerInfo{podInfo}
+	containerInfos := []*systemd.ContainerInfo{podInfo}
 	for ctr, dependencies := range graph.DependencyMap() {
 		// Skip the infra container as we already generated it.
 		if ctr.ID() == infraID {
@@ -1260,7 +1260,7 @@ func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (stri
 		if i > 0 {
 			builder.WriteByte('\n')
 		}
-		out, err := systemdgen.CreateContainerSystemdUnit(info, opts)
+		out, err := systemd.CreateContainerSystemdUnit(info, opts)
 		if err != nil {
 			return "", err
 		}

pkg/api/handlers/types.go

@@ -354,21 +354,24 @@ func ImageDataToImageInspect(ctx context.Context, l *libpodImage.Image) (*ImageI
 
 func LibpodToContainer(l *libpod.Container, infoData []define.InfoData) (*Container, error) {
 	imageId, imageName := l.Image()
-	sizeRW, err := l.RWSize()
-	if err != nil {
-		return nil, err
-	}
 
-	SizeRootFs, err := l.RootFsSize()
-	if err != nil {
-		return nil, err
-	}
+	var (
+		sizeRW     int64
+		SizeRootFs int64
+		err        error
+	)
 
 	state, err := l.State()
 	if err != nil {
 		return nil, err
 	}
 
+	// FIXME Why do the *Size() calls cause a panic I cannot catch on other states?
+	if state == define.ContainerStateRunning {
+		sizeRW, _ = l.RWSize()
+		SizeRootFs, _ = l.RootFsSize()
+	}
+
 	return &Container{docker.Container{
 		ID:         l.ID(),
 		Names:      []string{fmt.Sprintf("/%s", l.Name())},

pkg/api/server/handler_api.go

@@ -2,32 +2,51 @@ package server
 
 import (
 	"context"
+	"fmt"
 	"net/http"
+	"runtime"
 
+	"github.com/containers/libpod/pkg/api/handlers/utils"
 	log "github.com/sirupsen/logrus"
 )
 
 // APIHandler is a wrapper to enhance HandlerFunc's and remove redundant code
 func APIHandler(ctx context.Context, h http.HandlerFunc) http.HandlerFunc {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		log.Debugf("APIHandler -- Method: %s URL: %s", r.Method, r.URL.String())
-		if err := r.ParseForm(); err != nil {
-			log.Infof("Failed Request: unable to parse form: %q", err)
-		}
+	return func(w http.ResponseWriter, r *http.Request) {
+		defer func() {
+			log.Debugf("Recovering APIHandler -- Method: %s URL: %s", r.Method, r.URL.String())
+
+			err := recover()
+			if err != nil {
+				buf := make([]byte, 1<<20)
+				n := runtime.Stack(buf, true)
+				log.Warnf("panic: %v, %s", err, buf[:n])
+				utils.InternalServerError(w, fmt.Errorf("%v", err))
+			}
+		}()
+
+		fn := func(w http.ResponseWriter, r *http.Request) {
+			log.Debugf("APIHandler -- Method: %s URL: %s", r.Method, r.URL.String())
+
+			if err := r.ParseForm(); err != nil {
+				log.Infof("Failed Request: unable to parse form: %q", err)
+			}
 
-		// TODO: Use ConnContext when ported to go 1.13
-		c := context.WithValue(r.Context(), "decoder", ctx.Value("decoder"))
-		c = context.WithValue(c, "runtime", ctx.Value("runtime"))
-		c = context.WithValue(c, "shutdownFunc", ctx.Value("shutdownFunc"))
-		r = r.WithContext(c)
+			// TODO: Use ConnContext when ported to go 1.13
+			c := context.WithValue(r.Context(), "decoder", ctx.Value("decoder"))
+			c = context.WithValue(c, "runtime", ctx.Value("runtime"))
+			c = context.WithValue(c, "shutdownFunc", ctx.Value("shutdownFunc"))
+			r = r.WithContext(c)
 
-		h(w, r)
+			h(w, r)
 
-		shutdownFunc := r.Context().Value("shutdownFunc").(func() error)
-		if err := shutdownFunc(); err != nil {
-			log.Errorf("Failed to shutdown Server in APIHandler(): %s", err.Error())
+			shutdownFunc := r.Context().Value("shutdownFunc").(func() error)
+			if err := shutdownFunc(); err != nil {
+				log.Errorf("Failed to shutdown Server in APIHandler(): %s", err.Error())
+			}
 		}
-	})
+		fn(w, r)
+	}
 }
 
 // VersionedPath prepends the version parsing code

pkg/api/server/server.go

@@ -64,6 +64,7 @@ func newServer(runtime *libpod.Runtime, duration time.Duration, listener *net.Li
 	}
 
 	router := mux.NewRouter().UseEncodedPath()
+
 	server := APIServer{
 		Server: http.Server{
 			Handler:           router,

pkg/systemd/activation.go

@@ -0,0 +1,40 @@
+package systemd
+
+import (
+	"os"
+	"strconv"
+	"strings"
+)
+
+// Determine if podman is running under the socket activation protocol
+func SocketActivated() (int, bool) {
+	pid, pid_found := os.LookupEnv("LISTEN_PID")
+	fds, fds_found := os.LookupEnv("LISTEN_FDS")
+	fdnames, fdnames_found := os.LookupEnv("LISTEN_FDNAMES")
+
+	if !(pid_found && fds_found && fdnames_found) {
+		return 0, false
+	}
+
+	p, err := strconv.Atoi(pid)
+	if err != nil || p != os.Getpid() {
+		return 0, false
+	}
+
+	nfds, err := strconv.Atoi(fds)
+	if err != nil || nfds < 1 {
+		return 0, false
+	}
+
+	if nfds > 1 {
+		names := strings.Split(fdnames, ":")
+		for i, n := range names {
+			if strings.Contains(n, "podman") {
+				return 3 + i, true
+			}
+		}
+	}
+
+	// First available file descriptor is always 3.
+	return 3, true
+}

pkg/systemdgen/systemdgen.go → pkg/systemd/systemdgen.go

@@ -1,4 +1,4 @@
-package systemdgen
+package systemd
 
 import (
 	"bytes"

pkg/systemdgen/systemdgen_test.go → pkg/systemd/systemdgen_test.go

@@ -1,4 +1,4 @@
-package systemdgen
+package systemd
 
 import (
 	"testing"