summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrent Baude <bbaude@redhat.com>2020-05-20 12:49:55 -0500
committerBrent Baude <bbaude@redhat.com>2020-05-20 13:02:18 -0500
commitce24e1139c63b35ddc2c6b176e5701de1dc9f8b5 (patch)
tree4e14cca7c4f9c795bdd83008b66d0bf1680c37e2
parent09f8f14b4f7d09946d3d5cfc5460ec9923f7da59 (diff)
downloadpodman-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.go8
-rw-r--r--pkg/bindings/test/attach_test.go4
-rw-r--r--pkg/domain/infra/tunnel/containers.go9
-rw-r--r--test/e2e/commit_test.go1
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))