From 1f0e6ba6c6163fb3155a81f1129cead3d3b80f8a Mon Sep 17 00:00:00 2001
From: Spike Curtis <spike@coder.com>
Date: Thu, 18 Jan 2024 16:21:11 +0400
Subject: [PATCH] fix: use raw syscalls to write binary we execute (#11684)

Fixes flake seen here, I think

https://github.com/coder/coder/actions/runs/7565915337/job/20602500818

golang's file processing is complex, and in at least some cases it can return from a file.Close() call without having actually closed the file descriptor.

If we're holding open the file descriptor of an executable we just wrote, and try to execute it, it will fail with "text file busy" which is what we have seen.

So, to be extra sure, I've avoided the standard library and directly called the syscalls to open, write, and close the file we intend to use in the test.

I've also added some more logging so if it's some issue of multiple tests writing to the same location, the we might have a chance to see it.
---
 provisioner/terraform/executor.go       |  8 ++++++++
 provisioner/terraform/provision_test.go | 13 ++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go
index 3917e4ca15..0a6c1df943 100644
--- a/provisioner/terraform/executor.go
+++ b/provisioner/terraform/executor.go
@@ -123,6 +123,10 @@ func (e *executor) execParseJSON(ctx, killCtx context.Context, args, env []strin
 	cmd.Stdout = out
 	cmd.Stderr = stdErr
 
+	e.server.logger.Debug(ctx, "executing terraform command with JSON result",
+		slog.F("binary_path", e.binaryPath),
+		slog.F("args", args),
+	)
 	err := cmd.Start()
 	if err != nil {
 		return err
@@ -348,6 +352,10 @@ func (e *executor) graph(ctx, killCtx context.Context) (string, error) {
 	cmd.Dir = e.workdir
 	cmd.Env = e.basicEnv()
 
+	e.server.logger.Debug(ctx, "executing terraform command graph",
+		slog.F("binary_path", e.binaryPath),
+		slog.F("args", "graph"),
+	)
 	err := cmd.Start()
 	if err != nil {
 		return "", err
diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go
index 4c2187ced7..85868fe611 100644
--- a/provisioner/terraform/provision_test.go
+++ b/provisioner/terraform/provision_test.go
@@ -14,6 +14,7 @@ import (
 	"runtime"
 	"sort"
 	"strings"
+	"syscall"
 	"testing"
 	"time"
 
@@ -165,8 +166,18 @@ func TestProvision_Cancel(t *testing.T) {
 
 			// Example: exec /path/to/terrafork_fake_cancel.sh 1.2.1 apply "$@"
 			content := fmt.Sprintf("#!/bin/sh\nexec %q %s %s \"$@\"\n", fakeBin, terraform.TerraformVersion.String(), tt.mode)
-			err := os.WriteFile(binPath, []byte(content), 0o755) //#nosec
+
+			// golang's standard OS library can sometimes leave the file descriptor open even after
+			// "Closing" the file (which can then lead to a "text file busy" error, so we bypass this
+			// and use syscall directly).
+			fd, err := syscall.Open(binPath, syscall.O_WRONLY|syscall.O_CREAT, 0o755)
 			require.NoError(t, err)
+			n, err := syscall.Write(fd, []byte(content))
+			require.NoError(t, err)
+			require.Equal(t, len(content), n)
+			err = syscall.Close(fd)
+			require.NoError(t, err)
+			t.Logf("wrote fake terraform script to %s", binPath)
 
 			ctx, api := setupProvisioner(t, &provisionerServeOptions{
 				binaryPath: binPath,