From 6708bc235feb09f5a5bc3f673da1e7c4beee5e12 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Thu, 14 Jan 2021 12:26:34 -0500 Subject: use AttachSocketPath when removing conmon files Signed-off-by: Peter Hunt --- libpod/container_internal.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index a53027ab2..041fa13e7 100644 --- a/libpod/container_internal.go +++ b/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()) } -- cgit v1.2.3-54-g00ecf From fdfff1dab2131cf53335576df9ece63f7c95c859 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Thu, 14 Jan 2021 13:53:28 -0500 Subject: oci: use `--full-path` option for conmon and stop relying on socket path Signed-off-by: Peter Hunt --- libpod/oci_conmon_exec_linux.go | 10 ++-------- libpod/oci_conmon_linux.go | 13 ++----------- 2 files changed, 4 insertions(+), 19 deletions(-) (limited to 'libpod') diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index b43316951..f782193af 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -284,14 +284,8 @@ 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. +// ExecContainerCleanup cleans up files created when a command is run via ExecContainer. 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 } @@ -302,7 +296,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. diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 5e8ed12e7..e3da9a237 100644 --- a/libpod/oci_conmon_linux.go +++ b/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{} -- cgit v1.2.3-54-g00ecf From 452b9e204f21f32c71ac876f1fce82697a1935e9 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Thu, 14 Jan 2021 13:58:36 -0500 Subject: oci: drop ExecContainerCleanup without the socketsDir, we no longer need to worry about cleaning up after an exec. Signed-off-by: Peter Hunt --- libpod/container_exec.go | 2 +- libpod/oci.go | 4 ---- libpod/oci_conmon_exec_linux.go | 5 ----- libpod/oci_missing.go | 5 ----- 4 files changed, 1 insertion(+), 15 deletions(-) (limited to 'libpod') diff --git a/libpod/container_exec.go b/libpod/container_exec.go index 8d8ed14aa..c359f1e5d 100644 --- a/libpod/container_exec.go +++ b/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 diff --git a/libpod/oci.go b/libpod/oci.go index f2053f1b5..1f2c7dd71 100644 --- a/libpod/oci.go +++ b/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() diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index f782193af..76338b86c 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -284,11 +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. -func (r *ConmonOCIRuntime) ExecContainerCleanup(ctr *Container, sessionID string) error { - 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 diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go index eb8cdebad..10526f368 100644 --- a/libpod/oci_missing.go +++ b/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() -- cgit v1.2.3-54-g00ecf From b2c6663a9f6f52d43b01b7a45dd0d32781bc9ac1 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Thu, 14 Jan 2021 14:41:40 -0500 Subject: runtime: return findConmon to libpod I believe moving the conmon probing code to c/common wasn't the best strategy. Different container engines have different requrements of which conmon version is required (based on what flags they use). Signed-off-by: Peter Hunt --- libpod/define/errors.go | 4 ++ libpod/runtime.go | 113 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/define/errors.go b/libpod/define/errors.go index e19ac6a27..8d943099b 100644 --- a/libpod/define/errors.go +++ b/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" ) diff --git a/libpod/runtime.go b/libpod/runtime.go index 98ca2d5a4..53c34a10e 100644 --- a/libpod/runtime.go +++ b/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 = 1 +) + // 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\d+).(?P\d+).(?P\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 { -- cgit v1.2.3-54-g00ecf From 90d7f671385b2237facd65427d2aa059cc6fe24e Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Thu, 14 Jan 2021 14:43:34 -0500 Subject: runtime: bump required conmon version 2.0.24 introduced the new behavior with --full-attach, allowing podman to no longer use the socketDir Signed-off-by: Peter Hunt --- libpod/runtime.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/runtime.go b/libpod/runtime.go index 53c34a10e..dc53d5ef1 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -44,7 +44,7 @@ const ( conmonMinMinorVersion = 0 // conmonMinPatchVersion is the sub-minor version required for conmon. - conmonMinPatchVersion = 1 + conmonMinPatchVersion = 24 ) // A RuntimeOption is a functional option which alters the Runtime created by -- cgit v1.2.3-54-g00ecf