Merge pull request #8979 from haircommander/full-attach-path

Use full attach path, rather than a symlink

libpod/container_exec.go

@@ -773,7 +773,7 @@ func (c *Container) cleanupExecBundle(sessionID string) error {
 		return err
 	}
 
-	return c.ociRuntime.ExecContainerCleanup(c, sessionID)
+	return nil
 }
 
 // the path to a containers exec session bundle

libpod/container_internal.go

@@ -685,7 +685,11 @@ func (c *Container) removeIPv4Allocations() error {
 // This is necessary for restarting containers
 func (c *Container) removeConmonFiles() error {
 	// Files are allowed to not exist, so ignore ENOENT
-	attachFile := filepath.Join(c.bundlePath(), "attach")
+	attachFile, err := c.AttachSocketPath()
+	if err != nil {
+		return errors.Wrapf(err, "failed to get attach socket path for container %s", c.ID())
+	}
+
 	if err := os.Remove(attachFile); err != nil && !os.IsNotExist(err) {
 		return errors.Wrapf(err, "error removing container %s attach file", c.ID())
 	}

libpod/define/errors.go

@@ -206,4 +206,8 @@ var (
 	// ErrCanceled indicates that an operation has been cancelled by a user.
 	// Useful for potentially long running tasks.
 	ErrCanceled = errors.New("cancelled by user")
+
+	// ErrConmonVersionFormat is used when the expected versio-format of conmon
+	// has changed.
+	ErrConmonVersionFormat = "conmon version changed format"
 )

libpod/oci.go

@@ -94,10 +94,6 @@ type OCIRuntime interface {
 	// ExecUpdateStatus checks the status of a given exec session.
 	// Returns true if the session is still running, or false if it exited.
 	ExecUpdateStatus(ctr *Container, sessionID string) (bool, error)
-	// ExecContainerCleanup cleans up after an exec session exits.
-	// It removes any files left by the exec session that are no longer
-	// needed, including the attach socket.
-	ExecContainerCleanup(ctr *Container, sessionID string) error
 
 	// CheckpointContainer checkpoints the given container.
 	// Some OCI runtimes may not support this - if SupportsCheckpoint()

libpod/oci_conmon_exec_linux.go

@@ -284,17 +284,6 @@ func (r *ConmonOCIRuntime) ExecUpdateStatus(ctr *Container, sessionID string) (b
 	return true, nil
 }
 
-// ExecContainerCleanup cleans up files created when a command is run via
-// ExecContainer. This includes the attach socket for the exec session.
-func (r *ConmonOCIRuntime) ExecContainerCleanup(ctr *Container, sessionID string) error {
-	// Clean up the sockets dir. Issue #3962
-	// Also ignore if it doesn't exist for some reason; hence the conditional return below
-	if err := os.RemoveAll(filepath.Join(r.socketsDir, sessionID)); err != nil && !os.IsNotExist(err) {
-		return err
-	}
-	return nil
-}
-
 // ExecAttachSocketPath is the path to a container's exec session attach socket.
 func (r *ConmonOCIRuntime) ExecAttachSocketPath(ctr *Container, sessionID string) (string, error) {
 	// We don't even use container, so don't validity check it
@@ -302,7 +291,7 @@ func (r *ConmonOCIRuntime) ExecAttachSocketPath(ctr *Container, sessionID string
 		return "", errors.Wrapf(define.ErrInvalidArg, "must provide a valid session ID to get attach socket path")
 	}
 
-	return filepath.Join(r.socketsDir, sessionID, "attach"), nil
+	return filepath.Join(ctr.execBundlePath(sessionID), "attach"), nil
 }
 
 // This contains pipes used by the exec API.

libpod/oci_conmon_linux.go

@@ -59,7 +59,6 @@ type ConmonOCIRuntime struct {
 	conmonEnv         []string
 	tmpDir            string
 	exitsDir          string
-	socketsDir        string
 	logSizeMax        int64
 	noPivot           bool
 	reservePorts      bool
@@ -149,7 +148,6 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
 	}
 
 	runtime.exitsDir = filepath.Join(runtime.tmpDir, "exits")
-	runtime.socketsDir = filepath.Join(runtime.tmpDir, "socket")
 
 	// Create the exit files and attach sockets directories
 	if err := os.MkdirAll(runtime.exitsDir, 0750); err != nil {
@@ -158,13 +156,6 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
 			return nil, errors.Wrapf(err, "error creating OCI runtime exit files directory")
 		}
 	}
-	if err := os.MkdirAll(runtime.socketsDir, 0750); err != nil {
-		// The directory is allowed to exist
-		if !os.IsExist(err) {
-			return nil, errors.Wrap(err, "error creating OCI runtime attach sockets directory")
-		}
-	}
-
 	return runtime, nil
 }
 
@@ -865,7 +856,7 @@ func (r *ConmonOCIRuntime) AttachSocketPath(ctr *Container) (string, error) {
 		return "", errors.Wrapf(define.ErrInvalidArg, "must provide a valid container to get attach socket path")
 	}
 
-	return filepath.Join(r.socketsDir, ctr.ID(), "attach"), nil
+	return filepath.Join(ctr.bundlePath(), "attach"), nil
 }
 
 // ExitFilePath is the path to a container's exit file.
@@ -1240,7 +1231,7 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
 		"-p", pidPath,
 		"-n", ctr.Name(),
 		"--exit-dir", exitDir,
-		"--socket-dir-path", r.socketsDir,
+		"--full-attach",
 	}
 	if len(r.runtimeFlags) > 0 {
 		rFlags := []string{}

libpod/oci_missing.go

@@ -151,11 +151,6 @@ func (r *MissingRuntime) ExecUpdateStatus(ctr *Container, sessionID string) (boo
 	return false, r.printError()
 }
 
-// ExecContainerCleanup is not available as the runtime is missing
-func (r *MissingRuntime) ExecContainerCleanup(ctr *Container, sessionID string) error {
-	return r.printError()
-}
-
 // CheckpointContainer is not available as the runtime is missing
 func (r *MissingRuntime) CheckpointContainer(ctr *Container, options ContainerCheckpointOptions) error {
 	return r.printError()

libpod/runtime.go

@@ -2,10 +2,14 @@ package libpod
 
 import (
 	"bufio"
+	"bytes"
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"regexp"
+	"strconv"
 	"strings"
 	"sync"
 	"syscall"
@@ -32,6 +36,17 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+const (
+	// conmonMinMajorVersion is the major version required for conmon.
+	conmonMinMajorVersion = 2
+
+	// conmonMinMinorVersion is the minor version required for conmon.
+	conmonMinMinorVersion = 0
+
+	// conmonMinPatchVersion is the sub-minor version required for conmon.
+	conmonMinPatchVersion = 24
+)
+
 // A RuntimeOption is a functional option which alters the Runtime created by
 // NewRuntime
 type RuntimeOption func(*Runtime) error
@@ -260,7 +275,7 @@ func getLockManager(runtime *Runtime) (lock.Manager, error) {
 // Sets up containers/storage, state store, OCI runtime
 func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
 	// Find a working conmon binary
-	cPath, err := runtime.config.FindConmon()
+	cPath, err := findConmon(runtime.config.Engine.ConmonPath)
 	if err != nil {
 		return err
 	}
@@ -532,6 +547,102 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
 	return nil
 }
 
+// findConmon iterates over conmonPaths and returns the path
+// to the first conmon binary with a new enough version. If none is found,
+// we try to do a path lookup of "conmon".
+func findConmon(conmonPaths []string) (string, error) {
+	foundOutdatedConmon := false
+	for _, path := range conmonPaths {
+		stat, err := os.Stat(path)
+		if err != nil {
+			continue
+		}
+		if stat.IsDir() {
+			continue
+		}
+		if err := probeConmon(path); err != nil {
+			logrus.Warnf("Conmon at %s invalid: %v", path, err)
+			foundOutdatedConmon = true
+			continue
+		}
+		logrus.Debugf("Using conmon: %q", path)
+		return path, nil
+	}
+
+	// Search the $PATH as last fallback
+	if path, err := exec.LookPath("conmon"); err == nil {
+		if err := probeConmon(path); err != nil {
+			logrus.Warnf("Conmon at %s is invalid: %v", path, err)
+			foundOutdatedConmon = true
+		} else {
+			logrus.Debugf("Using conmon from $PATH: %q", path)
+			return path, nil
+		}
+	}
+
+	if foundOutdatedConmon {
+		return "", errors.Wrapf(define.ErrConmonOutdated,
+			"please update to v%d.%d.%d or later",
+			conmonMinMajorVersion, conmonMinMinorVersion, conmonMinPatchVersion)
+	}
+
+	return "", errors.Wrapf(define.ErrInvalidArg,
+		"could not find a working conmon binary (configured options: %v)",
+		conmonPaths)
+}
+
+// probeConmon calls conmon --version and verifies it is a new enough version for
+// the runtime expectations the container engine currently has.
+func probeConmon(conmonBinary string) error {
+	cmd := exec.Command(conmonBinary, "--version")
+	var out bytes.Buffer
+	cmd.Stdout = &out
+	err := cmd.Run()
+	if err != nil {
+		return err
+	}
+	r := regexp.MustCompile(`^conmon version (?P<Major>\d+).(?P<Minor>\d+).(?P<Patch>\d+)`)
+
+	matches := r.FindStringSubmatch(out.String())
+	if len(matches) != 4 {
+		return errors.Wrap(err, define.ErrConmonVersionFormat)
+	}
+	major, err := strconv.Atoi(matches[1])
+	if err != nil {
+		return errors.Wrap(err, define.ErrConmonVersionFormat)
+	}
+	if major < conmonMinMajorVersion {
+		return define.ErrConmonOutdated
+	}
+	if major > conmonMinMajorVersion {
+		return nil
+	}
+
+	minor, err := strconv.Atoi(matches[2])
+	if err != nil {
+		return errors.Wrap(err, define.ErrConmonVersionFormat)
+	}
+	if minor < conmonMinMinorVersion {
+		return define.ErrConmonOutdated
+	}
+	if minor > conmonMinMinorVersion {
+		return nil
+	}
+
+	patch, err := strconv.Atoi(matches[3])
+	if err != nil {
+		return errors.Wrap(err, define.ErrConmonVersionFormat)
+	}
+	if patch < conmonMinPatchVersion {
+		return define.ErrConmonOutdated
+	}
+	if patch > conmonMinPatchVersion {
+		return nil
+	}
+
+	return nil
+}
+
 // TmpDir gets the current Libpod temporary files directory.
 func (r *Runtime) TmpDir() (string, error) {
 	if !r.valid {

test/e2e/run_apparmor_test.go

@@ -14,6 +14,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+// wip
 func skipIfAppArmorEnabled() {
 	if apparmor.IsEnabled() {
 		Skip("Apparmor is enabled")