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 <[email protected]>
elastic · Jun 26, 2024 · d5641d1 · d5641d1
1 parent 364fd63
commit d5641d1
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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)
 		}