From ce24e1139c63b35ddc2c6b176e5701de1dc9f8b5 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Wed, 20 May 2020 12:49:55 -0500 Subject: 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 --- pkg/bindings/containers/containers.go | 8 +++++++- pkg/bindings/test/attach_test.go | 4 ++-- pkg/domain/infra/tunnel/containers.go | 9 ++++++--- test/e2e/commit_test.go | 1 - 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)) -- cgit v1.2.3-54-g00ecf