Merge branch 'master' into 10231-keep-cache-runtime-constraints

diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py
index 1581d20d2f62920a5259fe936248900966947ccb..c2029b965b4f3b5173bcd981a67ff8bf3c809d3d 100644 (file)
--- a/sdk/cwl/arvados_cwl/arvcontainer.py
+++ b/sdk/cwl/arvados_cwl/arvcontainer.py
@@ -97,7 +97,7 @@ class ArvadosContainer(object):
 
         runtime_req, _ = get_feature(self, "http://arvados.org/cwl#RuntimeConstraints")
         if runtime_req:
-            logger.warn("RuntimeConstraints not yet supported by container API")
+            runtime_constraints["keep_cache_ram"] = runtime_req["keep_cache"]
 
         partition_req, _ = get_feature(self, "http://arvados.org/cwl#PartitionRequirement")
         if partition_req:

diff --git a/sdk/cwl/tests/test_container.py b/sdk/cwl/tests/test_container.py
index b5499970531008a7474f2ae99fb5de48f1234aa2..3cae2514217519a0836630e376f3507b0354a04f 100644 (file)
--- a/sdk/cwl/tests/test_container.py
+++ b/sdk/cwl/tests/test_container.py
@@ -116,6 +116,7 @@ class TestContainer(unittest.TestCase):
                 'runtime_constraints': {
                     'vcpus': 3,
                     'ram': 3145728000,
+                    'keep_cache_ram': 512,
                     'API': True,
                     'partition': ['blurb']
                 }, 'priority': 1,

diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index e5050cb79c944673343878d559b75fd3e97807ae..6a76f1f396a32c89544f55030cb586ae413d0c0b 100644 (file)
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -30,10 +30,11 @@ type Mount struct {
 // RuntimeConstraints specify a container's compute resources (RAM,
 // CPU) and network connectivity.
 type RuntimeConstraints struct {
-       API       *bool
-       RAM       int      `json:"ram"`
-       VCPUs     int      `json:"vcpus"`
-       Partition []string `json:"partition"`
+       API          *bool
+       RAM          int      `json:"ram"`
+       VCPUs        int      `json:"vcpus"`
+       KeepCacheRAM int      `json:"keep_cache_ram"`
+       Partition    []string `json:"partition"`
 }
 
 // ContainerList is an arvados#containerList resource.

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 1798150af3e83145a49762a65b778f5adf96995c..d0b3076028bf33ae16c16c5b403e85bf70898246 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -223,6 +223,14 @@ class ContainerRequest < ArvadosModel
           errors.add :runtime_constraints, "#{k} must be a positive integer"
         end
       end
+
+      if runtime_constraints.include? 'keep_cache_ram' and
+         (!runtime_constraints['keep_cache_ram'].is_a?(Integer) or
+          runtime_constraints['keep_cache_ram'] <= 0)
+            errors.add :runtime_constraints, "keep_cache_ram must be a positive integer"
+      elsif !runtime_constraints.include? 'keep_cache_ram'
+        runtime_constraints['keep_cache_ram'] = Rails.configuration.container_default_keep_cache_ram
+      end
     end
   end
 

diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 5fe03024bf7d3e9ccaee6d108c889c1078421d36..a9aa953f9f36e948dc57d34336c7d3f1cc1df43c 100644 (file)
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -393,6 +393,9 @@ common:
   # with the cancelled container.
   container_count_max: 3
 
+  # Default value for keep_cache_ram of a container's runtime_constraints.
+  container_default_keep_cache_ram: 268435456
+
 development:
   force_ssl: false
   cache_classes: false

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 406bb42d1290c6f00b55421a695fac1813154245..b2b0d57df56f9fec8a899e4c2eb6edb53027cc24 100644 (file)
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -423,7 +423,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
                       command: ["echo", "hello"],
                       output_path: "test",
                       runtime_constraints: {"vcpus" => 4,
-                                            "ram" => 12000000000},
+                                            "ram" => 12000000000,
+                                            "keep_cache_ram" => 268435456},
                       mounts: {"test" => {"kind" => "json"}}}
       set_user_from_auth :active
       cr1 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Committed,
@@ -523,4 +524,32 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal cr.container_uuid, cr3.container_uuid
     assert_equal ContainerRequest::Final, cr3.state
   end
+
+  [
+    [{"vcpus" => 1, "ram" => 123, "keep_cache_ram" => 100}, ContainerRequest::Committed, 100],
+    [{"vcpus" => 1, "ram" => 123}, ContainerRequest::Uncommitted],
+    [{"vcpus" => 1, "ram" => 123}, ContainerRequest::Committed],
+    [{"vcpus" => 1, "ram" => 123, "keep_cache_ram" => -1}, ContainerRequest::Committed, ActiveRecord::RecordInvalid],
+    [{"vcpus" => 1, "ram" => 123, "keep_cache_ram" => '123'}, ContainerRequest::Committed, ActiveRecord::RecordInvalid],
+  ].each do |rc, state, expected|
+    test "create container request with #{rc} in state #{state} and verify keep_cache_ram #{expected}" do
+      common_attrs = {cwd: "test",
+                      priority: 1,
+                      command: ["echo", "hello"],
+                      output_path: "test",
+                      runtime_constraints: rc,
+                      mounts: {"test" => {"kind" => "json"}}}
+      set_user_from_auth :active
+
+      if expected == ActiveRecord::RecordInvalid
+        assert_raises(ActiveRecord::RecordInvalid) do
+          create_minimal_req!(common_attrs.merge({state: state}))
+        end
+      else
+        cr = create_minimal_req!(common_attrs.merge({state: state}))
+        expected = Rails.configuration.container_default_keep_cache_ram if state == ContainerRequest::Committed and expected.nil?
+        assert_equal expected, cr.runtime_constraints['keep_cache_ram']
+      end
+    end
+  end
 end

diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 0b59a3bb78c1b0c8919f198f5c34cc94ed00d5a9..8e5cdb1f3b20ef1fbd9ef6114689148d12f0e781 100644 (file)
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -357,6 +357,10 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
        }
        arvMountCmd = append(arvMountCmd, runner.ArvMountPoint)
 
+       if runner.Container.RuntimeConstraints.KeepCacheRAM > 0 {
+               arvMountCmd = append(arvMountCmd, "--file-cache", fmt.Sprintf("%d", runner.Container.RuntimeConstraints.KeepCacheRAM))
+       }
+
        token, err := runner.ContainerToken()
        if err != nil {
                return fmt.Errorf("could not get container token: %s", err)

diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 7f8e80cb107296b4184dfad5c1c76f9a29585c92..2fbbb4db97da2be13eeb5f4f37bc31ed2fb61c09 100644 (file)
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -842,6 +842,28 @@ func (s *TestSuite) TestSetupMounts(c *C) {
                checkEmpty()
        }
 
+       {
+               i = 0
+               cr.Container.RuntimeConstraints.KeepCacheRAM = 512
+               cr.Container.Mounts = map[string]arvados.Mount{
+                       "/keepinp": {Kind: "collection", PortableDataHash: "59389a8f9ee9d399be35462a0f92541c+53"},
+                       "/keepout": {Kind: "collection", Writable: true},
+               }
+               cr.OutputPath = "/keepout"
+
+               os.MkdirAll(realTemp+"/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53", os.ModePerm)
+               os.MkdirAll(realTemp+"/keep1/tmp0", os.ModePerm)
+
+               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", "--file-cache", "512"})
+               sort.StringSlice(cr.Binds).Sort()
+               c.Check(cr.Binds, DeepEquals, []string{realTemp + "/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53:/keepinp:ro",
+                       realTemp + "/keep1/tmp0:/keepout"})
+               cr.CleanupDirs()
+               checkEmpty()
+       }
+
        for _, test := range []struct {
                in  interface{}
                out string