[ARVADOS] created: 1.1.3-180-gcc14a0d

Git user git at public.curoverse.com
Mon Mar 12 16:52:20 EDT 2018


        at  cc14a0ddb50d0d9b3aa03295e4212bd13b073be1 (commit)


commit cc14a0ddb50d0d9b3aa03295e4212bd13b073be1
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Mon Mar 12 16:25:08 2018 -0400

    13134: Use the container token to fetch secret_mounts
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 3591907..4d231a4 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -112,6 +112,7 @@ type ContainerRunner struct {
 	SigChan       chan os.Signal
 	ArvMountExit  chan error
 	SecretMounts  map[string]arvados.Mount
+	MkArvClient   func(token string) (IArvadosClient, error)
 	finalState    string
 	parentTemp    string
 
@@ -1738,7 +1739,17 @@ func (runner *ContainerRunner) fetchContainerRecord() error {
 		SecretMounts map[string]arvados.Mount `json:"secret_mounts"`
 	}
 
-	err = runner.ArvClient.Call("GET", "containers", runner.Container.UUID, "secret_mounts", nil, &sm)
+	containerToken, err := runner.ContainerToken()
+	if err != nil {
+		return fmt.Errorf("error getting container token: %v", err)
+	}
+
+	containerClient, err := runner.MkArvClient(containerToken)
+	if err != nil {
+		return fmt.Errorf("error creating container API client: %v", err)
+	}
+
+	err = containerClient.Call("GET", "containers", runner.Container.UUID, "secret_mounts", nil, &sm)
 	if err != nil {
 		if apierr, ok := err.(arvadosclient.APIServerError); !ok || apierr.HttpStatusCode != 404 {
 			return fmt.Errorf("error fetching secret_mounts: %v", err)
@@ -1761,6 +1772,14 @@ func NewContainerRunner(api IArvadosClient,
 	cr.NewLogWriter = cr.NewArvLogWriter
 	cr.RunArvMount = cr.ArvMountCmd
 	cr.MkTempDir = ioutil.TempDir
+	cr.MkArvClient = func(token string) (IArvadosClient, error) {
+		cl, err := arvadosclient.MakeArvadosClient()
+		if err != nil {
+			return nil, err
+		}
+		cl.ApiToken = token
+		return cl, nil
+	}
 	cr.LogCollection = &CollectionWriter{0, kc, nil, nil, sync.Mutex{}}
 	cr.Container.UUID = containerUUID
 	cr.CrunchLog = NewThrottledLogger(cr.NewLogWriter("crunch-run"))
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 98420b0..dd977c3 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -693,7 +693,7 @@ func (s *TestSuite) fullRunHelper(c *C, record string, extraMounts []string, exi
 	s.docker.fn = fn
 	s.docker.ImageRemove(nil, hwImageId, dockertypes.ImageRemoveOptions{})
 
-	api = &ArvTestClient{Container: rec, secretMounts: secretMounts}
+	api = &ArvTestClient{Container: rec}
 	s.docker.api = api
 	cr = NewContainerRunner(api, &KeepTestClient{}, s.docker, "zzzzz-zzzzz-zzzzzzzzzzzzzzz")
 	cr.statInterval = 100 * time.Millisecond
@@ -717,6 +717,9 @@ func (s *TestSuite) fullRunHelper(c *C, record string, extraMounts []string, exi
 		}
 		return d, err
 	}
+	cr.MkArvClient = func(token string) (IArvadosClient, error) {
+		return &ArvTestClient{secretMounts: secretMounts}, nil
+	}
 
 	if extraMounts != nil && len(extraMounts) > 0 {
 		err := cr.SetupArvMountPoint("keep")
@@ -970,6 +973,9 @@ func (s *TestSuite) testStopContainer(c *C, setup func(cr *ContainerRunner)) {
 	api := &ArvTestClient{Container: rec}
 	cr := NewContainerRunner(api, &KeepTestClient{}, s.docker, "zzzzz-zzzzz-zzzzzzzzzzzzzzz")
 	cr.RunArvMount = func([]string, string) (*exec.Cmd, error) { return nil, nil }
+	cr.MkArvClient = func(token string) (IArvadosClient, error) {
+		return &ArvTestClient{}, nil
+	}
 	setup(cr)
 
 	done := make(chan error)
@@ -1446,6 +1452,9 @@ func (s *TestSuite) stdoutErrorRunHelper(c *C, record string, fn func(t *TestDoc
 	cr = NewContainerRunner(api, &KeepTestClient{}, s.docker, "zzzzz-zzzzz-zzzzzzzzzzzzzzz")
 	am := &ArvMountCmdLine{}
 	cr.RunArvMount = am.ArvMountTest
+	cr.MkArvClient = func(token string) (IArvadosClient, error) {
+		return &ArvTestClient{}, nil
+	}
 
 	err = cr.Run()
 	return

commit 8601710d87ab12014994a5baf6ddafa6647b77b2
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Mon Mar 12 13:46:46 2018 -0400

    13134: Get secret_mounts from separate API endpoint.  Update tests.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index e0e8c77..daafc49 100644
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -16,7 +16,6 @@ type Container struct {
 	Environment          map[string]string    `json:"environment"`
 	LockedByUUID         string               `json:"locked_by_uuid"`
 	Mounts               map[string]Mount     `json:"mounts"`
-	SecretMounts         map[string]Mount     `json:"secret_mounts"`
 	Output               string               `json:"output"`
 	OutputPath           string               `json:"output_path"`
 	Priority             int                  `json:"priority"`
diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 31af0bd..3591907 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -111,6 +111,7 @@ type ContainerRunner struct {
 	OutputPDH     *string
 	SigChan       chan os.Signal
 	ArvMountExit  chan error
+	SecretMounts  map[string]arvados.Mount
 	finalState    string
 	parentTemp    string
 
@@ -396,11 +397,11 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
 	for bind := range runner.Container.Mounts {
 		binds = append(binds, bind)
 	}
-	for bind := range runner.Container.SecretMounts {
-		if runner.Container.SecretMounts[bind].Kind != "json" &&
-			runner.Container.SecretMounts[bind].Kind != "text" {
+	for bind := range runner.SecretMounts {
+		if runner.SecretMounts[bind].Kind != "json" &&
+			runner.SecretMounts[bind].Kind != "text" {
 			return fmt.Errorf("Secret mount %q type is %q but only 'json' and 'text' are permitted.",
-				bind, runner.Container.SecretMounts[bind].Kind)
+				bind, runner.SecretMounts[bind].Kind)
 		}
 		binds = append(binds, bind)
 	}
@@ -409,7 +410,7 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
 	for _, bind := range binds {
 		mnt, ok := runner.Container.Mounts[bind]
 		if !ok {
-			mnt = runner.Container.SecretMounts[bind]
+			mnt = runner.SecretMounts[bind]
 		}
 		if bind == "stdout" || bind == "stderr" {
 			// Is it a "file" mount kind?
@@ -1280,7 +1281,7 @@ func (runner *ContainerRunner) CaptureOutput() error {
 	sort.Strings(binds)
 
 	// Delete secret mounts so they don't get saved to the output collection.
-	for bind := range runner.Container.SecretMounts {
+	for bind := range runner.SecretMounts {
 		if strings.HasPrefix(bind, runner.Container.OutputPath+"/") {
 			err = os.Remove(runner.HostOutputDir + bind[len(runner.Container.OutputPath):])
 			if err != nil {
@@ -1732,6 +1733,21 @@ func (runner *ContainerRunner) fetchContainerRecord() error {
 	if err != nil {
 		return fmt.Errorf("error decoding container record: %v", err)
 	}
+
+	var sm struct {
+		SecretMounts map[string]arvados.Mount `json:"secret_mounts"`
+	}
+
+	err = runner.ArvClient.Call("GET", "containers", runner.Container.UUID, "secret_mounts", nil, &sm)
+	if err != nil {
+		if apierr, ok := err.(arvadosclient.APIServerError); !ok || apierr.HttpStatusCode != 404 {
+			return fmt.Errorf("error fetching secret_mounts: %v", err)
+		}
+		// ok && apierr.HttpStatusCode == 404, which means
+		// secret_mounts isn't supported by this API server.
+	}
+	runner.SecretMounts = sm.SecretMounts
+
 	return nil
 }
 
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 487e95e..98420b0 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"log"
 	"net"
 	"os"
 	"os/exec"
@@ -57,7 +58,8 @@ type ArvTestClient struct {
 	Calls   int
 	Content []arvadosclient.Dict
 	arvados.Container
-	Logs map[string]*bytes.Buffer
+	secretMounts []byte
+	Logs         map[string]*bytes.Buffer
 	sync.Mutex
 	WasSetRunning bool
 	callraw       bool
@@ -240,6 +242,12 @@ func (client *ArvTestClient) Call(method, resourceType, uuid, action string, par
 			"uuid": "`+fakeAuthUUID+`",
 			"api_token": "`+fakeAuthToken+`"
 			}`), output)
+	case method == "GET" && resourceType == "containers" && action == "secret_mounts":
+		if client.secretMounts != nil {
+			return json.Unmarshal(client.secretMounts, output)
+		} else {
+			return json.Unmarshal([]byte(`{"secret_mounts":{}}`), output)
+		}
 	default:
 		return fmt.Errorf("Not found")
 	}
@@ -672,11 +680,20 @@ func (s *TestSuite) fullRunHelper(c *C, record string, extraMounts []string, exi
 	err := json.Unmarshal([]byte(record), &rec)
 	c.Check(err, IsNil)
 
+	var sm struct {
+		SecretMounts map[string]arvados.Mount `json:"secret_mounts"`
+	}
+	err = json.Unmarshal([]byte(record), &sm)
+	c.Check(err, IsNil)
+	secretMounts, err := json.Marshal(sm)
+	log.Printf("%q %q", sm, secretMounts)
+	c.Check(err, IsNil)
+
 	s.docker.exitCode = exitCode
 	s.docker.fn = fn
 	s.docker.ImageRemove(nil, hwImageId, dockertypes.ImageRemoveOptions{})
 
-	api = &ArvTestClient{Container: rec}
+	api = &ArvTestClient{Container: rec, secretMounts: secretMounts}
 	s.docker.api = api
 	cr = NewContainerRunner(api, &KeepTestClient{}, s.docker, "zzzzz-zzzzz-zzzzzzzzzzzzzzz")
 	cr.statInterval = 100 * time.Millisecond
@@ -2083,6 +2100,9 @@ func (s *TestSuite) TestSecretTextMountPoint(c *C) {
 	}`
 
 	api, _, _ := s.fullRunHelper(c, helperRecord, nil, 0, func(t *TestDockerClient) {
+		content, err := ioutil.ReadFile(t.realTemp + "/tmp2/secret.conf")
+		c.Check(err, IsNil)
+		c.Check(content, DeepEquals, []byte("mypassword"))
 		t.logWriter.Close()
 	})
 
@@ -2108,6 +2128,9 @@ func (s *TestSuite) TestSecretTextMountPoint(c *C) {
 	}`
 
 	api, _, _ = s.fullRunHelper(c, helperRecord, nil, 0, func(t *TestDockerClient) {
+		content, err := ioutil.ReadFile(t.realTemp + "/tmp2/secret.conf")
+		c.Check(err, IsNil)
+		c.Check(content, DeepEquals, []byte("mypassword"))
 		t.logWriter.Close()
 	})
 

commit 9e9509f777e04ba6c1e168cd6add478da13bd094
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Fri Mar 9 14:11:28 2018 -0500

    13134: Support for secret mounts in crunch-run.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index daafc49..e0e8c77 100644
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -16,6 +16,7 @@ type Container struct {
 	Environment          map[string]string    `json:"environment"`
 	LockedByUUID         string               `json:"locked_by_uuid"`
 	Mounts               map[string]Mount     `json:"mounts"`
+	SecretMounts         map[string]Mount     `json:"secret_mounts"`
 	Output               string               `json:"output"`
 	OutputPath           string               `json:"output_path"`
 	Priority             int                  `json:"priority"`
diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 653e0b4..31af0bd 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -396,10 +396,21 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
 	for bind := range runner.Container.Mounts {
 		binds = append(binds, bind)
 	}
+	for bind := range runner.Container.SecretMounts {
+		if runner.Container.SecretMounts[bind].Kind != "json" &&
+			runner.Container.SecretMounts[bind].Kind != "text" {
+			return fmt.Errorf("Secret mount %q type is %q but only 'json' and 'text' are permitted.",
+				bind, runner.Container.SecretMounts[bind].Kind)
+		}
+		binds = append(binds, bind)
+	}
 	sort.Strings(binds)
 
 	for _, bind := range binds {
-		mnt := runner.Container.Mounts[bind]
+		mnt, ok := runner.Container.Mounts[bind]
+		if !ok {
+			mnt = runner.Container.SecretMounts[bind]
+		}
 		if bind == "stdout" || bind == "stderr" {
 			// Is it a "file" mount kind?
 			if mnt.Kind != "file" {
@@ -428,8 +439,8 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
 		}
 
 		if strings.HasPrefix(bind, runner.Container.OutputPath+"/") && bind != runner.Container.OutputPath+"/" {
-			if mnt.Kind != "collection" {
-				return fmt.Errorf("Only mount points of kind 'collection' are supported underneath the output_path: %v", bind)
+			if mnt.Kind != "collection" && mnt.Kind != "text" && mnt.Kind != "json" {
+				return fmt.Errorf("Only mount points of kind 'collection', 'text' or 'json' are supported underneath the output_path for %q, was %q", bind, mnt.Kind)
 			}
 		}
 
@@ -503,26 +514,35 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
 				runner.HostOutputDir = tmpdir
 			}
 
-		case mnt.Kind == "json":
-			jsondata, err := json.Marshal(mnt.Content)
-			if err != nil {
-				return fmt.Errorf("encoding json data: %v", err)
+		case mnt.Kind == "json" || mnt.Kind == "text":
+			var filedata []byte
+			if mnt.Kind == "json" {
+				filedata, err = json.Marshal(mnt.Content)
+				if err != nil {
+					return fmt.Errorf("encoding json data: %v", err)
+				}
+			} else {
+				text, ok := mnt.Content.(string)
+				if !ok {
+					return fmt.Errorf("content for mount %q must be a string", bind)
+				}
+				filedata = []byte(text)
 			}
-			// Create a tempdir with a single file
-			// (instead of just a tempfile): this way we
-			// can ensure the file is world-readable
-			// inside the container, without having to
-			// make it world-readable on the docker host.
-			tmpdir, err := runner.MkTempDir(runner.parentTemp, "json")
+
+			tmpdir, err := runner.MkTempDir(runner.parentTemp, mnt.Kind)
 			if err != nil {
 				return fmt.Errorf("creating temp dir: %v", err)
 			}
-			tmpfn := filepath.Join(tmpdir, "mountdata.json")
-			err = ioutil.WriteFile(tmpfn, jsondata, 0644)
+			tmpfn := filepath.Join(tmpdir, "mountdata."+mnt.Kind)
+			err = ioutil.WriteFile(tmpfn, filedata, 0444)
 			if err != nil {
 				return fmt.Errorf("writing temp file: %v", err)
 			}
-			runner.Binds = append(runner.Binds, fmt.Sprintf("%s:%s:ro", tmpfn, bind))
+			if strings.HasPrefix(bind, runner.Container.OutputPath+"/") {
+				copyFiles = append(copyFiles, copyFile{tmpfn, runner.HostOutputDir + bind[len(runner.Container.OutputPath):]})
+			} else {
+				runner.Binds = append(runner.Binds, fmt.Sprintf("%s:%s:ro", tmpfn, bind))
+			}
 
 		case mnt.Kind == "git_tree":
 			tmpdir, err := runner.MkTempDir(runner.parentTemp, "git_tree")
@@ -1259,6 +1279,16 @@ func (runner *ContainerRunner) CaptureOutput() error {
 	}
 	sort.Strings(binds)
 
+	// Delete secret mounts so they don't get saved to the output collection.
+	for bind := range runner.Container.SecretMounts {
+		if strings.HasPrefix(bind, runner.Container.OutputPath+"/") {
+			err = os.Remove(runner.HostOutputDir + bind[len(runner.Container.OutputPath):])
+			if err != nil {
+				return fmt.Errorf("Unable to remove secret mount: %v", err)
+			}
+		}
+	}
+
 	var manifestText string
 
 	collectionMetafile := fmt.Sprintf("%s/.arvados#collection", runner.HostOutputDir)
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 94b7133..487e95e 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -1211,6 +1211,35 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 		checkEmpty()
 	}
 
+	for _, test := range []struct {
+		in  interface{}
+		out string
+	}{
+		{in: "foo", out: `foo`},
+		{in: nil, out: "error"},
+		{in: map[string]int64{"foo": 123456789123456789}, out: "error"},
+	} {
+		i = 0
+		cr.ArvMountPoint = ""
+		cr.Container.Mounts = map[string]arvados.Mount{
+			"/mnt/test.txt": {Kind: "text", Content: test.in},
+		}
+		err := cr.SetupMounts()
+		if test.out == "error" {
+			c.Check(err.Error(), Equals, "content for mount \"/mnt/test.txt\" must be a string")
+		} else {
+			c.Check(err, IsNil)
+			sort.StringSlice(cr.Binds).Sort()
+			c.Check(cr.Binds, DeepEquals, []string{realTemp + "/text2/mountdata.text:/mnt/test.txt:ro"})
+			content, err := ioutil.ReadFile(realTemp + "/text2/mountdata.text")
+			c.Check(err, IsNil)
+			c.Check(content, DeepEquals, []byte(test.out))
+		}
+		os.RemoveAll(cr.ArvMountPoint)
+		cr.CleanupDirs()
+		checkEmpty()
+	}
+
 	// Read-only mount points are allowed underneath output_dir mount point
 	{
 		i = 0
@@ -1277,13 +1306,13 @@ func (s *TestSuite) TestSetupMounts(c *C) {
 		cr.Container.Mounts = make(map[string]arvados.Mount)
 		cr.Container.Mounts = map[string]arvados.Mount{
 			"/tmp":     {Kind: "tmp"},
-			"/tmp/foo": {Kind: "json"},
+			"/tmp/foo": {Kind: "tmp"},
 		}
 		cr.OutputPath = "/tmp"
 
 		err := cr.SetupMounts()
 		c.Check(err, NotNil)
-		c.Check(err, ErrorMatches, `Only mount points of kind 'collection' are supported underneath the output_path.*`)
+		c.Check(err, ErrorMatches, `Only mount points of kind 'collection', 'text' or 'json' are supported underneath the output_path.*`)
 		os.RemoveAll(cr.ArvMountPoint)
 		cr.CleanupDirs()
 		checkEmpty()
@@ -2035,3 +2064,55 @@ func (s *TestSuite) TestBadCommand3(c *C) {
 	c.Check(api.CalledWith("container.state", "Cancelled"), NotNil)
 	c.Check(api.Logs["crunch-run"].String(), Matches, "(?ms).*Possible causes:.*is missing.*")
 }
+
+func (s *TestSuite) TestSecretTextMountPoint(c *C) {
+	// under normal mounts, gets captured in output, oops
+	helperRecord := `{
+		"command": ["true"],
+		"container_image": "d4ab34d3d4f8a72f5c4973051ae69fab+122",
+		"cwd": "/bin",
+		"mounts": {
+                    "/tmp": {"kind": "tmp"},
+                    "/tmp/secret.conf": {"kind": "text", "content": "mypassword"}
+                },
+                "secret_mounts": {
+                },
+		"output_path": "/tmp",
+		"priority": 1,
+		"runtime_constraints": {}
+	}`
+
+	api, _, _ := s.fullRunHelper(c, helperRecord, nil, 0, func(t *TestDockerClient) {
+		t.logWriter.Close()
+	})
+
+	c.Check(api.CalledWith("container.exit_code", 0), NotNil)
+	c.Check(api.CalledWith("container.state", "Complete"), NotNil)
+	c.Check(api.CalledWith("collection.manifest_text", ". 34819d7beeabb9260a5c854bc85b3e44+10 0:10:secret.conf\n"), NotNil)
+	c.Check(api.CalledWith("collection.manifest_text", ""), IsNil)
+
+	// under secret mounts, not captured in output
+	helperRecord = `{
+		"command": ["true"],
+		"container_image": "d4ab34d3d4f8a72f5c4973051ae69fab+122",
+		"cwd": "/bin",
+		"mounts": {
+                    "/tmp": {"kind": "tmp"}
+                },
+                "secret_mounts": {
+                    "/tmp/secret.conf": {"kind": "text", "content": "mypassword"}
+                },
+		"output_path": "/tmp",
+		"priority": 1,
+		"runtime_constraints": {}
+	}`
+
+	api, _, _ = s.fullRunHelper(c, helperRecord, nil, 0, func(t *TestDockerClient) {
+		t.logWriter.Close()
+	})
+
+	c.Check(api.CalledWith("container.exit_code", 0), NotNil)
+	c.Check(api.CalledWith("container.state", "Complete"), NotNil)
+	c.Check(api.CalledWith("collection.manifest_text", ". 34819d7beeabb9260a5c854bc85b3e44+10 0:10:secret.conf\n"), IsNil)
+	c.Check(api.CalledWith("collection.manifest_text", ""), NotNil)
+}

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list