diff options
author | Brent Baude <bbaude@redhat.com> | 2020-05-20 12:49:55 -0500 |
---|---|---|
committer | Brent Baude <bbaude@redhat.com> | 2020-05-20 13:02:18 -0500 |
commit | ce24e1139c63b35ddc2c6b176e5701de1dc9f8b5 (patch) | |
tree | 4e14cca7c4f9c795bdd83008b66d0bf1680c37e2 | |
parent | 09f8f14b4f7d09946d3d5cfc5460ec9923f7da59 (diff) | |
download | podman-ce24e1139c63b35ddc2c6b176e5701de1dc9f8b5.tar.gz podman-ce24e1139c63b35ddc2c6b176e5701de1dc9f8b5.tar.bz2 podman-ce24e1139c63b35ddc2c6b176e5701de1dc9f8b5.zip |
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>
-rw-r--r-- | pkg/bindings/containers/containers.go | 8 | ||||
-rw-r--r-- | pkg/bindings/test/attach_test.go | 4 | ||||
-rw-r--r-- | pkg/domain/infra/tunnel/containers.go | 9 | ||||
-rw-r--r-- | 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)) |