[ARVADOS] created: 1.1.4-495-g0e9452887
Git user
git at public.curoverse.com
Fri Jun 22 09:56:40 EDT 2018
at 0e94528878341faa941247eb3d990230c941868c (commit)
commit 0e94528878341faa941247eb3d990230c941868c
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Tue Jun 19 19:47:21 2018 -0300
13649: Fixes default preemptible parameter when submitting committed child CRs
Two problems fixed:
1) When submitting a committed child CR, its preemptible parameter is set.
2) Resolve a container for said child CR after the preemptible parameter setting.
Added tests for committed child CR submissions.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 799aa430f..a0ebdbab0 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -28,8 +28,8 @@ class ContainerRequest < ArvadosModel
before_validation :fill_field_defaults, :if => :new_record?
before_validation :validate_runtime_constraints
- before_validation :set_container
before_validation :set_default_preemptible_scheduling_parameter
+ 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 }
@@ -199,11 +199,11 @@ class ContainerRequest < ArvadosModel
end
def set_default_preemptible_scheduling_parameter
+ c = get_requesting_container()
if self.state == Committed
# If preemptible instances (eg: AWS Spot Instances) are allowed,
# ask them on child containers by default.
- if Rails.configuration.preemptible_instances and
- !self.requesting_container_uuid.nil? and
+ if Rails.configuration.preemptible_instances and !c.nil? and
self.scheduling_parameters['preemptible'].nil?
self.scheduling_parameters['preemptible'] = true
end
@@ -313,10 +313,18 @@ class ContainerRequest < ArvadosModel
end
def set_requesting_container_uuid
- return if !current_api_client_authorization
- if (c = Container.where('auth_uuid=?', current_api_client_authorization.uuid).select([:uuid, :priority]).first)
+ c = get_requesting_container()
+ if !c.nil?
self.requesting_container_uuid = c.uuid
self.priority = c.priority>0 ? 1 : 0
end
end
+
+ def get_requesting_container
+ return self.requesting_container_uuid if !self.requesting_container_uuid.nil?
+ return if !current_api_client_authorization
+ if (c = Container.where('auth_uuid=?', current_api_client_authorization.uuid).select([:uuid, :priority]).first)
+ return c
+ end
+ end
end
diff --git a/services/api/lib/whitelist_update.rb b/services/api/lib/whitelist_update.rb
index f25d42381..17aed4b48 100644
--- a/services/api/lib/whitelist_update.rb
+++ b/services/api/lib/whitelist_update.rb
@@ -6,7 +6,7 @@ module WhitelistUpdate
def check_update_whitelist permitted_fields
attribute_names.each do |field|
if !permitted_fields.include?(field.to_sym) && really_changed(field)
- errors.add field, "cannot be modified in this state (#{send(field+"_was").inspect}, #{send(field).inspect})"
+ errors.add field, "cannot be modified in state '#{self.state}' (#{send(field+"_was").inspect}, #{send(field).inspect})"
end
end
end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 8071e05ce..fc872e1d0 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -849,6 +849,25 @@ class ContainerRequestTest < ActiveSupport::TestCase
end
end
+ test "submit a Committed child CR with preemptible_instances active" do
+ attrs = {cwd: "test",
+ priority: 1,
+ state: ContainerRequest::Committed,
+ command: ["echo", "hello"],
+ output_path: "test",
+ mounts: {"test" => {"kind" => "json"}}}
+
+ Rails.configuration.preemptible_instances = true
+ set_user_from_auth :active
+
+ cr = with_container_auth(Container.find_by_uuid 'zzzzz-dz642-runningcontainr') do
+ create_minimal_req!(attrs)
+ end
+
+ assert_not_nil cr.requesting_container_uuid
+ assert_equal true, cr.scheduling_parameters['preemptible']
+ end
+
[
[{"partitions" => ["fastcpu","vfastcpu", 100]}, ContainerRequest::Committed, ActiveRecord::RecordInvalid],
[{"partitions" => ["fastcpu","vfastcpu", 100]}, ContainerRequest::Uncommitted],
@@ -881,6 +900,25 @@ class ContainerRequestTest < ActiveSupport::TestCase
end
end
+ test "Having preemptible_instances=true create a committed child container request and verify the scheduling parameter of its container" do
+ common_attrs = {cwd: "test",
+ priority: 1,
+ command: ["echo", "hello"],
+ output_path: "test",
+ state: ContainerRequest::Committed,
+ mounts: {"test" => {"kind" => "json"}}}
+ set_user_from_auth :active
+ Rails.configuration.preemptible_instances = true
+
+ cr = with_container_auth(Container.find_by_uuid 'zzzzz-dz642-runningcontainr') do
+ create_minimal_req!(common_attrs)
+ end
+ assert_equal true, cr.scheduling_parameters["preemptible"]
+
+ c = Container.find_by_uuid(cr.container_uuid)
+ assert_equal true, c.scheduling_parameters["preemptible"]
+ end
+
[['Committed', true, {name: "foobar", priority: 123}],
['Committed', false, {container_count: 2}],
['Committed', false, {container_count: 0}],
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list