From d5641d154cea818723dd388fc120b460d6da0bc6 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 26 Jun 2024 11:04:20 +0200 Subject: [PATCH] Refactor PTTY handling in compose commands (#1924) Use the helper `pty.Start()` instead of handling ourselves the tty and associated ptty. Wait for the finalization of the command just with the copy, to avoid a separate goroutine in an attempt to avoid the hangs we are seeing in CI. Downgrade to pty 1.1.19, that seems to solve some kind of race condition creack/pty#196 that could be producing the hangs. Co-authored-by: Mario Rodriguez Molins --- go.mod | 2 +- go.sum | 4 ++-- internal/compose/compose_other.go | 28 ++++++++-------------------- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index d249475f0..2e0c6d1ea 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/boumenot/gocover-cobertura v1.2.0 github.com/cbroglie/mustache v1.4.0 github.com/cespare/xxhash/v2 v2.3.0 - github.com/creack/pty v1.1.21 + github.com/creack/pty v1.1.19 github.com/dustin/go-humanize v1.0.1 github.com/elastic/elastic-integration-corpus-generator-tool v0.10.0 github.com/elastic/go-elasticsearch/v7 v7.17.10 diff --git a/go.sum b/go.sum index d611dc7d8..05c527bf5 100644 --- a/go.sum +++ b/go.sum @@ -71,8 +71,8 @@ github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBS github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/creack/pty v1.1.21 h1:1/QdRyBaHHJP61QkWMXlOIBfsgdDeeKfK8SYVUWJKf0= -github.com/creack/pty v1.1.21/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= +github.com/creack/pty v1.1.19 h1:tUN6H7LWqNx4hQVxomd0CVsDwaDr9gaRQaI4GpSmrsA= +github.com/creack/pty v1.1.19/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/creasty/defaults v1.7.0 h1:eNdqZvc5B509z18lD8yc212CAqJNvfT1Jq6L8WowdBA= github.com/creasty/defaults v1.7.0/go.mod h1:iGzKe6pbEHnpMPtfDXZEr0NVxWnPTjb1bbDy08fPzYM= github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= diff --git a/internal/compose/compose_other.go b/internal/compose/compose_other.go index da937b138..342a8df2e 100644 --- a/internal/compose/compose_other.go +++ b/internal/compose/compose_other.go @@ -13,7 +13,6 @@ import ( "io" "os" "os/exec" - "sync" "github.com/creack/pty" @@ -30,14 +29,9 @@ func (p *Project) runDockerComposeCmd(ctx context.Context, opts dockerComposeOpt } cmd.Env = append(os.Environ(), opts.env...) - ptty, tty, err := pty.Open() - if err != nil { - return fmt.Errorf("failed to open pseudo-tty to capture stderr: %w", err) - } - var errBuffer bytes.Buffer - cmd.Stderr = tty var stderr io.Writer = &errBuffer + cmd.Stdout = io.Discard if logger.IsDebugMode() { cmd.Stdout = os.Stdout stderr = io.MultiWriter(&errBuffer, os.Stderr) @@ -46,22 +40,16 @@ func (p *Project) runDockerComposeCmd(ctx context.Context, opts dockerComposeOpt cmd.Stdout = opts.stdout } - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - io.Copy(stderr, ptty) - }() - + ptty, err := pty.Start(cmd) + if err != nil { + return fmt.Errorf("failed to start command with pseudo-tty: %w", err) + } + defer ptty.Close() logger.Debugf("running command: %s", cmd) - err = cmd.Run() - tty.Close() - wg.Wait() - // Don't close the PTTY before the goroutine with the Copy has finished. - ptty.Close() + io.Copy(stderr, ptty) - if err != nil { + if err := cmd.Wait(); err != nil { if msg := cleanComposeError(errBuffer.String()); len(msg) > 0 { return fmt.Errorf("%w: %s", err, msg) }