From ba0bb5d5030a06a00f38779fbc9feb609c41dfee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Valais?= Date: Fri, 25 Nov 2022 12:04:52 +0100 Subject: [PATCH] e2e: the vault addon was incorrectly using StdoutPipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation [1] mentions that `StdoutPipe` should not be used along with `Run`: "Wait will close the pipe after seeing the command exit, so most callers need not close the pipe themselves. It is thus incorrect to call Wait before all reads from the pipe have completed. For the same reason, it is incorrect to call Run when using StdoutPipe. See the example for idiomatic usage." It seems we are using `Run`, meaning that the StdoutPipe gets closed when `Run` returns (because `Run` calls `Wait` and closes the StdoutPipe before returning). To reproduce: git fetch fa4c2cfcad79f0a8a806b71caefbf96b049533c5 git checkout fa4c2cfcad79f0a8a806b71caefbf96b049533c5 go test -tags=e2e_test ./test/e2e -- -test.outputdir=$PWD/_bin/artifacts \ -ginkgo.junit-report=junit__01.xml -ginkgo.flake-attempts=1 \ -test.timeout=24h -ginkgo.v -test.v -ginkgo.randomize-all \ -ginkgo.progress -ginkgo.trace -ginkgo.slow-spec-threshold=300s \ --repo-root=/home/mvalais/code/cert-manager \ --report-dir=/home/mvalais/code/cert-manager/_bin/artifacts \ --acme-dns-server=10.0.0.16 --acme-ingress-ip=10.0.0.15 \ --acme-gateway-ip=10.0.0.14 \ --ingress-controller-domain=ingress-nginx.http01.example.com \ --gateway-domain=gateway.http01.example.com \ --feature-gates="" \ --ginkgo.focus=".*should be ready with a valid serviceAccountRef" Result: error install helm chart: cmd.Run: exit status 1: io.Copy: write /dev/stdout: copy_file_range: use of closed file Signed-off-by: Maël Valais --- test/e2e/framework/addon/chart/addon.go | 37 +++++++++---------------- 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/test/e2e/framework/addon/chart/addon.go b/test/e2e/framework/addon/chart/addon.go index 613c75a9e..e03f22da2 100644 --- a/test/e2e/framework/addon/chart/addon.go +++ b/test/e2e/framework/addon/chart/addon.go @@ -17,6 +17,7 @@ limitations under the License. package chart import ( + "bytes" "context" "fmt" "io" @@ -164,16 +165,12 @@ func (c *Chart) runInstall() error { } cmd := c.buildHelmCmd(args...) - cmd.Stdout = nil - out, err := cmd.StdoutPipe() - if err != nil { - return err - } - defer out.Close() + stdoutBuf := &bytes.Buffer{} + cmd.Stdout = stdoutBuf - err = cmd.Run() + err := cmd.Run() if err != nil { - _, err2 := io.Copy(os.Stdout, out) + _, err2 := io.Copy(os.Stdout, stdoutBuf) if err2 != nil { return fmt.Errorf("cmd.Run: %v: io.Copy: %v", err, err2) } @@ -197,19 +194,15 @@ func (c *Chart) buildHelmCmd(args ...string) *exec.Cmd { func (c *Chart) getHelmVersion() (string, error) { cmd := c.buildHelmCmd("version", "--template", "{{.Client.Version}}") - cmd.Stdout = nil - out, err := cmd.StdoutPipe() - if err != nil { - return "", err - } - defer out.Close() + stdoutBuf := &bytes.Buffer{} + cmd.Stdout = stdoutBuf - err = cmd.Run() + err := cmd.Run() if err != nil { return "", err } - outBytes, err := io.ReadAll(out) + outBytes, err := io.ReadAll(stdoutBuf) if err != nil { return "", err } @@ -220,16 +213,12 @@ func (c *Chart) getHelmVersion() (string, error) { // Deprovision the deployed instance of tiller-deploy func (c *Chart) Deprovision() error { cmd := c.buildHelmCmd("delete", "--namespace", c.Namespace, c.ReleaseName) - cmd.Stdout = nil - out, err := cmd.StdoutPipe() - if err != nil { - return err - } - defer out.Close() + stdoutBuf := &bytes.Buffer{} + cmd.Stdout = stdoutBuf - err = cmd.Run() + err := cmd.Run() if err != nil { - _, err2 := io.Copy(os.Stdout, out) + _, err2 := io.Copy(os.Stdout, stdoutBuf) if err2 != nil { return fmt.Errorf("cmd.Run: %v: io.Copy: %v", err, err2) }