5824: Fix Keep server shutdown, check errors, simplify stderr redirection.

(Oops, we forgot to actually Run() the python command for stop_keep.)

diff --git a/sdk/go/arvadostest/run_servers.go b/sdk/go/arvadostest/run_servers.go
index 27c552a4e104094ca2ed15991e310a3b7e9cd65e..c61b68b319fbe50b5e71ea97e5751da57af687d9 100644 (file)
--- a/sdk/go/arvadostest/run_servers.go
+++ b/sdk/go/arvadostest/run_servers.go
@@ -3,9 +3,6 @@ package arvadostest
 import (
        "bufio"
        "bytes"
-       "fmt"
-       "io"
-       "io/ioutil"
        "log"
        "os"
        "os/exec"
@@ -68,24 +65,12 @@ func StartAPI() {
        chdirToPythonTests()
 
        cmd := exec.Command("python", "run_test_server.py", "start", "--auth", "admin")
-       stderr, err := cmd.StderrPipe()
-       if err != nil {
-               log.Fatal(err)
-       }
-       go io.Copy(os.Stderr, stderr)
-       stdout, err := cmd.StdoutPipe()
+       cmd.Stdin = nil
+       cmd.Stderr = os.Stderr
+
+       authScript, err := cmd.Output()
        if err != nil {
-               log.Fatal(err)
-       }
-       if err = cmd.Start(); err != nil {
-               log.Fatal(err)
-       }
-       var authScript []byte
-       if authScript, err = ioutil.ReadAll(stdout); err != nil {
-               log.Fatal(err)
-       }
-       if err = cmd.Wait(); err != nil {
-               log.Fatal(err)
+               log.Fatalf("%+v: %s", cmd.Args, err)
        }
        ParseAuthSettings(authScript)
        ResetEnv()
@@ -96,7 +81,7 @@ func StopAPI() {
        defer os.Chdir(cwd)
        chdirToPythonTests()
 
-       exec.Command("python", "run_test_server.py", "stop").Run()
+       bgRun(exec.Command("python", "run_test_server.py", "stop"))
 }
 
 // StartKeep starts the given number of keep servers,
@@ -112,16 +97,7 @@ func StartKeep(numKeepServers int, enforcePermissions bool) {
                cmdArgs = append(cmdArgs, "--keep-enforce-permissions")
        }
 
-       cmd := exec.Command("python", cmdArgs...)
-
-       stderr, err := cmd.StderrPipe()
-       if err != nil {
-               log.Fatalf("Setting up stderr pipe: %s", err)
-       }
-       go io.Copy(os.Stderr, stderr)
-       if err := cmd.Run(); err != nil {
-               panic(fmt.Sprintf("'python run_test_server.py start_keep' returned error %s", err))
-       }
+       bgRun(exec.Command("python", cmdArgs...))
 }
 
 // StopKeep stops keep servers that were started with StartKeep.
@@ -132,5 +108,27 @@ func StopKeep(numKeepServers int) {
        defer os.Chdir(cwd)
        chdirToPythonTests()
 
-       exec.Command("python", "run_test_server.py", "stop_keep", "--num-keep-servers", strconv.Itoa(numKeepServers))
+       cmd := exec.Command("python", "run_test_server.py", "stop_keep", "--num-keep-servers", strconv.Itoa(numKeepServers))
+       cmd.Stdin = nil
+       cmd.Stderr = os.Stderr
+       cmd.Stdout = os.Stderr
+       if err := cmd.Run(); err != nil {
+               log.Fatalf("%+v: %s", cmd.Args, err)
+       }
+}
+
+// Start cmd, with stderr and stdout redirected to our own
+// stderr. Return when the process exits, but do not wait for its
+// stderr and stdout to close: any grandchild processes will continue
+// writing to our stderr.
+func bgRun(cmd *exec.Cmd) {
+       cmd.Stdin = nil
+       cmd.Stderr = os.Stderr
+       cmd.Stdout = os.Stderr
+       if err := cmd.Start(); err != nil {
+               log.Fatalf("%+v: %s", cmd.Args, err)
+       }
+       if _, err := cmd.Process.Wait(); err != nil {
+               log.Fatalf("%+v: %s", cmd.Args, err)
+       }
 }

diff --git a/services/keepstore/pull_worker_integration_test.go b/services/keepstore/pull_worker_integration_test.go
index 3a3069ab7745c1efc0a21fd8c706565f65c525e0..52b59ec1ef797b1e09b25529b20285f5190166df 100644 (file)
--- a/services/keepstore/pull_worker_integration_test.go
+++ b/services/keepstore/pull_worker_integration_test.go
@@ -82,6 +82,8 @@ func TestPullWorkerIntegration_GetNonExistingLocator(t *testing.T) {
        }
 
        pullRequest := SetupPullWorkerIntegrationTest(t, testData, false)
+       defer arvadostest.StopAPI()
+       defer arvadostest.StopKeep(2)
 
        performPullWorkerIntegrationTest(testData, pullRequest, t)
 }
@@ -97,6 +99,8 @@ func TestPullWorkerIntegration_GetExistingLocator(t *testing.T) {
        }
 
        pullRequest := SetupPullWorkerIntegrationTest(t, testData, true)
+       defer arvadostest.StopAPI()
+       defer arvadostest.StopKeep(2)
 
        performPullWorkerIntegrationTest(testData, pullRequest, t)
 }

diff --git a/tools/keep-rsync/keep-rsync_test.go b/tools/keep-rsync/keep-rsync_test.go
index 9432a0d383b534236ff506c83ac98ed0c196c324..94281fa8bcbb89f5432614adf715376ad666beab 100644 (file)
--- a/tools/keep-rsync/keep-rsync_test.go
+++ b/tools/keep-rsync/keep-rsync_test.go
@@ -470,9 +470,11 @@ func (s *DoMainTestSuite) Test_doMainWithSrcAndDstConfig(c *C) {
        args := []string{"-src", srcConfig.Name(), "-dst", dstConfig.Name()}
        os.Args = append(os.Args, args...)
 
-       // Start keepservers. Since we are not doing any tweaking as in setupRsync func,
-       // kcSrc and kcDst will be the same and no actual copying to dst will happen, but that's ok.
+       // Start keepservers. Since we are not doing any tweaking as
+       // in setupRsync func, kcSrc and kcDst will be the same and no
+       // actual copying to dst will happen, but that's ok.
        arvadostest.StartKeep(2, false)
+       defer arvadostest.StopKeep(2)
 
        err := doMain()
        c.Check(err, IsNil)