Fixes for podman-remote run and attach

Fixes the ability to run (create,start) a container and attach to its
console correctly.  We can now also exit from the console without
hanging the remote client.

Signed-off-by: baude <bbaude@redhat.com>

10 files changed, 170 insertions, 50 deletions

diff --git a/API.md b/API.md
index 95507d189..738449f43 100755
--- a/API.md
+++ b/API.md
@@ -3,7 +3,7 @@ Podman Service Interface and API description.  The master version of this docume
 in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in the upstream libpod repository.
 ## Index
 
-[func Attach(name: string) ](#Attach)
+[func Attach(name: string, detachKeys: string, start: bool) ](#Attach)
 
 [func AttachControl(name: string) ](#AttachControl)
 
@@ -119,6 +119,8 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in
 
 [func PodStateData(name: string) string](#PodStateData)
 
+[func Ps(opts: PsOpts) PsContainer](#Ps)
+
 [func PullImage(name: string, certDir: string, creds: string, signaturePolicy: string, tlsVerify: ) MoreResponse](#PullImage)
 
 [func PushImage(name: string, tag: string, tlsverify: , signaturePolicy: string, creds: string, certDir: string, compress: bool, format: string, removeSignatures: bool, signBy: string) MoreResponse](#PushImage)
@@ -225,6 +227,10 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in
 
 [type PodmanInfo](#PodmanInfo)
 
+[type PsContainer](#PsContainer)
+
+[type PsOpts](#PsOpts)
+
 [type Runlabel](#Runlabel)
 
 [type Sockets](#Sockets)
@@ -261,8 +267,9 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in
 ### <a name="Attach"></a>func Attach
 <div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
 
-method Attach(name: [string](https://godoc.org/builtin#string)) </div>
-
+method Attach(name: [string](https://godoc.org/builtin#string), detachKeys: [string](https://godoc.org/builtin#string), start: [bool](https://godoc.org/builtin#bool)) </div>
+Attach takes the name or ID of a container and sets up a the ability to remotely attach to its console. The start
+bool is whether you wish to start the container in question first.
 ### <a name="AttachControl"></a>func AttachControl
 <div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
 
@@ -857,6 +864,11 @@ $ varlink call -m unix:/run/podman/io.podman/io.podman.PausePod '{"name": "fooba
 method PodStateData(name: [string](https://godoc.org/builtin#string)) [string](https://godoc.org/builtin#string)</div>
 PodStateData returns inspectr level information of a given pod in string form.  This call is for
 development of Podman only and generally should not be used.
+### <a name="Ps"></a>func Ps
+<div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
+
+method Ps(opts: [PsOpts](#PsOpts)) [PsContainer](#PsContainer)</div>
+
 ### <a name="PullImage"></a>func PullImage
 <div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
 
@@ -1749,6 +1761,82 @@ insecure_registries [[]string](#[]string)
 store [InfoStore](#InfoStore)
 
 podman [InfoPodmanBinary](#InfoPodmanBinary)
+### <a name="PsContainer"></a>type PsContainer
+
+
+
+id [string](https://godoc.org/builtin#string)
+
+image [string](https://godoc.org/builtin#string)
+
+command [string](https://godoc.org/builtin#string)
+
+created [string](https://godoc.org/builtin#string)
+
+ports [string](https://godoc.org/builtin#string)
+
+names [string](https://godoc.org/builtin#string)
+
+isInfra [bool](https://godoc.org/builtin#bool)
+
+status [string](https://godoc.org/builtin#string)
+
+state [string](https://godoc.org/builtin#string)
+
+pidNum [int](https://godoc.org/builtin#int)
+
+rootFsSize [int](https://godoc.org/builtin#int)
+
+rwSize [int](https://godoc.org/builtin#int)
+
+pod [string](https://godoc.org/builtin#string)
+
+createdAt [string](https://godoc.org/builtin#string)
+
+exitedAt [string](https://godoc.org/builtin#string)
+
+startedAt [string](https://godoc.org/builtin#string)
+
+labels [map[string]](#map[string])
+
+nsPid [string](https://godoc.org/builtin#string)
+
+cgroup [string](https://godoc.org/builtin#string)
+
+ipc [string](https://godoc.org/builtin#string)
+
+mnt [string](https://godoc.org/builtin#string)
+
+net [string](https://godoc.org/builtin#string)
+
+pidNs [string](https://godoc.org/builtin#string)
+
+user [string](https://godoc.org/builtin#string)
+
+uts [string](https://godoc.org/builtin#string)
+
+mounts [string](https://godoc.org/builtin#string)
+### <a name="PsOpts"></a>type PsOpts
+
+
+
+all [bool](https://godoc.org/builtin#bool)
+
+filters [](#)
+
+last [](#)
+
+latest [](#)
+
+noTrunc [](#)
+
+pod [](#)
+
+quiet [](#)
+
+sort [](#)
+
+sync [](#)
 ### <a name="Runlabel"></a>type Runlabel
 
 Runlabel describes the required input for container runlabel
diff --git a/cmd/podman/cliconfig/commands.go b/cmd/podman/cliconfig/commands.go
index 3361c14b8..00b66e32a 100644
--- a/cmd/podman/cliconfig/commands.go
+++ b/cmd/podman/cliconfig/commands.go
@@ -1,6 +1,8 @@
 package cliconfig
 
-import "github.com/sirupsen/logrus"
+import (
+	"github.com/sirupsen/logrus"
+)
 
 // GlobalIsSet is a compatibility method for urfave
 func (p *PodmanCommand) GlobalIsSet(opt string) bool {
diff --git a/cmd/podman/create.go b/cmd/podman/create.go
index 1af3920dd..3267e5b7b 100644
--- a/cmd/podman/create.go
+++ b/cmd/podman/create.go
@@ -66,7 +66,7 @@ func createCmd(c *cliconfig.CreateValues) error {
 }
 
 func createInit(c *cliconfig.PodmanCommand) error {
-	if c.Bool("trace") {
+	if !remote && c.Bool("trace") {
 		span, _ := opentracing.StartSpanFromContext(Ctx, "createInit")
 		defer span.Finish()
 	}
diff --git a/cmd/podman/run.go b/cmd/podman/run.go
index bac5c3c18..d3158de6b 100644
--- a/cmd/podman/run.go
+++ b/cmd/podman/run.go
@@ -38,7 +38,7 @@ func init() {
 }
 
 func runCmd(c *cliconfig.RunValues) error {
-	if c.Bool("trace") {
+	if !remote && c.Bool("trace") {
 		span, _ := opentracing.StartSpanFromContext(Ctx, "runCmd")
 		defer span.Finish()
 	}
diff --git a/cmd/podman/run_test.go b/cmd/podman/run_test.go
index 27b34c323..af9e6923c 100644
--- a/cmd/podman/run_test.go
+++ b/cmd/podman/run_test.go
@@ -83,7 +83,7 @@ func getRuntimeSpec(c *cliconfig.PodmanCommand) (*spec.Spec, error) {
 		createConfig, err := parseCreateOpts(c, runtime, "alpine", generateAlpineImageData())
 	*/
 	ctx := getContext()
-	genericResults := shared.NewIntermediateLayer(c)
+	genericResults := shared.NewIntermediateLayer(c, false)
 	createConfig, err := shared.ParseCreateOpts(ctx, &genericResults, nil, "alpine", generateAlpineImageData())
 	if err != nil {
 		return nil, err
diff --git a/cmd/podman/shared/intermediate.go b/cmd/podman/shared/intermediate.go
index 9afbd68c8..2e1827561 100644
--- a/cmd/podman/shared/intermediate.go
+++ b/cmd/podman/shared/intermediate.go
@@ -360,7 +360,7 @@ func newCRStringArray(c *cliconfig.PodmanCommand, flag string) CRStringArray {
 }
 
 // NewIntermediateLayer creates a GenericCLIResults from a create or run cli-command
-func NewIntermediateLayer(c *cliconfig.PodmanCommand) GenericCLIResults {
+func NewIntermediateLayer(c *cliconfig.PodmanCommand, remote bool) GenericCLIResults {
 	m := make(map[string]GenericCLIResult)
 
 	m["add-host"] = newCRStringSlice(c, "add-host")
@@ -458,8 +458,10 @@ func NewIntermediateLayer(c *cliconfig.PodmanCommand) GenericCLIResults {
 	m["volumes-from"] = newCRStringSlice(c, "volumes-from")
 	m["workdir"] = newCRString(c, "workdir")
 	// global flag
-	m["trace"] = newCRBool(c, "trace")
-	m["syslog"] = newCRBool(c, "syslog")
+	if !remote {
+		m["trace"] = newCRBool(c, "trace")
+		m["syslog"] = newCRBool(c, "syslog")
+	}
 
 	return GenericCLIResults{m, c.InputArgs}
 }
diff --git a/cmd/podman/varlink/io.podman.varlink b/cmd/podman/varlink/io.podman.varlink
index ae830f3e6..c6997cd3f 100644
--- a/cmd/podman/varlink/io.podman.varlink
+++ b/cmd/podman/varlink/io.podman.varlink
@@ -658,7 +658,9 @@ method PauseContainer(name: string) -> (container: string)
 # See also [PauseContainer](#PauseContainer).
 method UnpauseContainer(name: string) -> (container: string)
 
-method Attach(name: string) -> ()
+# Attach takes the name or ID of a container and sets up a the ability to remotely attach to its console. The start
+# bool is whether you wish to start the container in question first.
+method Attach(name: string, detachKeys: string, start: bool) -> ()
 
 method AttachControl(name: string) -> ()
 
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index a9b3232e7..d4e8e30d9 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -255,14 +255,14 @@ func (r *LocalRuntime) Log(c *cliconfig.LogsValues, options *libpod.LogOptions)
 
 // CreateContainer creates a libpod container
 func (r *LocalRuntime) CreateContainer(ctx context.Context, c *cliconfig.CreateValues) (string, error) {
-	results := shared.NewIntermediateLayer(&c.PodmanCommand)
+	results := shared.NewIntermediateLayer(&c.PodmanCommand, false)
 	ctr, _, err := shared.CreateContainer(ctx, &results, r.Runtime)
 	return ctr.ID(), err
 }
 
 // Run a libpod container
 func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode int) (int, error) {
-	results := shared.NewIntermediateLayer(&c.PodmanCommand)
+	results := shared.NewIntermediateLayer(&c.PodmanCommand, false)
 
 	ctr, createConfig, err := shared.CreateContainer(ctx, &results, r.Runtime)
 	if err != nil {
diff --git a/pkg/adapter/containers_remote.go b/pkg/adapter/containers_remote.go
index 1ae39749f..9ca823760 100644
--- a/pkg/adapter/containers_remote.go
+++ b/pkg/adapter/containers_remote.go
@@ -331,7 +331,7 @@ func (r *LocalRuntime) CreateContainer(ctx context.Context, c *cliconfig.CreateV
 		// TODO need to add attach when that function becomes available
 		return "", errors.New("the remote client only supports detached containers")
 	}
-	results := shared.NewIntermediateLayer(&c.PodmanCommand)
+	results := shared.NewIntermediateLayer(&c.PodmanCommand, true)
 	return iopodman.CreateContainer().Call(r.Conn, results.MakeVarlink())
 }
 
@@ -344,23 +344,21 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 	// being run.
 
 	// TODO the exit codes for run need to be figured out for remote connections
-	results := shared.NewIntermediateLayer(&c.PodmanCommand)
+	results := shared.NewIntermediateLayer(&c.PodmanCommand, true)
 	cid, err := iopodman.CreateContainer().Call(r.Conn, results.MakeVarlink())
 	if err != nil {
 		return 0, err
 	}
-	_, err = iopodman.StartContainer().Call(r.Conn, cid)
-	if err != nil {
+	if c.Bool("detach") {
+		_, err := iopodman.StartContainer().Call(r.Conn, cid)
+		fmt.Println(cid)
 		return 0, err
 	}
-	errChan, err := r.attach(ctx, os.Stdin, os.Stdout, cid)
+
+	errChan, err := r.attach(ctx, os.Stdin, os.Stdout, cid, true)
 	if err != nil {
 		return 0, err
 	}
-	if c.Bool("detach") {
-		fmt.Println(cid)
-		return 0, err
-	}
 	finalError := <-errChan
 	return 0, finalError
 }
@@ -441,7 +439,7 @@ func (r *LocalRuntime) Ps(c *cliconfig.PsValues, opts shared.PsOptions) ([]share
 	return psContainers, nil
 }
 
-func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid string) (chan error, error) {
+func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid string, start bool) (chan error, error) {
 	var (
 		oldTermState *term.State
 	)
@@ -471,8 +469,8 @@ func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid s
 		term.SetRawTerminal(os.Stdin.Fd())
 
 	}
-
-	_, err = iopodman.Attach().Send(r.Conn, varlink.Upgrade, cid)
+	// TODO add detach keys support
+	_, err = iopodman.Attach().Send(r.Conn, varlink.Upgrade, cid, "", start)
 	if err != nil {
 		restoreTerminal(oldTermState)
 		return nil, err
@@ -533,7 +531,7 @@ func (r *LocalRuntime) Attach(ctx context.Context, c *cliconfig.AttachValues) er
 	if c.NoStdin {
 		inputStream = nil
 	}
-	errChan, err := r.attach(ctx, inputStream, os.Stdout, c.InputArgs[0])
+	errChan, err := r.attach(ctx, inputStream, os.Stdout, c.InputArgs[0], false)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/varlinkapi/attach.go b/pkg/varlinkapi/attach.go
index 53c4d1ff6..9e2a265be 100644
--- a/pkg/varlinkapi/attach.go
+++ b/pkg/varlinkapi/attach.go
@@ -3,29 +3,17 @@
 package varlinkapi
 
 import (
+	"bufio"
 	"io"
 
 	"github.com/containers/libpod/cmd/podman/varlink"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/varlinkapi/virtwriter"
+	"github.com/sirupsen/logrus"
 	"k8s.io/client-go/tools/remotecommand"
 )
 
-// Close is method to close the writer
-
-// Attach ...
-func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string) error {
-	var finalErr error
-	resize := make(chan remotecommand.TerminalSize)
-	errChan := make(chan error)
-
-	if !call.WantsUpgrade() {
-		return call.ReplyErrorOccurred("client must use upgraded connection to attach")
-	}
-	ctr, err := i.Runtime.LookupContainer(name)
-	if err != nil {
-		return call.ReplyErrorOccurred(err.Error())
-	}
+func setupStreams(call iopodman.VarlinkCall) (*bufio.Reader, *bufio.Writer, *io.PipeReader, *io.PipeWriter, *libpod.AttachStreams) {
 
 	// These are the varlink sockets
 	reader := call.Call.Reader
@@ -49,6 +37,24 @@ func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string) error {
 		// Runc eats the error stream
 		AttachError: true,
 	}
+	return reader, writer, pr, pw, &streams
+}
+
+// Attach connects to a containers console
+func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string, detachKeys string, start bool) error {
+	var finalErr error
+	resize := make(chan remotecommand.TerminalSize)
+	errChan := make(chan error)
+
+	if !call.WantsUpgrade() {
+		return call.ReplyErrorOccurred("client must use upgraded connection to attach")
+	}
+	ctr, err := i.Runtime.LookupContainer(name)
+	if err != nil {
+		return call.ReplyErrorOccurred(err.Error())
+	}
+
+	reader, writer, _, pw, streams := setupStreams(call)
 
 	go func() {
 		if err := virtwriter.Reader(reader, nil, nil, pw, resize); err != nil {
@@ -56,20 +62,42 @@ func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string) error {
 		}
 	}()
 
+	if start {
+		finalErr = startAndAttach(ctr, streams, detachKeys, resize, errChan)
+	} else {
+		finalErr = attach(ctr, streams, detachKeys, resize, errChan)
+	}
+
+	if finalErr != libpod.ErrDetach && finalErr != nil {
+		logrus.Error(finalErr)
+	}
+	quitWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.Quit)
+	_, err = quitWriter.Write([]byte("HANG-UP"))
+	// TODO error handling is not quite right here yet
+	return call.Writer.Flush()
+}
+
+func attach(ctr *libpod.Container, streams *libpod.AttachStreams, detachKeys string, resize chan remotecommand.TerminalSize, errChan chan error) error {
 	go func() {
-		// TODO allow for customizable detach keys
-		if err := ctr.Attach(&streams, "", resize); err != nil {
+		if err := ctr.Attach(streams, detachKeys, resize); err != nil {
 			errChan <- err
 		}
 	}()
+	attachError := <-errChan
+	return attachError
+}
 
+func startAndAttach(ctr *libpod.Container, streams *libpod.AttachStreams, detachKeys string, resize chan remotecommand.TerminalSize, errChan chan error) error {
+	var finalErr error
+	attachChan, err := ctr.StartAndAttach(getContext(), streams, detachKeys, resize, false)
+	if err != nil {
+		return err
+	}
 	select {
-	// Blocking on an error
-	case finalErr = <-errChan:
-		// Need to close up shop
-		_ = finalErr
+	case attachChanErr := <-attachChan:
+		finalErr = attachChanErr
+	case chanError := <-errChan:
+		finalErr = chanError
 	}
-	quitWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.Quit)
-	_, err = quitWriter.Write([]byte("HANG-UP"))
-	return call.Writer.Flush()
+	return finalErr
 }