Move Attach under the OCI Runtime interface

With conmon-rs on the horizon, we need to disentangle Libpod from
legacy Conmon to the greatest extent possible. There are
definitely opportunities for codesharing between the two, but we
have to assume the implementations will be largely disjoint given
the different architectures.

Fortunately, most of the work has already been done in the past.
The conmon-managed OCI runtime mostly sits behind an interface,
with a few exceptions - the most notable of those being attach.
This PR thus moves Attach behind the interface, to ensure that we
can have attach implementations that don't use our existing unix
socket streaming if necessary.

Still to-do is conmon cleanup. There's a lot of code that removes
Conmon-specific files, or kills the Conmon PID, and all of it
will need to be refactored behind the interface.

[NO NEW TESTS NEEDED] Just moving some things around.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

libpod/container_api.go

@@ -123,7 +123,18 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachSt
 
 	// Attach to the container before starting it
 	go func() {
-		if err := c.attach(streams, keys, resize, true, startedChan, nil); err != nil {
+		// Start resizing
+		if c.LogDriver() != define.PassthroughLogging {
+			registerResizeFunc(resize, c.bundlePath())
+		}
+
+		opts := new(AttachOptions)
+		opts.Streams = streams
+		opts.DetachKeys = &keys
+		opts.Start = true
+		opts.Started = startedChan
+
+		if err := c.ociRuntime.Attach(c, opts); err != nil {
 			attachChan <- err
 		}
 		close(attachChan)
@@ -261,8 +272,18 @@ func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-
 		}()
 	}
 
+	// Start resizing
+	if c.LogDriver() != define.PassthroughLogging {
+		registerResizeFunc(resize, c.bundlePath())
+	}
+
+	opts := new(AttachOptions)
+	opts.Streams = streams
+	opts.DetachKeys = &keys
+	opts.AttachReady = attachRdy
+
 	c.newContainerEvent(events.Attach)
-	return c.attach(streams, keys, resize, false, nil, attachRdy)
+	return c.ociRuntime.Attach(c, opts)
 }
 
 // HTTPAttach forwards an attach session over a hijacked HTTP session.

libpod/oci.go

@@ -12,9 +12,7 @@ import (
 // management logic - e.g., we do not expect it to determine on its own that
 // calling 'UnpauseContainer()' on a container that is not paused is an error.
 // The code calling the OCIRuntime will manage this.
-// TODO: May want to move the Attach() code under this umbrella. It's highly OCI
+// TODO: May want to move the conmon cleanup code here - it depends on
-// runtime dependent.
-// TODO: May want to move the conmon cleanup code here too - it depends on
 // Conmon being in use.
 type OCIRuntime interface {
 	// Name returns the name of the runtime.
@@ -52,6 +50,8 @@ type OCIRuntime interface {
 	// UnpauseContainer unpauses the given container.
 	UnpauseContainer(ctr *Container) error
 
+	// Attach to a container.
+	Attach(ctr *Container, params *AttachOptions) error
 	// HTTPAttach performs an attach intended to be transported over HTTP.
 	// For terminal attach, the container's output will be directly streamed
 	// to output; otherwise, STDOUT and STDERR will be multiplexed, with
@@ -149,6 +149,30 @@ type OCIRuntime interface {
 	RuntimeInfo() (*define.ConmonInfo, *define.OCIRuntimeInfo, error)
 }
 
+// AttachOptions are options used when attached to a container or an exec
+// session.
+type AttachOptions struct {
+	// Streams are the streams to attach to.
+	Streams *define.AttachStreams
+	// DetachKeys containers the key combination that will detach from the
+	// attach session. Empty string is assumed as no detach keys - user
+	// detach is impossible. If unset, defaults from containers.conf will be
+	// used.
+	DetachKeys *string
+	// InitialSize is the initial size of the terminal. Set before the
+	// attach begins.
+	InitialSize *define.TerminalSize
+	// AttachReady signals when the attach has successfully completed and
+	// streaming has begun.
+	AttachReady chan<- bool
+	// Start indicates that the container should be started if it is not
+	// already running.
+	Start bool
+	// Started signals when the container has been successfully started.
+	// Required if Start is true, unused otherwise.
+	Started chan<- bool
+}
+
 // ExecOptions are options passed into ExecContainer. They control the command
 // that will be executed and how the exec will proceed.
 type ExecOptions struct {

libpod/oci_conmon_attach_linux.go

@@ -38,19 +38,28 @@ func openUnixSocket(path string) (*net.UnixConn, error) {
 	return net.DialUnix("unixpacket", nil, &net.UnixAddr{Name: fmt.Sprintf("/proc/self/fd/%d", fd), Net: "unixpacket"})
 }
 
-// Attach to the given container
+// Attach to the given container.
-// Does not check if state is appropriate
+// Does not check if state is appropriate.
-// started is only required if startContainer is true
+// started is only required if startContainer is true.
-func (c *Container) attach(streams *define.AttachStreams, keys string, resize <-chan define.TerminalSize, startContainer bool, started chan bool, attachRdy chan<- bool) error {
+func (r *ConmonOCIRuntime) Attach(c *Container, params *AttachOptions) error {
 	passthrough := c.LogDriver() == define.PassthroughLogging
 
-	if !streams.AttachOutput && !streams.AttachError && !streams.AttachInput && !passthrough {
+	if params == nil || params.Streams == nil {
+		return errors.Wrapf(define.ErrInternal, "must provide parameters to Attach")
+	}
+
+	if !params.Streams.AttachOutput && !params.Streams.AttachError && !params.Streams.AttachInput && !passthrough {
 		return errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to")
 	}
-	if startContainer && started == nil {
+	if params.Start && params.Started == nil {
 		return errors.Wrapf(define.ErrInternal, "started chan not passed when startContainer set")
 	}
 
+	keys := config.DefaultDetachKeys
+	if params.DetachKeys != nil {
+		keys = *params.DetachKeys
+	}
+
 	detachKeys, err := processDetachKeys(keys)
 	if err != nil {
 		return err
@@ -60,7 +69,12 @@ func (c *Container) attach(streams *define.AttachStreams, keys string, resize <-
 	if !passthrough {
 		logrus.Debugf("Attaching to container %s", c.ID())
 
-		registerResizeFunc(resize, c.bundlePath())
+		// If we have a resize, do it.
+		if params.InitialSize != nil {
+			if err := r.AttachResize(c, *params.InitialSize); err != nil {
+				return err
+			}
+		}
 
 		attachSock, err := c.AttachSocketPath()
 		if err != nil {
@@ -80,22 +94,22 @@ func (c *Container) attach(streams *define.AttachStreams, keys string, resize <-
 
 	// If starting was requested, start the container and notify when that's
 	// done.
-	if startContainer {
+	if params.Start {
 		if err := c.start(); err != nil {
 			return err
 		}
-		started <- true
+		params.Started <- true
 	}
 
 	if passthrough {
 		return nil
 	}
 
-	receiveStdoutError, stdinDone := setupStdioChannels(streams, conn, detachKeys)
+	receiveStdoutError, stdinDone := setupStdioChannels(params.Streams, conn, detachKeys)
-	if attachRdy != nil {
+	if params.AttachReady != nil {
-		attachRdy <- true
+		params.AttachReady <- true
 	}
-	return readStdio(conn, streams, receiveStdoutError, stdinDone)
+	return readStdio(conn, params.Streams, receiveStdoutError, stdinDone)
 }
 
 // Attach to the given container's exec session

libpod/oci_missing.go

@@ -108,6 +108,11 @@ func (r *MissingRuntime) UnpauseContainer(ctr *Container) error {
 	return r.printError()
 }
 
+// Attach is not available as the runtime is missing
+func (r *MissingRuntime) Attach(ctr *Container, params *AttachOptions) error {
+	return r.printError()
+}
+
 // HTTPAttach is not available as the runtime is missing
 func (r *MissingRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool, streamAttach, streamLogs bool) error {
 	return r.printError()