[ARVADOS] updated: 6fc8952ed133607f5ce317d929d731657e405edf
git at public.curoverse.com
git at public.curoverse.com
Tue Dec 8 11:58:05 EST 2015
Summary of changes:
services/api/app/models/container.rb | 17 +-
services/api/app/models/container_request.rb | 40 +++--
services/api/test/unit/container_request_test.rb | 209 ++++++++++++++++++++++-
3 files changed, 241 insertions(+), 25 deletions(-)
via 6fc8952ed133607f5ce317d929d731657e405edf (commit)
from b1b93d2d90e58fbdfe4a4f8e363a4705dbd39bd5 (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 6fc8952ed133607f5ce317d929d731657e405edf
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Tue Dec 8 11:57:52 2015 -0500
6429: Tests for priority update propagation to process tree, max priority from
multiple requests, finalize request when container completes or cancels.
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index f87004a..f452b10 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -46,7 +46,10 @@ class Container < ArvadosModel
(Cancelled = 'Cancelled')
]
+ # Turn a container request into a container.
def self.resolve req
+ # In the future this will do things like resolve symbolic git and keep
+ # references to content addresses.
Container.create!({ :command => req.command,
:container_image => req.container_image,
:cwd => req.cwd,
@@ -57,9 +60,11 @@ class Container < ArvadosModel
end
def update_priority!
+ # Update the priority of this container to the maximum priority of any of
+ # its committed container requests and save the record.
max = 0
ContainerRequest.where(container_uuid: uuid).each do |cr|
- if cr.priority > max
+ if cr.state == "Committed" and cr.priority > max
max = cr.priority
end
end
@@ -110,18 +115,18 @@ class Container < ArvadosModel
permitted.push :priority
if self.state_changed? and not self.state_was.nil?
- errors.add :state, "Can only go to Queued from nil"
+ errors.add :state, "Can only go to from nil to Queued"
end
when Running
if self.state_changed?
- # At point of state change, can only set state and started_at
+ # At point of state change, can set state and started_at
if self.state_was == Queued
permitted.push :state, :started_at
else
errors.add :state, "Can only go from Queued to Running"
end
- else
+
# While running, can update priority and progress.
permitted.push :priority, :progress
end
@@ -161,7 +166,7 @@ class Container < ArvadosModel
if self.state_changed? and [Complete, Cancelled].include? self.state
act_as_system_user do
ContainerRequest.where(container_uuid: uuid).each do |cr|
- cr.state = ContainerRequest.Final
+ cr.state = "Final"
cr.save!
end
end
@@ -170,6 +175,8 @@ class Container < ArvadosModel
def process_tree_priority
if self.priority_changed?
+ # This could propagate any parent priority to the children (not just
+ # priority 0)
if self.priority == 0
act_as_system_user do
ContainerRequest.where(requesting_container_uuid: uuid).each do |cr|
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index f936468..9dc5c7b 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -46,6 +46,11 @@ class ContainerRequest < ArvadosModel
(Final = 'Final'),
]
+ def skip_uuid_read_permission_check
+ # XXX temporary until permissions are sorted out.
+ %w(modified_by_client_uuid container_uuid requesting_container_uuid)
+ end
+
protected
def fill_field_defaults
@@ -58,14 +63,16 @@ class ContainerRequest < ArvadosModel
end
def set_container
- if self.state_changed?
- if self.state == Committed and (self.state_was == Uncommitted or self.state_was.nil?)
- act_as_system_user do
- self.container_uuid = Container.resolve(self).andand.uuid
- Link.create!(head_uuid: self.container_uuid,
- tail_uuid: self.owner_uuid,
- link_class: "permission",
- name: "can_read")
+ if self.container_uuid_changed?
+ if not current_user.andand.is_admin and not self.container_uuid.nil?
+ errors.add :container_uuid, "Cannot only update container_uuid to nil."
+ end
+ else
+ if self.state_changed?
+ if self.state == Committed and (self.state_was == Uncommitted or self.state_was.nil?)
+ act_as_system_user do
+ self.container_uuid = Container.resolve(self).andand.uuid
+ end
end
end
end
@@ -89,15 +96,14 @@ class ContainerRequest < ArvadosModel
end
# Can update priority, container count.
- permitted.push :priority, :container_count_max
+ permitted.push :priority, :container_count_max, :container_uuid
if self.state_changed?
if self.state_was == Uncommitted or self.state_was.nil?
# Allow create-and-commit in a single operation.
- permitted.push :command, :container_count_max,
- :container_image, :cwd, :description, :environment,
- :filters, :mounts, :name, :output_path, :priority,
- :properties, :requesting_container_uuid, :runtime_constraints,
+ permitted.push :command, :container_image, :cwd, :description, :environment,
+ :filters, :mounts, :name, :output_path, :properties,
+ :requesting_container_uuid, :runtime_constraints,
:state, :container_uuid
else
errors.add :state, "Can only go from Uncommitted to Committed"
@@ -123,9 +129,13 @@ class ContainerRequest < ArvadosModel
end
def update_priority
- if self.state == Committed and self.priority_changed?
+ if self.state == Committed and (self.state_changed? or
+ self.priority_changed? or
+ self.container_uuid_changed?)
c = Container.find_by_uuid self.container_uuid
- c.update_priority!
+ act_as_system_user do
+ c.update_priority!
+ end
end
end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 1248475..32a60f8 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -80,7 +80,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
c.cwd = "/tmp"
c.environment = {}
c.mounts = {"BAR" => "FOO"}
- c.output_path = "/tmp"
+ c.output_path = "/tmpout"
c.priority = 1
c.runtime_constraints = {}
c.name = "foo"
@@ -115,7 +115,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
c.cwd = "/tmp"
c.environment = {}
c.mounts = {"BAR" => "FOO"}
- c.output_path = "/tmp"
+ c.output_path = "/tmpout"
c.priority = 1
c.runtime_constraints = {}
c.name = "foo"
@@ -132,10 +132,209 @@ class ContainerRequestTest < ActiveSupport::TestCase
c.reload
t = Container.find_by_uuid c.container_uuid
- assert_equal c.command, t.command
- assert_equal c.container_image, t.container_image
- assert_equal c.cwd, t.cwd
+ assert_equal t.command, ["echo", "foo"]
+ assert_equal t.container_image, "img"
+ assert_equal t.cwd, "/tmp"
+ assert_equal t.environment, {}
+ assert_equal t.mounts, {"BAR" => "FOO"}
+ assert_equal t.output_path, "/tmpout"
+ assert_equal t.runtime_constraints, {}
+ assert_equal t.priority, 1
+
+ c.priority = 0
+ c.save!
+
+ c.reload
+ t.reload
+ assert_equal c.priority, 0
+ assert_equal t.priority, 0
+
+ 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 t.priority, 5
+
+ c2 = ContainerRequest.new
+ c2.container_image = "img"
+ c2.command = ["foo", "bar"]
+ c2.output_path = "/tmp"
+ c2.cwd = "/tmp"
+ c2.priority = 10
+ c2.save!
+
+ act_as_system_user do
+ c2.state = "Committed"
+ c2.container_uuid = c.container_uuid
+ c2.save!
+ end
+
+ t.reload
+ assert_equal t.priority, 10
+
+ c2.reload
+ c2.priority = 0
+ c2.save!
+
+ t.reload
+ assert_equal t.priority, 5
+
+ c.reload
+ c.priority = 0
+ c.save!
+
+ t.reload
+ assert_equal t.priority, 0
+
+ end
+
+ test "Container cancel process tree" 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!
+
+ c2 = ContainerRequest.new
+ c2.state = "Committed"
+ c2.container_image = "img"
+ c2.command = ["foo", "bar"]
+ c2.output_path = "/tmp"
+ c2.cwd = "/tmp"
+ c2.priority = 10
+ c2.requesting_container_uuid = c.container_uuid
+ c2.save!
+
+ t = Container.find_by_uuid c.container_uuid
+ assert_equal t.priority, 5
+
+ t2 = Container.find_by_uuid c2.container_uuid
+ assert_equal t2.priority, 10
+
+ c.priority = 0
+ c.save!
+
+ t.reload
+ assert_equal t.priority, 0
+
+ t2.reload
+ assert_equal t2.priority, 0
+
+ end
+
+
+ test "Independent containers" 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!
+
+ c2 = ContainerRequest.new
+ c2.state = "Committed"
+ c2.container_image = "img"
+ c2.command = ["foo", "bar"]
+ c2.output_path = "/tmp"
+ c2.cwd = "/tmp"
+ c2.priority = 10
+ c2.save!
+
+ t = Container.find_by_uuid c.container_uuid
+ assert_equal t.priority, 5
+
+ t2 = Container.find_by_uuid c2.container_uuid
+ assert_equal t2.priority, 10
+
+ c.priority = 0
+ c.save!
+
+ t.reload
+ assert_equal t.priority, 0
+
+ t2.reload
+ assert_equal t2.priority, 10
+ end
+
+ test "Container container cancel" 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!
+
+ c.reload
+ assert_equal c.state, "Committed"
+
+ t = Container.find_by_uuid c.container_uuid
+ assert_equal t.state, "Queued"
+
+ act_as_system_user do
+ t.state = "Cancelled"
+ t.save!
+ end
+
+ c.reload
+ assert_equal c.state, "Final"
+
end
+ test "Container container complete" 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!
+
+ c.reload
+ assert_equal c.state, "Committed"
+
+ t = Container.find_by_uuid c.container_uuid
+ assert_equal t.state, "Queued"
+
+ act_as_system_user do
+ t.state = "Running"
+ t.save!
+ end
+
+ c.reload
+ assert_equal c.state, "Committed"
+
+ act_as_system_user do
+ t.state = "Complete"
+ t.save!
+ end
+
+ c.reload
+ assert_equal c.state, "Final"
+
+ end
+
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list