[ARVADOS] updated: 83a4494e66f4f7447091779f25e6f202b2379de9

Git user git at public.curoverse.com
Wed Jul 27 17:35:13 EDT 2016


Summary of changes:
 sdk/go/arvadostest/fixtures.go                     |  5 +-
 sdk/go/arvadostest/run_servers.go                  | 30 +++++++-
 .../crunch-dispatch-slurm/crunch-dispatch-slurm.go | 67 +++++++++++++----
 .../crunch-dispatch-slurm_test.go                  | 86 +++++++++++++++++++++-
 services/datamanager/datamanager_test.go           | 39 +++++-----
 5 files changed, 188 insertions(+), 39 deletions(-)

       via  83a4494e66f4f7447091779f25e6f202b2379de9 (commit)
       via  86e1730f97383b3ae1685445323aa253b99ee821 (commit)
      from  7ba6bdf406546ec225baea49dbe6ccbf02e70f53 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 83a4494e66f4f7447091779f25e6f202b2379de9
Merge: 7ba6bdf 86e1730
Author: radhika <radhika at curoverse.com>
Date:   Wed Jul 27 17:15:00 2016 -0400

    closes #9581
    Merge branch '9581-slurm-dispatcher-config'


commit 86e1730f97383b3ae1685445323aa253b99ee821
Author: radhika <radhika at curoverse.com>
Date:   Tue Jul 26 12:28:27 2016 -0400

    9581: add json config file handling to slurm dispatcher.

diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index 84a3bff..ebb5992 100644
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -18,8 +18,8 @@ const (
 	Dispatch1AuthUUID = "zzzzz-gj3su-k9dvestay1plssr"
 )
 
-// A valid manifest designed to test various edge cases and parsing
-// requirements
+// PathologicalManifest : A valid manifest designed to test
+// various edge cases and parsing requirements
 const PathologicalManifest = ". acbd18db4cc2f85cedef654fccc4a4d8+3 37b51d194a7513e45b56f6524f2d51f2+3 73feffa4b7f6bb68e44cf984c85f6e88+3+Z+K at xyzzy acbd18db4cc2f85cedef654fccc4a4d8+3 0:0:zero at 0 0:1:f 1:0:zero at 1 1:4:ooba 4:0:zero at 4 5:1:r 5:4:rbaz 9:0:zero at 9\n" +
 	"./overlapReverse acbd18db4cc2f85cedef654fccc4a4d8+3 acbd18db4cc2f85cedef654fccc4a4d8+3 5:1:o 4:2:oo 2:4:ofoo\n" +
 	"./segmented acbd18db4cc2f85cedef654fccc4a4d8+3 37b51d194a7513e45b56f6524f2d51f2+3 0:1:frob 5:1:frob 1:1:frob 1:2:oof 0:1:oof 5:0:frob 3:1:frob\n" +
@@ -37,4 +37,5 @@ var (
 	MD5CollisionMD5 = "cee9a457e790cf20d4bdaa6d69f01e41"
 )
 
+// BlobSigningKey used by the test servers
 const BlobSigningKey = "zfhgfenhffzltr9dixws36j1yhksjoll2grmku38mi7yxd66h5j4q9w4jzanezacp8s6q0ro3hxakfye02152hncy6zml2ed0uc"
diff --git a/sdk/go/arvadostest/run_servers.go b/sdk/go/arvadostest/run_servers.go
index c61b68b..7edc482 100644
--- a/sdk/go/arvadostest/run_servers.go
+++ b/sdk/go/arvadostest/run_servers.go
@@ -3,21 +3,26 @@ package arvadostest
 import (
 	"bufio"
 	"bytes"
+	"fmt"
+	"io/ioutil"
 	"log"
 	"os"
 	"os/exec"
+	"path"
 	"strconv"
 	"strings"
 )
 
 var authSettings = make(map[string]string)
 
+// ResetEnv resets test env
 func ResetEnv() {
 	for k, v := range authSettings {
 		os.Setenv(k, v)
 	}
 }
 
+// ParseAuthSettings parses auth settings from given input
 func ParseAuthSettings(authScript []byte) {
 	scanner := bufio.NewScanner(bytes.NewReader(authScript))
 	for scanner.Scan() {
@@ -36,7 +41,7 @@ func ParseAuthSettings(authScript []byte) {
 	log.Printf("authSettings: %v", authSettings)
 }
 
-var pythonTestDir string = ""
+var pythonTestDir string
 
 func chdirToPythonTests() {
 	if pythonTestDir != "" {
@@ -59,6 +64,7 @@ func chdirToPythonTests() {
 	}
 }
 
+// StartAPI starts test API server
 func StartAPI() {
 	cwd, _ := os.Getwd()
 	defer os.Chdir(cwd)
@@ -76,6 +82,7 @@ func StartAPI() {
 	ResetEnv()
 }
 
+// StopAPI stops test API server
 func StopAPI() {
 	cwd, _ := os.Getwd()
 	defer os.Chdir(cwd)
@@ -132,3 +139,24 @@ func bgRun(cmd *exec.Cmd) {
 		log.Fatalf("%+v: %s", cmd.Args, err)
 	}
 }
+
+// CreateBadPath creates a tmp dir, appends given string and returns that path
+// This will guarantee that the path being returned does not exist
+func CreateBadPath() (badpath string, err error) {
+	tempdir, err := ioutil.TempDir("", "bad")
+	if err != nil {
+		return "", fmt.Errorf("Could not create temporary directory for bad path: %v", err)
+	}
+	badpath = path.Join(tempdir, "bad")
+	return badpath, nil
+}
+
+// DestroyBadPath deletes the tmp dir created by the previous CreateBadPath call
+func DestroyBadPath(badpath string) error {
+	tempdir := path.Join(badpath, "..")
+	err := os.Remove(tempdir)
+	if err != nil {
+		return fmt.Errorf("Could not remove bad path temporary directory %v: %v", tempdir, err)
+	}
+	return nil
+}
diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
index c78a2f6..740df55 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
@@ -3,6 +3,7 @@ package main
 // Dispatcher service for Crunch that submits containers to the slurm queue.
 
 import (
+	"encoding/json"
 	"flag"
 	"fmt"
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
@@ -17,6 +18,13 @@ import (
 	"time"
 )
 
+// Config used by crunch-dispatch-slurm
+type Config struct {
+	SbatchArguments  []string
+	PollPeriod       *time.Duration
+	CrunchRunCommand *string
+}
+
 func main() {
 	err := doMain()
 	if err != nil {
@@ -25,19 +33,26 @@ func main() {
 }
 
 var (
-	crunchRunCommand *string
-	squeueUpdater    Squeue
+	config        Config
+	squeueUpdater Squeue
 )
 
+const defaultConfigPath = "/etc/arvados/crunch-dispatch-slurm/config.json"
+
 func doMain() error {
 	flags := flag.NewFlagSet("crunch-dispatch-slurm", flag.ExitOnError)
 
-	pollInterval := flags.Int(
+	configPath := flags.String(
+		"config",
+		defaultConfigPath,
+		"`path` to json configuration file")
+
+	config.PollPeriod = flags.Duration(
 		"poll-interval",
-		10,
-		"Interval in seconds to poll for queued containers")
+		10*time.Second,
+		"Time duration to poll for queued containers")
 
-	crunchRunCommand = flags.String(
+	config.CrunchRunCommand = flags.String(
 		"crunch-run-command",
 		"/usr/bin/crunch-run",
 		"Crunch command to run container")
@@ -45,6 +60,12 @@ func doMain() error {
 	// Parse args; omit the first arg which is the command name
 	flags.Parse(os.Args[1:])
 
+	err := readConfig(&config, *configPath)
+	if err != nil {
+		log.Printf("Error reading configuration: %v", err)
+		return err
+	}
+
 	arv, err := arvadosclient.MakeArvadosClient()
 	if err != nil {
 		log.Printf("Error making Arvados client: %v", err)
@@ -52,13 +73,13 @@ func doMain() error {
 	}
 	arv.Retries = 25
 
-	squeueUpdater.StartMonitor(time.Duration(*pollInterval) * time.Second)
+	squeueUpdater.StartMonitor(*config.PollPeriod)
 	defer squeueUpdater.Done()
 
 	dispatcher := dispatch.Dispatcher{
 		Arv:            arv,
 		RunContainer:   run,
-		PollInterval:   time.Duration(*pollInterval) * time.Second,
+		PollInterval:   *config.PollPeriod,
 		DoneProcessing: make(chan struct{})}
 
 	err = dispatcher.RunDispatcher()
@@ -72,10 +93,15 @@ func doMain() error {
 // sbatchCmd
 func sbatchFunc(container arvados.Container) *exec.Cmd {
 	memPerCPU := math.Ceil(float64(container.RuntimeConstraints.RAM) / (float64(container.RuntimeConstraints.VCPUs) * 1048576))
-	return exec.Command("sbatch", "--share",
-		fmt.Sprintf("--job-name=%s", container.UUID),
-		fmt.Sprintf("--mem-per-cpu=%d", int(memPerCPU)),
-		fmt.Sprintf("--cpus-per-task=%d", container.RuntimeConstraints.VCPUs))
+
+	var sbatchArgs []string
+	sbatchArgs = append(sbatchArgs, "--share")
+	sbatchArgs = append(sbatchArgs, config.SbatchArguments...)
+	sbatchArgs = append(sbatchArgs, fmt.Sprintf("--job-name=%s", container.UUID))
+	sbatchArgs = append(sbatchArgs, fmt.Sprintf("--mem-per-cpu=%d", int(memPerCPU)))
+	sbatchArgs = append(sbatchArgs, fmt.Sprintf("--cpus-per-task=%d", container.RuntimeConstraints.VCPUs))
+
+	return exec.Command("sbatch", sbatchArgs...)
 }
 
 // scancelCmd
@@ -189,7 +215,7 @@ func monitorSubmitOrCancel(dispatcher *dispatch.Dispatcher, container arvados.Co
 
 			log.Printf("About to submit queued container %v", container.UUID)
 
-			if err := submit(dispatcher, container, *crunchRunCommand); err != nil {
+			if err := submit(dispatcher, container, *config.CrunchRunCommand); err != nil {
 				log.Printf("Error submitting container %s to slurm: %v",
 					container.UUID, err)
 				// maybe sbatch is broken, put it back to queued
@@ -267,3 +293,18 @@ func run(dispatcher *dispatch.Dispatcher,
 	}
 	monitorDone = true
 }
+
+func readConfig(dst interface{}, path string) error {
+	if buf, err := ioutil.ReadFile(path); err != nil && os.IsNotExist(err) {
+		if path == defaultConfigPath {
+			log.Printf("Config not specified. Continue with default configuration.")
+		} else {
+			return fmt.Errorf("Config file not found %q: %v", path, err)
+		}
+	} else if err != nil {
+		return fmt.Errorf("Error reading config %q: %v", path, err)
+	} else if err = json.Unmarshal(buf, dst); err != nil {
+		return fmt.Errorf("Error decoding config %q: %v", path, err)
+	}
+	return nil
+}
diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
index af27832..a559298 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
@@ -8,6 +8,7 @@ import (
 	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
 	"git.curoverse.com/arvados.git/sdk/go/dispatch"
 	"io"
+	"io/ioutil"
 	"log"
 	"net/http"
 	"net/http/httptest"
@@ -143,7 +144,7 @@ func (s *TestSuite) integrationTest(c *C,
 	c.Check(len(containers.Items), Equals, 1)
 
 	echo := "echo"
-	crunchRunCommand = &echo
+	config.CrunchRunCommand = &echo
 
 	doneProcessing := make(chan struct{})
 	dispatcher := dispatch.Dispatcher{
@@ -179,7 +180,7 @@ func (s *TestSuite) integrationTest(c *C,
 	return container
 }
 
-func (s *MockArvadosServerSuite) Test_APIErrorGettingContainers(c *C) {
+func (s *MockArvadosServerSuite) TestAPIErrorGettingContainers(c *C) {
 	apiStubResponses := make(map[string]arvadostest.StubResponse)
 	apiStubResponses["/arvados/v1/api_client_authorizations/current"] = arvadostest.StubResponse{200, `{"uuid":"` + arvadostest.Dispatch1AuthUUID + `"}`}
 	apiStubResponses["/arvados/v1/containers"] = arvadostest.StubResponse{500, string(`{}`)}
@@ -205,7 +206,7 @@ func testWithServerStub(c *C, apiStubResponses map[string]arvadostest.StubRespon
 	log.SetOutput(io.MultiWriter(buf, os.Stderr))
 	defer log.SetOutput(os.Stderr)
 
-	crunchRunCommand = &crunchCmd
+	config.CrunchRunCommand = &crunchCmd
 
 	doneProcessing := make(chan struct{})
 	dispatcher := dispatch.Dispatcher{
@@ -236,3 +237,82 @@ func testWithServerStub(c *C, apiStubResponses map[string]arvadostest.StubRespon
 
 	c.Check(buf.String(), Matches, `(?ms).*`+expected+`.*`)
 }
+
+func (s *MockArvadosServerSuite) TestNoSuchConfigFile(c *C) {
+	var config Config
+	err := readConfig(&config, "/nosuchdir89j7879/8hjwr7ojgyy7")
+	c.Assert(err, NotNil)
+}
+
+func (s *MockArvadosServerSuite) TestBadSbatchArgsConfig(c *C) {
+	var config Config
+
+	tmpfile, err := ioutil.TempFile(os.TempDir(), "config")
+	c.Check(err, IsNil)
+	defer os.Remove(tmpfile.Name())
+
+	_, err = tmpfile.Write([]byte(`{"SbatchArguments": "oops this is not a string array"}`))
+	c.Check(err, IsNil)
+
+	err = readConfig(&config, tmpfile.Name())
+	c.Assert(err, NotNil)
+}
+
+func (s *MockArvadosServerSuite) TestNoSuchArgInConfigIgnored(c *C) {
+	var config Config
+
+	tmpfile, err := ioutil.TempFile(os.TempDir(), "config")
+	c.Check(err, IsNil)
+	defer os.Remove(tmpfile.Name())
+
+	_, err = tmpfile.Write([]byte(`{"NoSuchArg": "Nobody loves me, not one tiny hunk."}`))
+	c.Check(err, IsNil)
+
+	err = readConfig(&config, tmpfile.Name())
+	c.Assert(err, IsNil)
+	c.Check(0, Equals, len(config.SbatchArguments))
+}
+
+func (s *MockArvadosServerSuite) TestReadConfig(c *C) {
+	var config Config
+
+	tmpfile, err := ioutil.TempFile(os.TempDir(), "config")
+	c.Check(err, IsNil)
+	defer os.Remove(tmpfile.Name())
+
+	args := []string{"--arg1=v1", "--arg2", "--arg3=v3"}
+	argsS := `{"SbatchArguments": ["--arg1=v1",  "--arg2", "--arg3=v3"]}`
+	_, err = tmpfile.Write([]byte(argsS))
+	c.Check(err, IsNil)
+
+	err = readConfig(&config, tmpfile.Name())
+	c.Assert(err, IsNil)
+	c.Check(3, Equals, len(config.SbatchArguments))
+	c.Check(args, DeepEquals, config.SbatchArguments)
+}
+
+func (s *MockArvadosServerSuite) TestSbatchFuncWithNoConfigArgs(c *C) {
+	testSbatchFuncWithArgs(c, nil)
+}
+
+func (s *MockArvadosServerSuite) TestSbatchFuncWithEmptyConfigArgs(c *C) {
+	testSbatchFuncWithArgs(c, []string{})
+}
+
+func (s *MockArvadosServerSuite) TestSbatchFuncWithConfigArgs(c *C) {
+	testSbatchFuncWithArgs(c, []string{"--arg1=v1", "--arg2"})
+}
+
+func testSbatchFuncWithArgs(c *C, args []string) {
+	config.SbatchArguments = append(config.SbatchArguments, args...)
+
+	container := arvados.Container{UUID: "123", RuntimeConstraints: arvados.RuntimeConstraints{RAM: 1000000, VCPUs: 2}}
+	sbatchCmd := sbatchFunc(container)
+
+	var expected []string
+	expected = append(expected, "sbatch", "--share")
+	expected = append(expected, config.SbatchArguments...)
+	expected = append(expected, "--job-name=123", "--mem-per-cpu=1", "--cpus-per-task=2")
+
+	c.Check(sbatchCmd.Args, DeepEquals, expected)
+}
diff --git a/services/datamanager/datamanager_test.go b/services/datamanager/datamanager_test.go
index a99ec6b..0ff6b77 100644
--- a/services/datamanager/datamanager_test.go
+++ b/services/datamanager/datamanager_test.go
@@ -538,32 +538,31 @@ func TestPutAndGetBlocks_NoErrorDuringSingleRun(t *testing.T) {
 	testOldBlocksNotDeletedOnDataManagerError(t, "", "", false, false)
 }
 
-func createBadPath(t *testing.T) (badpath string) {
-	tempdir, err := ioutil.TempDir("", "bad")
-	if err != nil {
-		t.Fatalf("Could not create temporary directory for bad path: %v", err)
-	}
-	badpath = path.Join(tempdir, "bad")
-	return
-}
-
-func destroyBadPath(t *testing.T, badpath string) {
-	tempdir := path.Join(badpath, "..")
-	err := os.Remove(tempdir)
+func TestPutAndGetBlocks_ErrorDuringGetCollectionsBadWriteTo(t *testing.T) {
+	badpath, err := arvadostest.CreateBadPath()
 	if err != nil {
-		t.Fatalf("Could not remove bad path temporary directory %v: %v", tempdir, err)
+		t.Fatalf(err.Error())
 	}
-}
-
-func TestPutAndGetBlocks_ErrorDuringGetCollectionsBadWriteTo(t *testing.T) {
-	badpath := createBadPath(t)
-	defer destroyBadPath(t, badpath)
+	defer func() {
+		err = arvadostest.DestroyBadPath(badpath)
+		if err != nil {
+			t.Fatalf(err.Error())
+		}
+	}()
 	testOldBlocksNotDeletedOnDataManagerError(t, path.Join(badpath, "writetofile"), "", true, true)
 }
 
 func TestPutAndGetBlocks_ErrorDuringGetCollectionsBadHeapProfileFilename(t *testing.T) {
-	badpath := createBadPath(t)
-	defer destroyBadPath(t, badpath)
+	badpath, err := arvadostest.CreateBadPath()
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+	defer func() {
+		err = arvadostest.DestroyBadPath(badpath)
+		if err != nil {
+			t.Fatalf(err.Error())
+		}
+	}()
 	testOldBlocksNotDeletedOnDataManagerError(t, "", path.Join(badpath, "heapprofilefile"), true, true)
 }
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list