[ARVADOS] updated: c53fb9e0fab0ce043602c50e25d7fe8b94c15b79
git at public.curoverse.com
git at public.curoverse.com
Thu Dec 10 16:37:36 EST 2015
Summary of changes:
services/api/app/models/container.rb | 6 +-
services/api/app/models/container_request.rb | 9 +-
services/api/test/unit/container_request_test.rb | 359 ++++++++++++-----------
3 files changed, 193 insertions(+), 181 deletions(-)
via c53fb9e0fab0ce043602c50e25d7fe8b94c15b79 (commit)
from b781e22ea6be76ece696c80de6b59ae223f3a06f (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit c53fb9e0fab0ce043602c50e25d7fe8b94c15b79
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Dec 10 16:37:29 2015 -0500
6429: Improve variable names in tests a bit. Default priority of container
request to nil and check for nil priority. Complete/cancelled container
finalizes requests that created it and sets priority 0 on requests that it
created itself.
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 903f392..33607af 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -13,7 +13,7 @@ class Container < ArvadosModel
before_validation :fill_field_defaults, :if => :new_record?
before_validation :set_timestamps
- validates :command, :container_image, :output_path, :cwd, :presence => true
+ validates :command, :container_image, :output_path, :cwd, :priority, :presence => true
validate :validate_state_change
validate :validate_change
after_save :request_finalize
@@ -159,10 +159,10 @@ class Container < ArvadosModel
cr.save
end
- # Try to close any outstanding container requests made by this container.
+ # Try to cancel any outstanding container requests made by this container.
ContainerRequest.where(requesting_container_uuid: uuid,
:state => ContainerRequest::Committed).each do |cr|
- cr.state = ContainerRequest::Final
+ cr.priority = 0
cr.save
end
end
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 4e76a0e..acadaa7 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -70,7 +70,6 @@ class ContainerRequest < ArvadosModel
self.runtime_constraints ||= {}
self.mounts ||= {}
self.cwd ||= "."
- self.priority ||= 1
end
# Turn a container request into a container.
@@ -119,6 +118,10 @@ class ContainerRequest < ArvadosModel
errors.add :container_uuid, "has not been resolved to a container."
end
+ if priority.nil?
+ errors.add :priority, "cannot be nil"
+ end
+
# Can update priority, container count.
permitted.push :priority, :container_count_max, :container_uuid
@@ -131,6 +134,10 @@ class ContainerRequest < ArvadosModel
end
when Final
+ if not current_user.andand.is_admin
+ errors.add :state, "of container request can only be set to Final by system."
+ end
+
if self.state_changed?
permitted.push :state
else
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 6470b78..74c230c 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -74,164 +74,168 @@ class ContainerRequestTest < ActiveSupport::TestCase
test "Container request create" do
set_user_from_auth :active_trustedclient
- c = ContainerRequest.new
- c.command = ["echo", "foo"]
- c.container_image = "img"
- c.cwd = "/tmp"
- c.environment = {}
- c.mounts = {"BAR" => "FOO"}
- c.output_path = "/tmpout"
- c.priority = 1
- c.runtime_constraints = {}
- c.name = "foo"
- c.description = "bar"
- c.save!
-
- assert_nil c.container_uuid
-
- check_bogus_states c
+ 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.runtime_constraints = {}
+ cr.name = "foo"
+ cr.description = "bar"
+ cr.save!
+
+ assert_nil cr.container_uuid
+ assert_nil cr.priority
+
+ check_bogus_states cr
+
+ cr.reload
+ cr.command = ["echo", "foo3"]
+ cr.container_image = "img3"
+ cr.cwd = "/tmp3"
+ cr.environment = {"BUP" => "BOP"}
+ cr.mounts = {"BAR" => "BAZ"}
+ cr.output_path = "/tmp4"
+ cr.priority = 2
+ cr.runtime_constraints = {"X" => "Y"}
+ cr.name = "foo3"
+ cr.description = "bar3"
+ cr.save!
+
+ assert_nil cr.container_uuid
+ end
- c.reload
- c.command = ["echo", "foo3"]
- c.container_image = "img3"
- c.cwd = "/tmp3"
- c.environment = {"BUP" => "BOP"}
- c.mounts = {"BAR" => "BAZ"}
- c.output_path = "/tmp4"
- c.priority = 2
- c.runtime_constraints = {"X" => "Y"}
- c.name = "foo3"
- c.description = "bar3"
- c.save!
-
- assert_nil c.container_uuid
+ test "Container request priority must be non-nil" 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.runtime_constraints = {}
+ cr.name = "foo"
+ cr.description = "bar"
+ cr.save!
+
+ cr.reload
+ cr.state = "Committed"
+ assert_raises(ActiveRecord::RecordInvalid) do
+ cr.save!
+ end
end
test "Container request commit" do
set_user_from_auth :active_trustedclient
- c = ContainerRequest.new
- c.command = ["echo", "foo"]
- c.container_image = "img"
- c.cwd = "/tmp"
- c.environment = {}
- c.mounts = {"BAR" => "FOO"}
- c.output_path = "/tmpout"
- c.priority = 1
- c.runtime_constraints = {}
- c.name = "foo"
- c.description = "bar"
- c.save!
-
- c.reload
- assert_nil c.container_uuid
-
- c.reload
- c.state = "Committed"
- c.save!
-
- c.reload
-
- t = Container.find_by_uuid c.container_uuid
- assert_equal ["echo", "foo"], t.command
- assert_equal "img", t.container_image
- assert_equal "/tmp", t.cwd
- assert_equal({}, t.environment)
- assert_equal({"BAR" => "FOO"}, t.mounts)
- assert_equal "/tmpout", t.output_path
- assert_equal({}, t.runtime_constraints)
- assert_equal 1, t.priority
+ 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!
+
+ cr.reload
+ assert_nil cr.container_uuid
+
+ cr.reload
+ cr.state = "Committed"
+ cr.save!
+
+ cr.reload
+
+ c = Container.find_by_uuid cr.container_uuid
+ 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 1, c.priority
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ cr.priority = nil
+ cr.save!
+ end
- c.priority = 0
- c.save!
+ cr.priority = 0
+ cr.save!
+ cr.reload
c.reload
- t.reload
+ assert_equal 0, cr.priority
assert_equal 0, c.priority
- assert_equal 0, t.priority
end
test "Container request max priority" do
set_user_from_auth :active_trustedclient
- c = ContainerRequest.new
- c.state = "Committed"
- c.container_image = "img"
- c.command = ["foo", "bar"]
- c.output_path = "/tmp"
- c.cwd = "/tmp"
- c.priority = 5
- c.save!
-
- t = Container.find_by_uuid c.container_uuid
- assert_equal 5, t.priority
-
- c2 = ContainerRequest.new
- c2.container_image = "img"
- c2.command = ["foo", "bar"]
- c2.output_path = "/tmp"
- c2.cwd = "/tmp"
- c2.priority = 10
- c2.save!
+ cr = ContainerRequest.new
+ cr.state = "Committed"
+ cr.container_image = "img"
+ cr.command = ["foo", "bar"]
+ cr.output_path = "/tmp"
+ cr.cwd = "/tmp"
+ cr.priority = 5
+ cr.save!
+
+ c = Container.find_by_uuid cr.container_uuid
+ assert_equal 5, c.priority
+
+ cr2 = ContainerRequest.new
+ cr2.container_image = "img"
+ cr2.command = ["foo", "bar"]
+ cr2.output_path = "/tmp"
+ cr2.cwd = "/tmp"
+ cr2.priority = 10
+ cr2.save!
act_as_system_user do
- c2.state = "Committed"
- c2.container_uuid = c.container_uuid
- c2.save!
+ cr2.state = "Committed"
+ cr2.container_uuid = cr.container_uuid
+ cr2.save!
end
- t.reload
- assert_equal 10, t.priority
-
- c2.reload
- c2.priority = 0
- c2.save!
-
- t.reload
- assert_equal 5, t.priority
-
c.reload
- c.priority = 0
- c.save!
-
- t.reload
- assert_equal 0, t.priority
+ assert_equal 10, c.priority
- end
-
- test "Container request finalize" do
- set_user_from_auth :active_trustedclient
- c = ContainerRequest.new
- c.state = "Committed"
- c.container_image = "img"
- c.command = ["foo", "bar"]
- c.output_path = "/tmp"
- c.cwd = "/tmp"
- c.priority = 5
- c.save!
+ cr2.reload
+ cr2.priority = 0
+ cr2.save!
- t = Container.find_by_uuid c.container_uuid
- assert_equal 5, t.priority
+ c.reload
+ assert_equal 5, c.priority
- c.state = "Final"
- c.save!
+ cr.reload
+ cr.priority = 0
+ cr.save!
- t.reload
- assert_equal 0, t.priority
+ c.reload
+ assert_equal 0, c.priority
end
- test "Independent containers" do
+ test "Independent container requests" do
set_user_from_auth :active_trustedclient
- c = ContainerRequest.new
- c.state = "Committed"
- c.container_image = "img"
- c.command = ["foo", "bar"]
- c.output_path = "/tmp"
- c.cwd = "/tmp"
- c.priority = 5
- c.save!
+ cr = ContainerRequest.new
+ cr.state = "Committed"
+ cr.container_image = "img"
+ cr.command = ["foo", "bar"]
+ cr.output_path = "/tmp"
+ cr.cwd = "/tmp"
+ cr.priority = 5
+ cr.save!
c2 = ContainerRequest.new
c2.state = "Committed"
@@ -242,82 +246,83 @@ class ContainerRequestTest < ActiveSupport::TestCase
c2.priority = 10
c2.save!
- t = Container.find_by_uuid c.container_uuid
- assert_equal 5, t.priority
+ c = Container.find_by_uuid cr.container_uuid
+ assert_equal 5, c.priority
- t2 = Container.find_by_uuid c2.container_uuid
- assert_equal 10, t2.priority
+ c2 = Container.find_by_uuid c2.container_uuid
+ assert_equal 10, c2.priority
- c.priority = 0
- c.save!
+ cr.priority = 0
+ cr.save!
- t.reload
- assert_equal 0, t.priority
+ c.reload
+ assert_equal 0, c.priority
- t2.reload
- assert_equal 10, t2.priority
+ c2.reload
+ assert_equal 10, c2.priority
end
- test "Container container cancel" do
+
+ test "Container cancelled finalizes request" do
set_user_from_auth :active_trustedclient
- c = ContainerRequest.new
- c.state = "Committed"
- c.container_image = "img"
- c.command = ["foo", "bar"]
- c.output_path = "/tmp"
- c.cwd = "/tmp"
- c.priority = 5
- c.save!
+ cr = ContainerRequest.new
+ cr.state = "Committed"
+ cr.container_image = "img"
+ cr.command = ["foo", "bar"]
+ cr.output_path = "/tmp"
+ cr.cwd = "/tmp"
+ cr.priority = 5
+ cr.save!
- c.reload
- assert_equal "Committed", c.state
+ cr.reload
+ assert_equal "Committed", cr.state
- t = Container.find_by_uuid c.container_uuid
- assert_equal "Queued", t.state
+ c = Container.find_by_uuid cr.container_uuid
+ assert_equal "Queued", c.state
act_as_system_user do
- t.state = "Cancelled"
- t.save!
+ c.state = "Cancelled"
+ c.save!
end
- c.reload
- assert_equal "Final", c.state
+ cr.reload
+ assert_equal "Final", cr.state
end
- test "Container container complete" do
+ test "Container complete finalizes request" do
set_user_from_auth :active_trustedclient
- c = ContainerRequest.new
- c.state = "Committed"
- c.container_image = "img"
- c.command = ["foo", "bar"]
- c.output_path = "/tmp"
- c.cwd = "/tmp"
- c.priority = 5
- c.save!
+ cr = ContainerRequest.new
+ cr.state = "Committed"
+ cr.container_image = "img"
+ cr.command = ["foo", "bar"]
+ cr.output_path = "/tmp"
+ cr.cwd = "/tmp"
+ cr.priority = 5
+ cr.save!
- c.reload
- assert_equal "Committed", c.state
+ cr.reload
+ assert_equal "Committed", cr.state
- t = Container.find_by_uuid c.container_uuid
- assert_equal "Queued", t.state
+ c = Container.find_by_uuid cr.container_uuid
+ assert_equal "Queued", c.state
act_as_system_user do
- t.state = "Running"
- t.save!
+ c.state = "Running"
+ c.save!
end
- c.reload
- assert_equal "Committed", c.state
+ cr.reload
+ assert_equal "Committed", cr.state
act_as_system_user do
- t.state = "Complete"
- t.save!
+ c.state = "Complete"
+ c.save!
end
- c.reload
- assert_equal "Final", c.state
+ cr.reload
+ assert_equal "Final", cr.state
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list