[ARVADOS] updated: 1.1.0-101-ge16989e
Git user
git at public.curoverse.com
Fri Nov 3 16:14:56 EDT 2017
Summary of changes:
services/crunch-run/crunchrun.go | 67 +++++++++++++++++++----------------
services/crunch-run/crunchrun_test.go | 33 ++++++++++++-----
2 files changed, 60 insertions(+), 40 deletions(-)
via e16989ed7897833f14766341838abd4e382a32a1 (commit)
via ce9c77376595f6062e1ac67d1b7393ab002ed39f (commit)
from e9d5acfe69ea1b72ba4e7b8b9ee521f26229e7e6 (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 e16989ed7897833f14766341838abd4e382a32a1
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Fri Nov 3 16:14:42 2017 -0400
12538: Refactor force-unmount behavior.
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 9e05489..7080ca2 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -183,7 +183,6 @@ type ContainerRunner struct {
enableNetwork string // one of "default" or "always"
networkMode string // passed through to HostConfig.NetworkMode
arvMountLog *ThrottledLogger
- arvMountKill func()
}
// setupSignals sets up signal handling to gracefully terminate the underlying
@@ -309,10 +308,6 @@ func (runner *ContainerRunner) ArvMountCmd(arvMountCmd []string, token string) (
return nil, err
}
- runner.arvMountKill = func() {
- c.Process.Kill()
- }
-
statReadme := make(chan bool)
runner.ArvMountExit = make(chan error)
@@ -1258,38 +1253,46 @@ func (runner *ContainerRunner) getCollectionManifestForPath(mnt arvados.Mount, b
return extracted.Text, nil
}
+func (runner *ContainerRunner) tryUnmount(umount *exec.Cmd) error {
+ umnterr := umount.Start()
+ if umnterr != nil {
+ runner.CrunchLog.Printf("Error: %v", umnterr)
+ }
+ go func() {
+ mnterr := umount.Wait()
+ if mnterr != nil {
+ runner.CrunchLog.Printf("Error running %v: %v", umount.Args, mnterr)
+ }
+ }()
+
+ timeout := time.NewTimer(9 * time.Second)
+ select {
+ case <-runner.ArvMountExit:
+ return nil
+ case <-timeout.C:
+ return fmt.Errorf("Timed out")
+ }
+}
+
func (runner *ContainerRunner) CleanupDirs() {
if runner.ArvMount != nil {
- var umount *exec.Cmd
- umount = exec.Command("fusermount", "-u", "-z", runner.ArvMountPoint)
- done := false
- try := 1
- for !done {
- umnterr := umount.Run()
- if umnterr != nil {
- runner.CrunchLog.Printf("Error: %v", umnterr)
- }
- timeout := time.NewTimer(10 * time.Second)
- select {
- case <-runner.ArvMountExit:
- done = true
- case <-timeout.C:
- if try == 1 {
- runner.CrunchLog.Printf("Timeout waiting for arv-mount to end. Will force unmount.")
- umount = exec.Command("arv-mount", "--unmount-timeout=10", "--unmount", runner.ArvMountPoint)
- try = 2
- } else {
- runner.CrunchLog.Printf("Killing arv-mount")
- runner.arvMountKill()
- umount = exec.Command("fusermount", "-u", "-z", runner.ArvMountPoint)
- }
+ if err := runner.tryUnmount(exec.Command("fusermount", "-u", runner.ArvMountPoint)); err != nil {
+ runner.CrunchLog.Printf("arv-mount not ended, will try force unmount: %v", err)
+ err = runner.tryUnmount(exec.Command("arv-mount", "--unmount-timeout=8", "--unmount", runner.ArvMountPoint))
+ if err != nil {
+ runner.CrunchLog.Printf("Error running arv-mount --unmount: %v", err)
}
}
+ if rmerr := os.Remove(runner.ArvMountPoint); rmerr != nil {
+ runner.CrunchLog.Printf("While cleaning up arv-mount directory %s: %v", runner.ArvMountPoint, rmerr)
+ }
}
for _, tmpdir := range runner.CleanupTempDir {
- rmerr := os.RemoveAll(tmpdir)
- if rmerr != nil {
+ if tmpdir == runner.ArvMountPoint {
+ continue
+ }
+ if rmerr := os.RemoveAll(tmpdir); rmerr != nil {
runner.CrunchLog.Printf("While cleaning up temporary directory %s: %v", tmpdir, rmerr)
}
}
commit ce9c77376595f6062e1ac67d1b7393ab002ed39f
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Fri Nov 3 15:04:00 2017 -0400
12538: Fix tests.
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 55edb99..9e05489 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -1299,7 +1299,9 @@ func (runner *ContainerRunner) CleanupDirs() {
func (runner *ContainerRunner) CommitLogs() error {
runner.CrunchLog.Print(runner.finalState)
- runner.arvMountLog.Close()
+ if runner.arvMountLog != nil {
+ runner.arvMountLog.Close()
+ }
runner.CrunchLog.Close()
// Closing CrunchLog above allows them to be committed to Keep at this
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index d3c9990..5992be0 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -131,7 +131,7 @@ func (t *TestDockerClient) ContainerCreate(ctx context.Context, config *dockerco
func (t *TestDockerClient) ContainerStart(ctx context.Context, container string, options dockertypes.ContainerStartOptions) error {
if container == "abcde" {
- go t.fn(t)
+ // t.fn gets executed in ContainerWait
return nil
} else {
return errors.New("Invalid container id")
@@ -147,6 +147,7 @@ func (t *TestDockerClient) ContainerWait(ctx context.Context, container string,
body := make(chan dockercontainer.ContainerWaitOKBody)
err := make(chan error)
go func() {
+ t.fn(t)
body <- dockercontainer.ContainerWaitOKBody{StatusCode: int64(t.finish)}
close(body)
close(err)
@@ -1002,10 +1003,12 @@ func (s *TestSuite) TestSetupMounts(c *C) {
cr.Container.Mounts = make(map[string]arvados.Mount)
cr.Container.Mounts["/tmp"] = arvados.Mount{Kind: "tmp"}
cr.OutputPath = "/tmp"
-
+ cr.statInterval = 5 * time.Second
err := cr.SetupMounts()
c.Check(err, IsNil)
- c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+ c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+ "--read-write", "--crunchstat-interval=5",
+ "--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/tmp"})
cr.CleanupDirs()
checkEmpty()
@@ -1021,7 +1024,9 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, IsNil)
- c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+ c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+ "--read-write", "--crunchstat-interval=5",
+ "--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/out", realTemp + "/3:/tmp"})
cr.CleanupDirs()
checkEmpty()
@@ -1039,7 +1044,9 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, IsNil)
- c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+ c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+ "--read-write", "--crunchstat-interval=5",
+ "--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/tmp", stubCertPath + ":/etc/arvados/ca-certificates.crt:ro"})
cr.CleanupDirs()
checkEmpty()
@@ -1059,7 +1066,9 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, IsNil)
- c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+ c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+ "--read-write", "--crunchstat-interval=5",
+ "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/keep1/tmp0:/keeptmp"})
cr.CleanupDirs()
checkEmpty()
@@ -1079,7 +1088,9 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, IsNil)
- c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+ c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+ "--read-write", "--crunchstat-interval=5",
+ "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
sort.StringSlice(cr.Binds).Sort()
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53:/keepinp:ro",
realTemp + "/keep1/tmp0:/keepout"})
@@ -1102,7 +1113,9 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, IsNil)
- c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+ c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+ "--read-write", "--crunchstat-interval=5",
+ "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
sort.StringSlice(cr.Binds).Sort()
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53:/keepinp:ro",
realTemp + "/keep1/tmp0:/keepout"})
@@ -1149,7 +1162,9 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, IsNil)
- c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+ c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other",
+ "--read-write", "--crunchstat-interval=5",
+ "--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/tmp", realTemp + "/keep1/tmp0:/tmp/foo:ro"})
cr.CleanupDirs()
checkEmpty()
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list