12983: Command line option takes duration. Add tests.

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 490d2cf480eee13c1c1d51fda7c3f2353a9ec2df..fc206a91c3eea15bb61f4caafe691e76c08d9fcf 100644 (file)
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -84,6 +84,10 @@ type ThinDockerClient interface {
        ImageRemove(ctx context.Context, image string, options dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDeleteResponseItem, error)
 }
 
+type PsProcess interface {
+       CmdlineSlice() ([]string, error)
+}
+
 // ContainerRunner is the main stateful struct used for a single execution of a
 // container.
 type ContainerRunner struct {
@@ -119,6 +123,8 @@ type ContainerRunner struct {
        finalState    string
        parentTemp    string
 
+       ListProcesses func() ([]PsProcess, error)
+
        statLogger       io.WriteCloser
        statReporter     *crunchstat.Reporter
        hoststatLogger   io.WriteCloser
@@ -145,7 +151,7 @@ type ContainerRunner struct {
        enableNetwork   string // one of "default" or "always"
        networkMode     string // passed through to HostConfig.NetworkMode
        arvMountLog     *ThrottledLogger
-       checkContainerd bool
+       checkContainerd time.Duration
 }
 
 // setupSignals sets up signal handling to gracefully terminate the underlying
@@ -1007,6 +1013,10 @@ func (runner *ContainerRunner) CreateContainer() error {
        runner.ContainerConfig.Volumes = runner.Volumes
 
        maxRAM := int64(runner.Container.RuntimeConstraints.RAM)
+       if maxRAM < 4*1024*1024 {
+               // Docker daemon won't let you set a limit less than 4 MiB
+               maxRAM = 4 * 1024 * 1024
+       }
        runner.HostConfig = dockercontainer.HostConfig{
                Binds: runner.Binds,
                LogConfig: dockercontainer.LogConfig{
@@ -1079,10 +1089,10 @@ func (runner *ContainerRunner) StartContainer() error {
 
 // checkContainerd checks if "containerd" is present in the process list.
 func (runner *ContainerRunner) CheckContainerd() error {
-       if !runner.checkContainerd {
+       if runner.checkContainerd == 0 {
                return nil
        }
-       p, _ := process.Processes()
+       p, _ := runner.ListProcesses()
        for _, i := range p {
                e, _ := i.CmdlineSlice()
                if len(e) > 0 {
@@ -1114,21 +1124,23 @@ func (runner *ContainerRunner) WaitFinish() error {
        defer func() {
                close(containerdGone)
        }()
-       go func() {
-               ticker := time.NewTicker(time.Duration(60 * time.Second))
-               defer ticker.Stop()
-               for {
-                       select {
-                       case <-ticker.C:
-                               if ck := runner.CheckContainerd(); ck != nil {
-                                       containerdGone <- ck
-                                       return
+       if runner.checkContainerd > 0 {
+               go func() {
+                       ticker := time.NewTicker(time.Duration(runner.checkContainerd))
+                       defer ticker.Stop()
+                       for {
+                               select {
+                               case <-ticker.C:
+                                       if ck := runner.CheckContainerd(); ck != nil {
+                                               containerdGone <- ck
+                                               return
+                                       }
+                               case <-containerdGone:
+                                       break
                                }
-                       case <-containerdGone:
-                               break
                        }
-               }
-       }()
+               }()
+       }
 
        for {
                select {
@@ -1584,6 +1596,17 @@ func NewContainerRunner(client *arvados.Client, api IArvadosClient, kc IKeepClie
        cr.NewLogWriter = cr.NewArvLogWriter
        cr.RunArvMount = cr.ArvMountCmd
        cr.MkTempDir = ioutil.TempDir
+       cr.ListProcesses = func() ([]PsProcess, error) {
+               pr, err := process.Processes()
+               if err != nil {
+                       return nil, err
+               }
+               ps := make([]PsProcess, len(pr))
+               for i, j := range pr {
+                       ps[i] = j
+               }
+               return ps, nil
+       }
        cr.MkArvClient = func(token string) (IArvadosClient, error) {
                cl, err := arvadosclient.MakeArvadosClient()
                if err != nil {
@@ -1626,7 +1649,7 @@ func main() {
        `)
        memprofile := flag.String("memprofile", "", "write memory profile to `file` after running container")
        getVersion := flag.Bool("version", false, "Print version information and exit.")
-       checkContainerd := flag.Bool("check-containerd", false, "Periodically check if (docker-)containerd is running, cancel if missing.")
+       checkContainerd := flag.Duration("check-containerd", 60*time.Second, "Periodic check if (docker-)containerd is running, period in seconds.")
        flag.Parse()
 
        // Print version information if requested

diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 8ad487d7719b6a0dc4936ddda6312f8e0ec2948f..8d8e0400003a94dae160ee65a69ccd92f723c823 100644 (file)
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -2067,3 +2067,49 @@ func (s *TestSuite) TestSecretTextMountPoint(c *C) {
        c.Check(api.CalledWith("collection.manifest_text", ". 34819d7beeabb9260a5c854bc85b3e44+10 0:10:secret.conf\n"), IsNil)
        c.Check(api.CalledWith("collection.manifest_text", ""), NotNil)
 }
+
+type FakeProcess struct {
+       cmdLine []string
+}
+
+func (fp FakeProcess) CmdlineSlice() ([]string, error) {
+       return fp.cmdLine, nil
+}
+
+func (s *TestSuite) helpCheckContainerd(c *C, lp func() ([]PsProcess, error)) error {
+       kc := &KeepTestClient{}
+       defer kc.Close()
+       cr, err := NewContainerRunner(s.client, &ArvTestClient{callraw: true}, kc, s.docker, "zzzzz-zzzzz-zzzzzzzzzzzzzzz")
+       cr.checkContainerd = time.Duration(100 * time.Millisecond)
+       c.Assert(err, IsNil)
+       cr.ListProcesses = lp
+
+       s.docker.fn = func(t *TestDockerClient) {
+               time.Sleep(1 * time.Second)
+               t.logWriter.Close()
+       }
+
+       err = cr.CreateContainer()
+       c.Check(err, IsNil)
+
+       err = cr.StartContainer()
+       c.Check(err, IsNil)
+
+       err = cr.WaitFinish()
+       return err
+
+}
+
+func (s *TestSuite) TestCheckContainerdPresent(c *C) {
+       err := s.helpCheckContainerd(c, func() ([]PsProcess, error) {
+               return []PsProcess{FakeProcess{[]string{"docker-containerd"}}}, nil
+       })
+       c.Check(err, IsNil)
+}
+
+func (s *TestSuite) TestCheckContainerdMissing(c *C) {
+       err := s.helpCheckContainerd(c, func() ([]PsProcess, error) {
+               return []PsProcess{FakeProcess{[]string{"abc"}}}, nil
+       })
+       c.Check(err, ErrorMatches, `'containerd' not found in process list.`)
+}