Update e2e tests for remote exec

including changing -l to the container id
and separating a case of setting the env that remote can't handle

Signed-off-by: Peter Hunt <pehunt@redhat.com>

4 files changed, 48 insertions, 43 deletions

diff --git a/pkg/adapter/containers_remote.go b/pkg/adapter/containers_remote.go
index 665821bb6..7a9cf94b7 100644
--- a/pkg/adapter/containers_remote.go
+++ b/pkg/adapter/containers_remote.go
@@ -1054,15 +1054,14 @@ func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecVal
 	if err != nil {
 		return ec, errors.Wrapf(err, "Exec operation failed for %s", cli.InputArgs)
 	}
-
 	ecChan := make(chan int, 1)
 	errChan := configureVarlinkAttachStdio(r.Conn.Reader, r.Conn.Writer, inputStream, os.Stdout, oldTermState, resize, ecChan)
 
 	select {
-		case err = <-errChan:
-			break
-		case ec = <-ecChan:
-			break
+	case ec = <-ecChan:
+		break
+	case err = <-errChan:
+		break
 	}
 
 	return ec, err
diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go
index 4714f3fa0..a70a87675 100644
--- a/pkg/varlinkapi/containers.go
+++ b/pkg/varlinkapi/containers.go
@@ -800,51 +800,46 @@ func (i *LibpodAPI) ExecContainer(call iopodman.VarlinkCall, opts iopodman.ExecO
 	call.ReplyExecContainer()
 
 	resizeChan := make(chan remotecommand.TerminalSize)
-	errChan := make(chan error)
 
 	reader, writer, _, pipeWriter, streams := setupStreams(call)
 	//reader, _, _, pipeWriter, streams := setupStreams(call)
-	ecChan := make(chan uint32, 1)
+	type ExitCodeError struct {
+		ExitCode uint32
+		Error    error
+	}
+	ecErrChan := make(chan ExitCodeError, 1)
 
 	go func() {
-		fmt.Printf("ExecContainer Start Reader\n")
 		if err := virtwriter.Reader(reader, nil, nil, pipeWriter, resizeChan, nil); err != nil {
-			//fmt.Printf("ExecContainer Reader err %s, %s\n", err.Error(), errors.Cause(err).Error())
-			errChan <- errors.Wrapf(err, "error")
+			ecErrChan <- ExitCodeError{
+				125, //TODO FIXME magic number, define package?
+				err,
+			}
 		}
 	}()
 
-	fmt.Printf("ExecContainer Start ctr.Exec\n")
-	// TODO detach keys and resize
-	// TODO add handling for exit code
-	// TODO capture exit code and return to main thread
+	// TODO FIXME detach keys
 	go func() {
 		ec, err := ctr.Exec(opts.Tty, opts.Privileged, envs, opts.Cmd, user, workDir, streams, 0, resizeChan, "")
 		if err != nil {
 			logrus.Errorf("ExecContainer Exec err %s, %s\n", err.Error(), errors.Cause(err).Error())
-			errChan <-err
 		}
-		ecChan <-uint32(ec)
-
+		ecErrChan <- ExitCodeError{
+			uint32(ec),
+			err,
+		}
 	}()
 
-	ec := uint32(125)
-	var execErr error
-	select {
-		case execErr = <-errChan:
-			fmt.Println(execErr.Error())
-		case ec = <-ecChan:
-			fmt.Println("found", ec)
-	}
+	ecErr := <-ecErrChan
 
-	// TODO FIXME prevent all of vthese conversions
-	if err = virtwriter.HangUp(writer, int(ec)); err != nil {
+	// TODO FIXME prevent all of these conversions
+	if err = virtwriter.HangUp(writer, int(ecErr.ExitCode)); err != nil {
 		logrus.Errorf("ExecContainer failed to HANG-UP on %s: %s", ctr.ID(), err.Error())
 	}
-	defer fmt.Println("Succeeded in exec'ing")
+
 	if err := call.Writer.Flush(); err != nil {
 		logrus.Errorf("Exec Container err: %s", err.Error())
 	}
 
-	return execErr
+	return ecErr.Error
 }
diff --git a/pkg/varlinkapi/virtwriter/virtwriter.go b/pkg/varlinkapi/virtwriter/virtwriter.go
index 23f945704..8cdedeed8 100644
--- a/pkg/varlinkapi/virtwriter/virtwriter.go
+++ b/pkg/varlinkapi/virtwriter/virtwriter.go
@@ -113,7 +113,7 @@ func Reader(r *bufio.Reader, output, errput, input io.Writer, resize chan remote
 			if output != nil {
 				_, err := io.CopyN(output, r, messageSize)
 				if err != nil {
-						return errors.Wrapf(err, "issue stdout")
+					return errors.Wrapf(err, "issue stdout")
 				}
 			}
 		case ToStderr:
@@ -121,14 +121,14 @@ func Reader(r *bufio.Reader, output, errput, input io.Writer, resize chan remote
 				_, err := io.CopyN(errput, r, messageSize)
 				if err != nil {
 					return err
-						return errors.Wrapf(err, "issue stderr")
+					return errors.Wrapf(err, "issue stderr")
 				}
 			}
 		case ToStdin:
 			if input != nil {
 				_, err := io.CopyN(input, r, messageSize)
 				if err != nil {
-						return errors.Wrapf(err, "issue stdin")
+					return errors.Wrapf(err, "issue stdin")
 				}
 			}
 		case TerminalResize:
@@ -159,7 +159,7 @@ func Reader(r *bufio.Reader, output, errput, input io.Writer, resize chan remote
 			}
 			if execEcChan != nil {
 				ecInt := binary.BigEndian.Uint32(out)
-				execEcChan <-int(ecInt)
+				execEcChan <- int(ecInt)
 			}
 			return nil
 
@@ -175,8 +175,7 @@ func HangUp(writer *bufio.Writer, ec int) (err error) {
 	n := 0
 	msg := make([]byte, 4)
 
-	binary.LittleEndian.PutUint32(msg, uint32(ec))
-
+	binary.BigEndian.PutUint32(msg, uint32(ec))
 
 	writeQuit := NewVirtWriteCloser(writer, Quit)
 	if n, err = writeQuit.Write(msg); err != nil {
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index 316575b0f..bd4fb7aad 100644
--- a/test/e2e/exec_test.go
+++ b/test/e2e/exec_test.go
@@ -65,6 +65,8 @@ var _ = Describe("Podman exec", func() {
 	})
 
 	It("podman exec simple command using latest", func() {
+		// the remote client doesn't use latest
+		SkipIfRemote()
 		setup := podmanTest.RunTopContainer("test1")
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
@@ -75,27 +77,37 @@ var _ = Describe("Podman exec", func() {
 	})
 
 	It("podman exec environment test", func() {
+		// passing environment variables is not supported in the remote client
+		SkipIfRemote()
 		setup := podmanTest.RunTopContainer("test1")
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
 
-		session := podmanTest.Podman([]string{"exec", "-l", "--env", "FOO=BAR", "printenv", "FOO"})
+		session := podmanTest.Podman([]string{"exec", "--env", "FOO=BAR", "test1", "printenv", "FOO"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ := session.GrepString("BAR")
 		Expect(match).Should(BeTrue())
 
-		session = podmanTest.Podman([]string{"exec", "-l", "--env", "PATH=/bin", "printenv", "PATH"})
+		session = podmanTest.Podman([]string{"exec", "--env", "PATH=/bin", "test1", "printenv", "PATH"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ = session.GrepString("/bin")
 		Expect(match).Should(BeTrue())
+	})
+
+	It("podman exec os.Setenv env", func() {
+		// remote doesn't properly interpret os.Setenv
+		SkipIfRemote()
+		setup := podmanTest.RunTopContainer("test1")
+		setup.WaitWithDefaultTimeout()
+		Expect(setup.ExitCode()).To(Equal(0))
 
 		os.Setenv("FOO", "BAR")
-		session = podmanTest.Podman([]string{"exec", "-l", "--env", "FOO", "printenv", "FOO"})
+		session := podmanTest.Podman([]string{"exec", "--env", "FOO", "test1", "printenv", "FOO"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
-		match, _ = session.GrepString("BAR")
+		match, _ := session.GrepString("BAR")
 		Expect(match).Should(BeTrue())
 		os.Unsetenv("FOO")
 
@@ -141,13 +153,13 @@ var _ = Describe("Podman exec", func() {
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
 
-		session := podmanTest.Podman([]string{"exec", "-l", "--workdir", "/tmp", "pwd"})
+		session := podmanTest.Podman([]string{"exec", "--workdir", "/tmp", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ := session.GrepString("/tmp")
 		Expect(match).Should(BeTrue())
 
-		session = podmanTest.Podman([]string{"exec", "-l", "-w", "/tmp", "pwd"})
+		session = podmanTest.Podman([]string{"exec", "-w", "/tmp", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ = session.GrepString("/tmp")
@@ -159,11 +171,11 @@ var _ = Describe("Podman exec", func() {
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
 
-		session := podmanTest.Podman([]string{"exec", "-l", "--workdir", "/missing", "pwd"})
+		session := podmanTest.Podman([]string{"exec", "--workdir", "/missing", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(1))
 
-		session = podmanTest.Podman([]string{"exec", "-l", "-w", "/missing", "pwd"})
+		session = podmanTest.Podman([]string{"exec", "-w", "/missing", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(1))
 	})