robustness: minor cleanups (#726)

Remove unnecessary intermediate variables.
Send SIGTERM instead of SIGKILL to terminate child kopia server process.
Set Pdeathsig on Linux for child kopia server process.
Trivial: reduce scope of hostFioDataPathStr variable.
Trivial: rename local variable.
Trivial: Use log.Fatalln instead of log + exit(1).
Improve error message in robustness test to tell apart failure cause.

tests/robustness/engine/engine.go

@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/kopia/kopia/tests/robustness/checker"
@@ -333,7 +334,11 @@ func (e *Engine) InitFilesystemWithServer(ctx context.Context, testRepoPath, met
 
 // cleanUpServer cleans up the server process.
 func (e *Engine) cleanUpServer() {
-	if e.serverCmd != nil {
-		e.serverCmd.Process.Kill() // nolint:errcheck
+	if e.serverCmd == nil {
+		return
+	}
+
+	if err := e.serverCmd.Process.Signal(syscall.SIGTERM); err != nil {
+		log.Println("Failed to send termination signal to kopia server process:", err)
 	}
 }

tests/robustness/robustness_test/main_test.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"errors"
 	"flag"
-	"fmt"
 	"log"
 	"os"
 	"path"
@@ -39,12 +38,14 @@ func TestMain(m *testing.M) {
 	eng, err = engine.NewEngine("")
 
 	switch {
-	case err == kopiarunner.ErrExeVariableNotSet || errors.Is(err, fio.ErrEnvNotSet):
-		fmt.Println("Skipping robustness tests if KOPIA_EXE is not set")
+	case err == kopiarunner.ErrExeVariableNotSet:
+		log.Println("Skipping robustness tests because KOPIA_EXE is not set")
+		os.Exit(0)
+	case errors.Is(err, fio.ErrEnvNotSet):
+		log.Println("Skipping robustness tests because FIO environment is not set")
 		os.Exit(0)
 	case err != nil:
-		fmt.Printf("error on engine creation: %s\n", err.Error())
-		os.Exit(1)
+		log.Fatalln("error on engine creation:", err)
 	}
 
 	dataRepoPath := path.Join(*repoPathPrefix, dataSubPath)
@@ -59,16 +60,14 @@ func TestMain(m *testing.M) {
 		// Clean the temporary dirs from the file system, don't write out the
 		// metadata, in case there was an issue loading it
 		eng.CleanComponents()
-		fmt.Printf("error initializing engine for S3: %s\n", err.Error())
-		os.Exit(1)
+		log.Fatalln("error initializing engine for S3:", err)
 	}
 
 	// Restore a random snapshot into the data directory
 	_, err = eng.ExecAction(engine.RestoreIntoDataDirectoryActionKey, nil)
 	if err != nil && err != engine.ErrNoOp {
 		eng.Cleanup()
-		fmt.Printf("error restoring into the data directory: %s\n", err.Error())
-		os.Exit(1)
+		log.Fatalln("error restoring into the data directory:", err)
 	}
 
 	result := m.Run()

tests/tools/fio/fio.go

@@ -68,16 +68,15 @@ type Runner struct {
 func NewRunner() (fr *Runner, err error) {
 	exeStr := os.Getenv(FioExeEnvKey)
 	imgStr := os.Getenv(FioDockerImageEnvKey)
-	localFioDataPathStr := os.Getenv(LocalFioDataPathEnvKey)
-	hostFioDataPathStr := os.Getenv(HostFioDataPathEnvKey)
+	localDataPath := os.Getenv(LocalFioDataPathEnvKey)
 
 	var exeArgs []string
 
 	var fioWriteBaseDir string
 
 	var Exe string
 
-	dataDir, err := ioutil.TempDir(localFioDataPathStr, "fio-data-")
+	dataDir, err := ioutil.TempDir(localDataPath, "fio-data-")
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to create temp directory for fio runner")
 	}
@@ -98,6 +97,7 @@ func NewRunner() (fr *Runner, err error) {
 
 		// If the host path wasn't provided, assume it's the same as the local
 		// data directory path and we are not running from within a container already
+		hostFioDataPathStr := os.Getenv(HostFioDataPathEnvKey)
 		if hostFioDataPathStr == "" {
 			hostFioDataPathStr = dataDirParent
 		}
@@ -144,7 +144,7 @@ func NewRunner() (fr *Runner, err error) {
 		log.Printf("   Set %s (=%q)to the fio executable\n", FioExeEnvKey, exeStr)
 		log.Printf("   - OR -\n")
 		log.Printf("   Set %s (=%q) to the fio docker image", FioDockerImageEnvKey, imgStr)
-		log.Printf("   Set %s (=%q) to the path where fio data will be used locally", LocalFioDataPathEnvKey, localFioDataPathStr)
+		log.Printf("   Set %s (=%q) to the path where fio data will be used locally", LocalFioDataPathEnvKey, localDataPath)
 		log.Printf("   Set %s (=%q) to the fio data path on the docker host (defaults to %v, if not running in a dev container)", HostFioDataPathEnvKey, os.Getenv(HostFioDataPathEnvKey), LocalFioDataPathEnvKey)
 
 		return nil, errors.Wrap(err, "fio setup could not be validated")

tests/tools/kopiarunner/kopiarun.go

@@ -79,15 +79,14 @@ func (kr *Runner) Run(args ...string) (stdout, stderr string, err error) {
 
 // RunAsync will execute the kopia command with the given args in background.
 func (kr *Runner) RunAsync(args ...string) (*exec.Cmd, error) {
-	argsStr := strings.Join(args, " ")
-	log.Printf("running async '%s %v'", kr.Exe, argsStr)
+	log.Printf("running async '%s %v'", kr.Exe, strings.Join(args, " "))
 	cmdArgs := append(append([]string(nil), kr.fixedArgs...), args...)
 	//nolint:gosec //G204
 	c := exec.Command(kr.Exe, cmdArgs...)
 	c.Env = append(os.Environ(), kr.environment...)
+	c.Stderr = &bytes.Buffer{}
 
-	errOut := &bytes.Buffer{}
-	c.Stderr = errOut
+	setpdeath(c)
 
 	err := c.Start()
 	if err != nil {

tests/tools/kopiarunner/setpdeath.go

@@ -0,0 +1,9 @@
+// +build !linux
+
+package kopiarunner
+
+import "os/exec"
+
+func setpdeath(c *exec.Cmd) *exec.Cmd {
+	return c
+}

tests/tools/kopiarunner/setpdeath_linux.go

@@ -0,0 +1,20 @@
+package kopiarunner
+
+import (
+	"os/exec"
+	"syscall"
+)
+
+func setpdeath(c *exec.Cmd) *exec.Cmd {
+	if c == nil {
+		return nil
+	}
+
+	if c.SysProcAttr == nil {
+		c.SysProcAttr = &syscall.SysProcAttr{}
+	}
+
+	c.SysProcAttr.Pdeathsig = syscall.SIGTERM
+
+	return c
+}