govern remote attach and start

fixes a race where container would start before attach could occur resulting in an error.

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

4 files changed, 15 insertions, 7 deletions

diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go
index f0984b8e3..74f6ded45 100644
--- a/pkg/bindings/containers/containers.go
+++ b/pkg/bindings/containers/containers.go
@@ -346,7 +346,7 @@ func ContainerInit(ctx context.Context, nameOrID string) error {
 }
 
 // Attach attaches to a running container
-func Attach(ctx context.Context, nameOrId string, detachKeys *string, logs, stream *bool, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
+func Attach(ctx context.Context, nameOrId string, detachKeys *string, logs, stream *bool, stdin io.Reader, stdout io.Writer, stderr io.Writer, attachReady chan bool) error {
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return err
@@ -427,6 +427,12 @@ func Attach(ctx context.Context, nameOrId string, detachKeys *string, logs, stre
 		return err
 	}
 	defer response.Body.Close()
+	// If we are attaching around a start, we need to "signal"
+	// back that we are in fact attached so that started does
+	// not execute before we can attach.
+	if attachReady != nil {
+		attachReady <- true
+	}
 	if !(response.IsSuccess() || response.IsInformational()) {
 		return response.Process(nil)
 	}
diff --git a/pkg/bindings/test/attach_test.go b/pkg/bindings/test/attach_test.go
index 906bd2950..6fb166828 100644
--- a/pkg/bindings/test/attach_test.go
+++ b/pkg/bindings/test/attach_test.go
@@ -54,7 +54,7 @@ var _ = Describe("Podman containers attach", func() {
 		go func() {
 			defer GinkgoRecover()
 
-			err := containers.Attach(bt.conn, id, nil, bindings.PTrue, bindings.PTrue, nil, stdout, stderr)
+			err := containers.Attach(bt.conn, id, nil, bindings.PTrue, bindings.PTrue, nil, stdout, stderr, nil)
 			Expect(err).ShouldNot(HaveOccurred())
 		}()
 
@@ -98,7 +98,7 @@ var _ = Describe("Podman containers attach", func() {
 		go func() {
 			defer GinkgoRecover()
 
-			err := containers.Attach(bt.conn, ctnr.ID, nil, bindings.PFalse, bindings.PTrue, stdin, stdout, stderr)
+			err := containers.Attach(bt.conn, ctnr.ID, nil, bindings.PFalse, bindings.PTrue, stdin, stdout, stderr, nil)
 			Expect(err).ShouldNot(HaveOccurred())
 		}()
 
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index cebd332e3..828bfae5b 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -326,7 +326,7 @@ func (ic *ContainerEngine) ContainerLogs(ctx context.Context, containers []strin
 }
 
 func (ic *ContainerEngine) ContainerAttach(ctx context.Context, nameOrId string, options entities.AttachOptions) error {
-	return containers.Attach(ic.ClientCxt, nameOrId, &options.DetachKeys, nil, bindings.PTrue, options.Stdin, options.Stdout, options.Stderr)
+	return containers.Attach(ic.ClientCxt, nameOrId, &options.DetachKeys, nil, bindings.PTrue, options.Stdin, options.Stdout, options.Stderr, nil)
 }
 
 func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, options entities.ExecOptions) (int, error) {
@@ -335,11 +335,14 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, o
 
 func startAndAttach(ic *ContainerEngine, name string, detachKeys *string, input, output, errput *os.File) error { //nolint
 	attachErr := make(chan error)
+	attachReady := make(chan bool)
 	go func() {
-		err := containers.Attach(ic.ClientCxt, name, detachKeys, bindings.PFalse, bindings.PTrue, input, output, errput)
+		err := containers.Attach(ic.ClientCxt, name, detachKeys, bindings.PFalse, bindings.PTrue, input, output, errput, attachReady)
 		attachErr <- err
 	}()
-
+	// Wait for the attach to actually happen before starting
+	// the container.
+	<-attachReady
 	if err := containers.Start(ic.ClientCxt, name, detachKeys); err != nil {
 		return err
 	}
diff --git a/test/e2e/commit_test.go b/test/e2e/commit_test.go
index d3b6fba0c..72387ed8c 100644
--- a/test/e2e/commit_test.go
+++ b/test/e2e/commit_test.go
@@ -217,7 +217,6 @@ var _ = Describe("Podman commit", func() {
 	})
 
 	It("podman commit container check env variables", func() {
-		Skip(v2remotefail)
 		s := podmanTest.Podman([]string{"run", "--name", "test1", "-e", "TEST=1=1-01=9.01", "-it", "alpine", "true"})
 		s.WaitWithDefaultTimeout()
 		Expect(s.ExitCode()).To(Equal(0))