[ARVADOS] created: e3c6b307c9aa963c22457cd42eed41047d4c72c9

[Git user]

        at  e3c6b307c9aa963c22457cd42eed41047d4c72c9 (commit)


commit e3c6b307c9aa963c22457cd42eed41047d4c72c9
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jun 23 14:16:45 2016 -0400

    8470: Resolve runtime_constraints ranges to numbers when satisfying a Container Request.

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 496a6b1..221da0d 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -82,8 +82,7 @@ class ContainerRequest < ArvadosModel
   # Create a new container (or find an existing one) to satisfy this
   # request.
   def resolve
-    # TODO: resolve symbolic git and keep references to content
-    # addresses.
+    # TODO: resolve mounts and container_image to content addresses.
     c = act_as_system_user do
       Container.create!(command: self.command,
                         container_image: self.container_image,
@@ -91,11 +90,32 @@ class ContainerRequest < ArvadosModel
                         environment: self.environment,
                         mounts: self.mounts,
                         output_path: self.output_path,
-                        runtime_constraints: self.runtime_constraints)
+                        runtime_constraints: runtime_constraints_for_container)
     end
     self.container_uuid = c.uuid
   end
 
+  # Return a runtime_constraints hash that complies with
+  # self.runtime_constraints but is suitable for saving in a container
+  # record, i.e., has specific values instead of ranges.
+  #
+  # Doing this as a step separate from other resolutions, like "git
+  # revision range to commit hash", makes sense only when there is no
+  # opportunity to reuse an existing container (e.g., container reuse
+  # is not implemented yet, or we have already found that no existing
+  # containers are suitable).
+  def runtime_constraints_for_container
+    rc = {}
+    runtime_constraints.each do |k, v|
+      if v.is_a? Array
+        rc[k] = v[0]
+      else
+        rc[k] = v
+      end
+    end
+    rc
+  end
+
   def set_container
     if (container_uuid_changed? and
         not current_user.andand.is_admin and
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index df89b93..f510f25 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -1,6 +1,24 @@
 require 'test_helper'
 
 class ContainerRequestTest < ActiveSupport::TestCase
+  def create_minimal_req! attrs={}
+    defaults = {
+      command: ["echo", "foo"],
+      container_image: "img",
+      cwd: "/tmp",
+      environment: {},
+      mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}},
+      output_path: "/out",
+      priority: 1,
+      runtime_constraints: {},
+      name: "foo",
+      description: "bar",
+    }
+    cr = ContainerRequest.create!(defaults.merge(attrs))
+    cr.reload
+    return cr
+  end
+
   def check_illegal_modify c
       assert_raises(ActiveRecord::RecordInvalid) do
         c.reload
@@ -129,21 +147,9 @@ class ContainerRequestTest < ActiveSupport::TestCase
   end
 
   test "Container request commit" do
-    set_user_from_auth :active_trustedclient
-    cr = ContainerRequest.new
-    cr.command = ["echo", "foo"]
-    cr.container_image = "img"
-    cr.cwd = "/tmp"
-    cr.environment = {}
-    cr.mounts = {"BAR" => "FOO"}
-    cr.output_path = "/tmpout"
-    cr.priority = 1
-    cr.runtime_constraints = {}
-    cr.name = "foo"
-    cr.description = "bar"
-    cr.save!
+    set_user_from_auth :active
+    cr = create_minimal_req!(runtime_constraints: {"vcpus" => [2,3]})
 
-    cr.reload
     assert_nil cr.container_uuid
 
     cr.reload
@@ -152,14 +158,16 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
     cr.reload
 
+    assert_not_nil cr.container_uuid
     c = Container.find_by_uuid cr.container_uuid
+    assert_not_nil c
     assert_equal ["echo", "foo"], c.command
     assert_equal "img", c.container_image
     assert_equal "/tmp", c.cwd
     assert_equal({}, c.environment)
-    assert_equal({"BAR" => "FOO"}, c.mounts)
-    assert_equal "/tmpout", c.output_path
-    assert_equal({}, c.runtime_constraints)
+    assert_equal({"/out" => {"kind"=>"tmp", "capacity"=>1000000}}, c.mounts)
+    assert_equal "/out", c.output_path
+    assert_equal({"vcpus" => 2}, c.runtime_constraints)
     assert_equal 1, c.priority
 
     assert_raises(ActiveRecord::RecordInvalid) do
@@ -379,4 +387,23 @@ class ContainerRequestTest < ActiveSupport::TestCase
       assert_equal expected, cr.requesting_container_uuid
     end
   end
+
+  [[{"vcpus" => [2, nil]},
+    lambda { |resolved| resolved["vcpus"] == 2 }],
+   [{"vcpus" => [3, 7]},
+    lambda { |resolved| resolved["vcpus"] == 3 }],
+   [{"vcpus" => 4},
+    lambda { |resolved| resolved["vcpus"] == 4 }],
+   [{"ram" => [1000000000, 2000000000]},
+    lambda { |resolved| resolved["ram"] == 1000000000 }],
+   [{"ram" => [1234234234]},
+    lambda { |resolved| resolved["ram"] == 1234234234 }],
+  ].each do |rc, okfunc|
+    test "resolve runtime constraint range #{rc} to values" do
+      cr = ContainerRequest.new(runtime_constraints: rc)
+      resolved = cr.send :runtime_constraints_for_container
+      assert(okfunc.call(resolved),
+             "container runtime_constraints was #{resolved.inspect}")
+    end
+  end
 end

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


hooks/post-receive
--