Merge branch '18290-lsf-bsub-args-template'

closes #18290

Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward@curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 49f35d15cf4addb7691a8d23119dd616a29c8b39..3ff5382059033d2987073207d7fd52a5b6969429 100644 (file)
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1021,14 +1021,23 @@ Clusters:
           AssignNodeHostname: "compute%<slot_number>d"
 
       LSF:
-        # Additional arguments to bsub when submitting Arvados
-        # containers as LSF jobs.
+        # Arguments to bsub when submitting Arvados containers as LSF jobs.
+        #
+        # Template variables starting with % will be substituted as follows:
+        #
+        # %U uuid
+        # %C number of VCPUs
+        # %M memory in MB
+        # %T tmp in MB
+        #
+        # Use %% to express a literal %. The %%J in the default will be changed
+        # to %J, which is interpreted by bsub itself.
         #
         # Note that the default arguments cause LSF to write two files
         # in /tmp on the compute node each time an Arvados container
         # runs. Ensure you have something in place to delete old files
-        # from /tmp, or adjust these arguments accordingly.
-        BsubArgumentsList: ["-o", "/tmp/crunch-run.%J.out", "-e", "/tmp/crunch-run.%J.err"]
+        # from /tmp, or adjust the "-o" and "-e" arguments accordingly.
+        BsubArgumentsList: ["-o", "/tmp/crunch-run.%%J.out", "-e", "/tmp/crunch-run.%%J.err", "-J", "%U", "-n", "%C", "-D", "%MMB", "-R", "rusage[mem=%MMB:tmp=%TMB] span[hosts=1]"]
 
         # Use sudo to switch to this user account when submitting LSF
         # jobs.

diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 6244b8dbdaba06d8ec8a9528bae601026872a6d1..2fe207eff8c500cdb4d71c7c45588449a57d681f 100644 (file)
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -1027,14 +1027,23 @@ Clusters:
           AssignNodeHostname: "compute%<slot_number>d"
 
       LSF:
-        # Additional arguments to bsub when submitting Arvados
-        # containers as LSF jobs.
+        # Arguments to bsub when submitting Arvados containers as LSF jobs.
+        #
+        # Template variables starting with % will be substituted as follows:
+        #
+        # %U uuid
+        # %C number of VCPUs
+        # %M memory in MB
+        # %T tmp in MB
+        #
+        # Use %% to express a literal %. The %%J in the default will be changed
+        # to %J, which is interpreted by bsub itself.
         #
         # Note that the default arguments cause LSF to write two files
         # in /tmp on the compute node each time an Arvados container
         # runs. Ensure you have something in place to delete old files
-        # from /tmp, or adjust these arguments accordingly.
-        BsubArgumentsList: ["-o", "/tmp/crunch-run.%J.out", "-e", "/tmp/crunch-run.%J.err"]
+        # from /tmp, or adjust the "-o" and "-e" arguments accordingly.
+        BsubArgumentsList: ["-o", "/tmp/crunch-run.%%J.out", "-e", "/tmp/crunch-run.%%J.err", "-J", "%U", "-n", "%C", "-D", "%MMB", "-R", "rusage[mem=%MMB:tmp=%TMB] span[hosts=1]"]
 
         # Use sudo to switch to this user account when submitting LSF
         # jobs.

diff --git a/lib/lsf/dispatch.go b/lib/lsf/dispatch.go
index d3ba605abac12ae3f24a28ce9bdfe056c8744c7c..6e35b7de929f8843bdd7bdb848698ca96c62a123 100644 (file)
--- a/lib/lsf/dispatch.go
+++ b/lib/lsf/dispatch.go
@@ -271,27 +271,40 @@ func (disp *dispatcher) bkill(ctr arvados.Container) {
 
 func (disp *dispatcher) bsubArgs(container arvados.Container) ([]string, error) {
        args := []string{"bsub"}
-       args = append(args, disp.Cluster.Containers.LSF.BsubArgumentsList...)
-       args = append(args, "-J", container.UUID)
-       args = append(args, disp.bsubConstraintArgs(container)...)
-       if u := disp.Cluster.Containers.LSF.BsubSudoUser; u != "" {
-               args = append([]string{"sudo", "-E", "-u", u}, args...)
-       }
-       return args, nil
-}
 
-func (disp *dispatcher) bsubConstraintArgs(container arvados.Container) []string {
-       // TODO: propagate container.SchedulingParameters.Partitions
        tmp := int64(math.Ceil(float64(dispatchcloud.EstimateScratchSpace(&container)) / 1048576))
        vcpus := container.RuntimeConstraints.VCPUs
        mem := int64(math.Ceil(float64(container.RuntimeConstraints.RAM+
                container.RuntimeConstraints.KeepCacheRAM+
                int64(disp.Cluster.Containers.ReserveExtraRAM)) / 1048576))
-       return []string{
-               "-n", fmt.Sprintf("%d", vcpus),
-               "-D", fmt.Sprintf("%dMB", mem), // ulimit -d (note this doesn't limit the total container memory usage)
-               "-R", fmt.Sprintf("rusage[mem=%dMB:tmp=%dMB] span[hosts=1]", mem, tmp),
+
+       repl := map[string]string{
+               "%%": "%",
+               "%C": fmt.Sprintf("%d", vcpus),
+               "%M": fmt.Sprintf("%d", mem),
+               "%T": fmt.Sprintf("%d", tmp),
+               "%U": container.UUID,
        }
+
+       re := regexp.MustCompile(`%.`)
+       var substitutionErrors string
+       for _, a := range disp.Cluster.Containers.LSF.BsubArgumentsList {
+               args = append(args, re.ReplaceAllStringFunc(a, func(s string) string {
+                       subst := repl[s]
+                       if len(subst) == 0 {
+                               substitutionErrors += fmt.Sprintf("Unknown substitution parameter %s in BsubArgumentsList, ", s)
+                       }
+                       return subst
+               }))
+       }
+       if len(substitutionErrors) != 0 {
+               return nil, fmt.Errorf("%s", substitutionErrors[:len(substitutionErrors)-2])
+       }
+
+       if u := disp.Cluster.Containers.LSF.BsubSudoUser; u != "" {
+               args = append([]string{"sudo", "-E", "-u", u}, args...)
+       }
+       return args, nil
 }
 
 // Check the next bjobs report, and invoke TrackContainer for all the

diff --git a/lib/lsf/dispatch_test.go b/lib/lsf/dispatch_test.go
index 44a1a3d8cb316bece57378f358b43b922df4963d..641453e5480ced43609efcb499d5dbff61383cae 100644 (file)
--- a/lib/lsf/dispatch_test.go
+++ b/lib/lsf/dispatch_test.go
@@ -72,11 +72,10 @@ func (stub lsfstub) stubCommand(s *suite, c *check.C) func(prog string, args ...
                switch prog {
                case "bsub":
                        defaultArgs := s.disp.Cluster.Containers.LSF.BsubArgumentsList
-                       c.Assert(len(args) > len(defaultArgs), check.Equals, true)
-                       c.Check(args[:len(defaultArgs)], check.DeepEquals, defaultArgs)
-                       args = args[len(defaultArgs):]
-
-                       c.Check(args[0], check.Equals, "-J")
+                       c.Assert(len(args), check.Equals, len(defaultArgs))
+                       // %%J must have been rewritten to %J
+                       c.Check(args[1], check.Equals, "/tmp/crunch-run.%J.out")
+                       args = args[4:]
                        switch args[1] {
                        case arvadostest.LockedContainerUUID:
                                c.Check(args, check.DeepEquals, []string{