12573: Clamp range of priority to [0,1000] for container & container request.

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

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 466adc0eefb573a08b18dff2d5280102ef448210..98e523339f986f254ec3ef9946d729e6b42bb24d 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -24,6 +24,7 @@ class Container < ArvadosModel
   before_validation :fill_field_defaults, :if => :new_record?
   before_validation :set_timestamps
   validates :command, :container_image, :output_path, :cwd, :priority, :presence => true
+  validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 }
   validate :validate_state_change
   validate :validate_change
   validate :validate_lock

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 94e4e1f9ddd4289bc8bbe23c0f61d54d06213a18..178c15645abf64751da45a9371565b32ec6a02b1 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -23,6 +23,7 @@ class ContainerRequest < ArvadosModel
   before_validation :set_container
   validates :command, :container_image, :output_path, :cwd, :presence => true
   validates :output_ttl, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+  validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 }
   validate :validate_state_change
   validate :check_update_whitelist
   after_save :update_priority
@@ -158,6 +159,7 @@ class ContainerRequest < ArvadosModel
     self.container_count_max ||= Rails.configuration.container_count_max
     self.scheduling_parameters ||= {}
     self.output_ttl ||= 0
+    self.priority ||= 500
   end
 
   def set_container

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index cecf7b818e7004f74fb4544033d1a6e636e55069..68f5da0363cd56efe9ca4415f5f02ffd77410bf2 100644 (file)
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -41,7 +41,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     cr = create_minimal_req!
 
     assert_nil cr.container_uuid
-    assert_nil cr.priority
+    assert_equal 500, cr.priority
 
     check_bogus_states cr
 
@@ -109,6 +109,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
   test "Container request priority must be non-nil" do
     set_user_from_auth :active
     cr = create_minimal_req!(priority: nil)
+    cr.priority = nil
     cr.state = "Committed"
     assert_raises(ActiveRecord::RecordInvalid) do
       cr.save!
@@ -803,4 +804,35 @@ class ContainerRequestTest < ActiveSupport::TestCase
       assert_nothing_raised {cr.destroy}
     end
   end
+
+  test "Container request valid priority" do
+    set_user_from_auth :active
+    cr = create_minimal_req!
+
+    assert_raises(ActiveRecord::RecordInvalid) do
+      cr.priority = -1
+      cr.save!
+    end
+
+    cr.priority = 0
+    cr.save!
+
+    cr.priority = 1
+    cr.save!
+
+    cr.priority = 500
+    cr.save!
+
+    cr.priority = 999
+    cr.save!
+
+    cr.priority = 1000
+    cr.save!
+
+    assert_raises(ActiveRecord::RecordInvalid) do
+      cr.priority = 1001
+      cr.save!
+    end
+  end
+
 end

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 09373fdc05588ea593a0ff33906484d47be082ef..ab53240708e294d6aa1edb7aecd1e1ebdcf28fc8 100644 (file)
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -95,6 +95,42 @@ class ContainerTest < ActiveSupport::TestCase
     end
   end
 
+  test "Container request valid priority" do
+    act_as_system_user do
+      cr, _ = minimal_new(environment: {},
+                      mounts: {"BAR" => "FOO"},
+                      output_path: "/tmp",
+                      priority: 1,
+                      runtime_constraints: {"vcpus" => 1, "ram" => 1})
+
+      assert_raises(ActiveRecord::RecordInvalid) do
+        cr.priority = -1
+        cr.save!
+      end
+
+      cr.priority = 0
+      cr.save!
+
+      cr.priority = 1
+      cr.save!
+
+      cr.priority = 500
+      cr.save!
+
+      cr.priority = 999
+      cr.save!
+
+      cr.priority = 1000
+      cr.save!
+
+      assert_raises(ActiveRecord::RecordInvalid) do
+        cr.priority = 1001
+        cr.save!
+      end
+    end
+  end
+
+
   test "Container serialized hash attributes sorted before save" do
     env = {"C" => 3, "B" => 2, "A" => 1}
     m = {"F" => {"kind" => 3}, "E" => {"kind" => 2}, "D" => {"kind" => 1}}