[ARVADOS] updated: 934694fb672f0768ec61701009a4eb2e149d6431
Git user
git at public.curoverse.com
Thu Sep 8 15:35:47 EDT 2016
Summary of changes:
.../arvados/v1/containers_controller.rb | 5 +--
services/api/app/models/arvados_model.rb | 6 +++
services/api/app/models/container.rb | 19 ++++++++--
services/api/test/fixtures/containers.yml | 1 +
.../arvados/v1/containers_controller_test.rb | 43 +++++++---------------
services/api/test/unit/container_test.rb | 21 +++++------
6 files changed, 48 insertions(+), 47 deletions(-)
via 934694fb672f0768ec61701009a4eb2e149d6431 (commit)
from cc13854bd4675d9f1de807c38dfada0315bf3291 (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 934694fb672f0768ec61701009a4eb2e149d6431
Author: radhika <radhika at curoverse.com>
Date: Thu Sep 8 15:33:05 2016 -0400
9898: add unlock method also on the container model.
diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb
index 3ed1e9b..fb748e9 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -21,13 +21,12 @@ class Arvados::V1::ContainersController < ApplicationController
end
def lock
- @object.lock or raise Exception.new("Error locking container")
+ @object.lock
show
end
def unlock
- reload_object_before_update
- @object.update_attributes! state: Container::Queued
+ @object.unlock
show
end
end
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 38bd5cf..f89e9f4 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -46,6 +46,12 @@ class ArvadosModel < ActiveRecord::Base
end
end
+ class InvalidStateTransitionError < StandardError
+ def http_status
+ 403
+ end
+ end
+
class UnauthorizedError < StandardError
def http_status
401
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index ae759de..ba169cc 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -77,11 +77,22 @@ class Container < ArvadosModel
end
def lock
+ if self.state == Locked
+ raise AlreadyLockedError
+ end
with_lock do
- if self.state == Queued
- self.state = Locked
- self.save!
- end
+ self.state = Locked
+ self.save!
+ end
+ end
+
+ def unlock
+ if self.state == Queued
+ raise InvalidStateTransitionError
+ end
+ with_lock do
+ self.state = Queued
+ self.save!
end
end
diff --git a/services/api/test/fixtures/containers.yml b/services/api/test/fixtures/containers.yml
index 2e3021d..060925b 100644
--- a/services/api/test/fixtures/containers.yml
+++ b/services/api/test/fixtures/containers.yml
@@ -17,6 +17,7 @@ queued:
running:
uuid: zzzzz-dz642-runningcontainr
owner_uuid: zzzzz-tpzed-000000000000000
+ state: Running
priority: 1
created_at: <%= 1.minute.ago.to_s(:db) %>
updated_at: <%= 1.minute.ago.to_s(:db) %>
diff --git a/services/api/test/functional/arvados/v1/containers_controller_test.rb b/services/api/test/functional/arvados/v1/containers_controller_test.rb
index 208c1cc..abaa612 100644
--- a/services/api/test/functional/arvados/v1/containers_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/containers_controller_test.rb
@@ -24,7 +24,7 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
test 'cannot get auth with wrong token' do
authorize_with :dispatch1
c = containers(:queued)
- assert c.update_attributes(state: Container::Locked), show_errors(c)
+ assert c.lock, show_errors(c)
authorize_with :system_user
get :auth, id: c.uuid
@@ -34,7 +34,7 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
test 'get auth' do
authorize_with :dispatch1
c = containers(:queued)
- assert c.update_attributes(state: Container::Locked), show_errors(c)
+ assert c.lock, show_errors(c)
get :auth, id: c.uuid
assert_response :success
assert_operator 32, :<, json_response['api_token'].length
@@ -44,7 +44,7 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
test 'no auth in container response' do
authorize_with :dispatch1
c = containers(:queued)
- assert c.update_attributes(state: Container::Locked), show_errors(c)
+ assert c.lock, show_errors(c)
get :show, id: c.uuid
assert_response :success
assert_nil json_response['auth']
@@ -91,33 +91,18 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
end
[
- [['Queued', :success], ['Locked', :success]],
- [['Locked', :success], ['Locked', 422]],
- [['Locked', :success], ['Queued', :success]],
- [['Locked', :success], ['Running', :success], ['Queued', 422]],
- ].each do |transitions|
- test "lock and unlock state transitions #{transitions}" do
+ [:queued, :lock, :success, 'Locked'],
+ [:queued, :unlock, 403, 'Queued'],
+ [:locked, :lock, 403, 'Locked'],
+ [:running, :lock, 422, 'Running'],
+ [:running, :unlock, 422, 'Running'],
+ ].each do |fixture, action, response, state|
+ test "state transitions from #{fixture } to #{action}" do
authorize_with :dispatch1
-
- container = create_new_container()
-
- transitions.each do |state, status|
- @test_counter = 0 # Reset executed action counter
- @controller = Arvados::V1::ContainersController.new
- authorize_with :dispatch1
-
- if state == 'Locked'
- post :lock, {id: container.uuid}
- elsif state == 'Queued'
- post :unlock, {id: container.uuid}
- else
- container.update_attributes!(state: state)
- end
- assert_response status
-
- container = Container.where(uuid: container['uuid']).first
- assert_equal state, container.state if status == :success
- end
+ uuid = containers(fixture).uuid
+ post action, {id: uuid}
+ assert_response response
+ assert_equal state, Container.where(uuid: uuid).first.state
end
end
end
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index c0b95f3..453586c 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -85,7 +85,7 @@ class ContainerTest < ActiveSupport::TestCase
check_illegal_updates c, [{state: Container::Running},
{state: Container::Complete}]
- c.update_attributes! state: Container::Locked
+ c.lock
c.update_attributes! state: Container::Running
check_illegal_modify c
@@ -103,7 +103,7 @@ class ContainerTest < ActiveSupport::TestCase
set_user_from_auth :dispatch1
assert_equal Container::Queued, c.state
- refute c.update_attributes(state: Container::Locked), "no priority"
+ assert_raise(ActiveRecord::RecordInvalid) {c.lock} # "no priority"
c.reload
assert cr.update_attributes priority: 1
@@ -116,10 +116,10 @@ class ContainerTest < ActiveSupport::TestCase
assert c.locked_by_uuid
assert c.auth_uuid
- refute c.lock, "already locked"
+ assert_raise(ArvadosModel::AlreadyLockedError) {c.lock}
c.reload
- assert c.update_attributes(state: Container::Queued), show_errors(c)
+ assert c.unlock, show_errors(c)
refute c.locked_by_uuid
refute c.auth_uuid
@@ -128,16 +128,16 @@ class ContainerTest < ActiveSupport::TestCase
refute c.locked_by_uuid
refute c.auth_uuid
- assert c.update_attributes(state: Container::Locked), show_errors(c)
+ assert c.lock, show_errors(c)
assert c.update_attributes(state: Container::Running), show_errors(c)
assert c.locked_by_uuid
assert c.auth_uuid
auth_uuid_was = c.auth_uuid
- refute c.update_attributes(state: Container::Locked), "already running"
+ assert_raise(ActiveRecord::RecordInvalid) {c.lock} # Running to Locked is not allowed
c.reload
- refute c.update_attributes(state: Container::Queued), "already running"
+ assert_raise(ActiveRecord::RecordInvalid) {c.unlock} # Running to Queued is not allowed
c.reload
assert c.update_attributes(state: Container::Complete), show_errors(c)
@@ -158,7 +158,7 @@ class ContainerTest < ActiveSupport::TestCase
test "Container locked cancel" do
c, _ = minimal_new
set_user_from_auth :dispatch1
- assert c.update_attributes(state: Container::Locked), show_errors(c)
+ assert c.lock, show_errors(c)
assert c.update_attributes(state: Container::Cancelled), show_errors(c)
check_no_change_from_cancelled c
end
@@ -166,8 +166,7 @@ class ContainerTest < ActiveSupport::TestCase
test "Container running cancel" do
c, _ = minimal_new
set_user_from_auth :dispatch1
- c.update_attributes! state: Container::Queued
- c.update_attributes! state: Container::Locked
+ c.lock
c.update_attributes! state: Container::Running
c.update_attributes! state: Container::Cancelled
check_no_change_from_cancelled c
@@ -189,7 +188,7 @@ class ContainerTest < ActiveSupport::TestCase
test "Container only set exit code on complete" do
c, _ = minimal_new
set_user_from_auth :dispatch1
- c.update_attributes! state: Container::Locked
+ c.lock
c.update_attributes! state: Container::Running
check_illegal_updates c, [{exit_code: 1},
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list