Merge pull request #14384 from mheon/move_attach

Move Attach under the OCI Runtime interface

4 files changed, 82 insertions, 18 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index d87deb71a..b064d3528 100644
--- a/libpod/container_api.go
+++ b/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)
@@ -260,8 +271,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.
diff --git a/libpod/oci.go b/libpod/oci.go
index 09f856ac7..90862969c 100644
--- a/libpod/oci.go
+++ b/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
-// runtime dependent.
-// TODO: May want to move the conmon cleanup code here too - it depends on
+// TODO: May want to move the conmon cleanup code here - 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 {
diff --git a/libpod/oci_attach_linux.go b/libpod/oci_conmon_attach_linux.go
index 06f8f8719..155a8fbc3 100644
--- a/libpod/oci_attach_linux.go
+++ b/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
-// Does not check if state is appropriate
-// 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 {
+// Attach to the given container.
+// Does not check if state is appropriate.
+// started is only required if startContainer is true.
+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)
-	if attachRdy != nil {
-		attachRdy <- true
+	receiveStdoutError, stdinDone := setupStdioChannels(params.Streams, conn, detachKeys)
+	if params.AttachReady != nil {
+		params.AttachReady <- true
 	}
-	return readStdio(conn, streams, receiveStdoutError, stdinDone)
+	return readStdio(conn, params.Streams, receiveStdoutError, stdinDone)
 }
 
 // Attach to the given container's exec session
diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go
index 86f54c02e..fd8160830 100644
--- a/libpod/oci_missing.go
+++ b/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()