[ARVADOS] created: b7f8614e177be3fb989f5ddf17ba474492d19af1

[Git user]

        at  b7f8614e177be3fb989f5ddf17ba474492d19af1 (commit)


commit b7f8614e177be3fb989f5ddf17ba474492d19af1
Author: Lucas Di Pentima <lucas at curoverse.com>
Date:   Fri Jul 29 18:07:15 2016 -0300

    9617, 9618: Added validations and wrote the corresponding test. Also, corrected some other tests to conform this new requirement.

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index e84026a..bf244bd 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -17,6 +17,7 @@ class ContainerRequest < ArvadosModel
   validates :command, :container_image, :output_path, :cwd, :presence => true
   validate :validate_state_change
   validate :validate_change
+  validate :validate_runtime_constraints
   after_save :update_priority
   before_create :set_requesting_container_uuid
 
@@ -170,6 +171,19 @@ class ContainerRequest < ArvadosModel
     end
   end
 
+  def validate_runtime_constraints
+    case self.state
+    when Committed
+      ['vcpus', 'ram'].each do |k|
+        if not (runtime_constraints.include? k and
+                runtime_constraints[k].is_a? Integer and
+                runtime_constraints[k] > 0)
+          errors.add :runtime_constraints, "#{k} must be a positive integer"
+        end
+      end
+    end
+  end
+
   def validate_change
     permitted = [:owner_uuid]
 
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 8621daa..1156979 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -9,7 +9,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
       environment: {},
       mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}},
       output_path: "/out",
-      runtime_constraints: {},
+      runtime_constraints: {"vcpus" => 1, "ram" => 2},
       name: "foo",
       description: "bar",
     }
@@ -53,6 +53,23 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_nil cr.container_uuid
   end
 
+  test "Container request constraints must include valid vcpus and ram fields when committed" do
+    set_user_from_auth :active
+    cr = create_minimal_req!(state: "Committed", priority: 1)
+    assert_raises(ActiveRecord::RecordInvalid) do
+      cr.runtime_constraints = {"vcpus" => 1}
+      cr.save!
+    end
+    assert_raises(ActiveRecord::RecordInvalid) do
+      cr.runtime_constraints = {"vcpus" => 1, "ram" => nil}
+      cr.save!
+    end
+    assert_raises(ActiveRecord::RecordInvalid) do
+      cr.runtime_constraints = {"vcpus" => 0, "ram" => 123}
+      cr.save!
+    end
+  end
+
   test "Container request priority must be non-nil" do
     set_user_from_auth :active
     cr = create_minimal_req!(priority: nil)
@@ -64,7 +81,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
   test "Container request commit" do
     set_user_from_auth :active
-    cr = create_minimal_req!(runtime_constraints: {"vcpus" => [2,3]})
+    cr = create_minimal_req!(runtime_constraints: {"vcpus" => 2, "ram" => 30})
 
     assert_nil cr.container_uuid
 
@@ -84,7 +101,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal({}, c.environment)
     assert_equal({"/out" => {"kind"=>"tmp", "capacity"=>1000000}}, c.mounts)
     assert_equal "/out", c.output_path
-    assert_equal({"vcpus" => 2}, c.runtime_constraints)
+    assert_equal({"vcpus" => 2, "ram" => 30}, c.runtime_constraints)
     assert_equal 1, c.priority
 
     assert_raises(ActiveRecord::RecordInvalid) do
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 9cc0981..24186e8 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -8,6 +8,7 @@ class ContainerTest < ActiveSupport::TestCase
     container_image: 'img',
     output_path: '/tmp',
     priority: 1,
+    runtime_constraints: {"vcpus" => 1, "ram" => 1},
   }
 
   def minimal_new attrs={}
@@ -66,7 +67,7 @@ class ContainerTest < ActiveSupport::TestCase
                       mounts: {"BAR" => "FOO"},
                       output_path: "/tmp",
                       priority: 1,
-                      runtime_constraints: {})
+                      runtime_constraints: {"vcpus" => 1, "ram" => 1})
 
       check_illegal_modify c
       check_bogus_states c

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


hooks/post-receive
--