[ARVADOS] updated: d1baf718d0866c64252006bf61a6f0c5da353f7b
git at public.curoverse.com
git at public.curoverse.com
Thu Dec 3 17:22:59 EST 2015
Summary of changes:
services/api/app/models/container.rb | 91 ++++++++++++++++++++++--
services/api/app/models/container_request.rb | 73 +++++++++++++++++++
services/api/test/unit/container_test.rb | 101 ++++++++++++++++++++++++++-
3 files changed, 256 insertions(+), 9 deletions(-)
via d1baf718d0866c64252006bf61a6f0c5da353f7b (commit)
from d9f8f46ccd5a418dcf7b5f43aeb59cd2d9d424ba (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 d1baf718d0866c64252006bf61a6f0c5da353f7b
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Dec 3 17:22:54 2015 -0500
Working on state change assertions and testing for container record.
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 66b33cd..a0ac077 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -3,15 +3,17 @@ class Container < ArvadosModel
include KindAndEtag
include CommonApiTemplate
- serialize :properties, Hash
serialize :environment, Hash
serialize :mounts, Hash
serialize :runtime_constraints, Hash
serialize :command, Array
- has_many :container_requests, :foreign_key => :container_uuid, :class_name => 'ContainerRequest', :primary_key => :uuid
+ before_validation :fill_field_defaults
+ before_validation :set_timestamps
+ validates :command, :container_image, :output_path, :cwd, :presence => true
+ validate :validate_change
- before_create :set_state_before_save
+ has_many :container_requests, :foreign_key => :container_uuid, :class_name => 'ContainerRequest', :primary_key => :uuid
api_accessible :user, extend: :common do |t|
t.add :command
@@ -28,7 +30,6 @@ class Container < ArvadosModel
t.add :runtime_constraints
t.add :started_at
t.add :state
- t.add :uuid
end
# Supported states for a container
@@ -40,9 +41,87 @@ class Container < ArvadosModel
(Cancelled = 'Cancelled')
]
- def set_state_before_save
- self.state ||= Queued
+ def fill_field_defaults
+ if self.new_record?
+ self.state ||= Queued
+ self.environment ||= {}
+ self.runtime_constraints ||= {}
+ self.cwd ||= "."
+ end
+ end
+
+ protected
+
+ def permission_to_create
+ current_user.andand.is_admin
+ end
+
+ def permission_to_update
+ current_user.andand.is_admin
+ end
+
+ def check_permitted_updates permitted_fields
+ attribute_names.each do |field|
+ if not permitted_fields.include? field.to_sym and self.send((field.to_s + "_changed?").to_sym)
+ errors.add field, "Illegal update of field #{field}"
+ end
+ end
end
+ def set_timestamps
+ if self.state_changed? and self.state == Running
+ self.started_at ||= db_current_time
+ end
+
+ if self.state_changed? and [Complete, Cancelled].include? self.state
+ self.finished_at ||= db_current_time
+ end
+ end
+
+ def validate_change
+ permitted = [:modified_at, :modified_by_user_uuid, :modified_by_client_uuid]
+
+ if self.new_record?
+ permitted.push :owner_uuid, :command, :container_image, :cwd, :environment,
+ :mounts, :output_path, :priority, :runtime_constraints, :state
+ end
+
+ case self.state
+ when Queued
+ # permit priority change only.
+ if self.state_changed? and not self.state_was.nil?
+ errors.add :state, "Can only go to Queued from nil"
+ else
+ permitted.push :priority
+ end
+ when Running
+ if self.state_changed?
+ if self.state_was == Queued
+ permitted.push :state, :started_at
+ else
+ errors.add :state, "Can only go to Runinng from Queued"
+ end
+ else
+ permitted.push :progress
+ end
+ when Complete, Cancelled
+ if self.state_changed?
+ if self.state_was == Running
+ permitted.push :state, :finished_at, :output, :log
+ elsif self.state_was == Queued
+ permitted.push :state
+ else
+ errors.add :state, "Can only go to #{self.state} from Running"
+ end
+ end
+ else
+ errors.add :state, "Invalid state #{self.state}"
+ end
+
+ check_permitted_updates permitted
+ end
+
+ def validate_fields
+ end
end
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 9356a70..22b6bde 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -10,6 +10,9 @@ class ContainerRequest < ArvadosModel
serialize :command, Array
before_create :set_state_before_save
+ validate :validate_change_permitted
+ validate :validate_status
+ validate :validate_state_change
api_accessible :user, extend: :common do |t|
t.add :command
@@ -43,6 +46,76 @@ class ContainerRequest < ArvadosModel
self.state ||= Uncommitted
end
+ def validate_change_permitted
+ if self.changed?
+ ok = case self.state
+ when nil
+ true
+ when Uncommitted
+ true
+ when Committed
+ # only allow state and priority to change.
+ not (self.command_changed? or
+ self.container_count_max_changed? or
+ self.container_image_changed? or
+ self.container_uuid_changed? or
+ self.cwd_changed? or
+ self.description_changed? or
+ self.environment_changed? or
+ self.expires_at_changed? or
+ self.filters_changed? or
+ self.mounts_changed? or
+ self.name_changed? or
+ self.output_path_changed? or
+ self.properties_changed? or
+ self.requesting_container_uuid_changed? or
+ self.runtime_constraints_changed?)
+ when Final
+ false
+ else
+ false
+ end
+ if not ok
+ errors.add :state, "Invalid update of container request in #{self.state} state"
+ end
+ end
+ end
+
+ def validate_status
+ if self.state.in?(States)
+ true
+ else
+ errors.add :state, "#{state.inspect} must be one of: #{States.inspect}"
+ false
+ end
+ end
+
+ def validate_state_change
+ ok = true
+ if self.state_changed?
+ ok = case self.state_was
+ when nil
+ # Must go to Uncommitted
+ self.state == Uncommitted
+ when Uncommitted
+ # Must go to Committed
+ self.state == Committed
+ when Committed
+ # Must to go Final
+ self.state == Final
+ when Final
+ # Once in a final state, don't permit any more state changes
+ false
+ else
+ # Any other state transition is also invalid
+ false
+ end
+ if not ok
+ errors.add :state, "invalid change from #{self.state_was} to #{self.state}"
+ end
+ end
+ ok
+ end
end
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 3ba35c4..c635af3 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -1,7 +1,102 @@
require 'test_helper'
class ContainerTest < ActiveSupport::TestCase
- # test "the truth" do
- # assert true
- # end
+ def check_illegal_modify c
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.command = ["echo", "bar"]
+ c.save!
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.container_image = "img2"
+ c.save!
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.cwd = "/tmp2"
+ c.save!
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.environment = {"FOO" => "BAR"}
+ c.save!
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.mounts = {"FOO" => "BAR"}
+ c.save!
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.output_path = "/tmp3"
+ c.save!
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.runtime_constraints = {"FOO" => "BAR"}
+ c.save!
+ end
+
+ end
+
+ def check_bogus_states c
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.state = nil
+ c.save!
+ end
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.state = "Flubber"
+ c.save!
+ end
+ end
+
+ test "Container create" do
+ act_as_system_user do
+ c = Container.new
+ c.command = ["echo", "foo"]
+ c.container_image = "img"
+ c.cwd = "/tmp"
+ c.environment = {}
+ c.mounts = {}
+ c.output_path = "/tmp"
+ c.priority = 1
+ c.runtime_constraints = {}
+ c.save!
+
+ check_illegal_modify c
+ check_bogus_states c
+
+ c.reload
+ c.priority = 2
+ c.save!
+
+ c.reload
+ c.state = "Running"
+ c.save!
+
+ check_illegal_modify c
+ check_bogus_states c
+
+ assert_raises(ActiveRecord::RecordInvalid) do
+ c.reload
+ c.state = "Queued"
+ c.save!
+ end
+
+ c.reload
+ c.priority = 3
+ c.save!
+
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list