[ARVADOS] created: 4eef0a298418a751b5941fd6f4bf32d91b817d54
Git user
git at public.curoverse.com
Wed Sep 7 15:12:28 EDT 2016
at 4eef0a298418a751b5941fd6f4bf32d91b817d54 (commit)
commit 4eef0a298418a751b5941fd6f4bf32d91b817d54
Author: Lucas Di Pentima <lucas at curoverse.com>
Date: Wed Sep 7 16:09:40 2016 -0300
9623: Check for reusable containers in Completed state that has existing output and log data. Added additional tests to check for correct container reuse preferences.
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 435f5f4..2d3402f 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -86,9 +86,11 @@ class Container < ArvadosModel
where('container_image = ?', attrs[:container_image]).
where('mounts = ?', self.deep_sort_hash(attrs[:mounts]).to_yaml).
where('runtime_constraints = ?', self.deep_sort_hash(attrs[:runtime_constraints]).to_yaml).
- where('state in (?)', ['Queued', 'Locked', 'Running', 'Complete']).
- reject {|c| c.state == 'Complete' and c.exit_code != 0}
-
+ where('state in (?)', [Container::Queued, Container::Locked,
+ Container::Running, Container::Complete]).
+ reject {|c| c.state == Container::Complete and (c.exit_code != 0 or
+ c.output.nil? or
+ c.log.nil?)}
if candidates.empty?
nil
elsif candidates.count == 1
@@ -96,20 +98,21 @@ class Container < ArvadosModel
else
# Multiple candidates found, search for the best one:
# The most recent completed container
- winner = candidates.select {|c| c.state == 'Complete'}.sort_by {|c| c.finished_at}.last
- winner if not winner.nil?
+ winner = candidates.select {|c| c.state == Container::Complete}.
+ sort_by {|c| c.finished_at}.last
+ return winner if not winner.nil?
# The running container that's most likely to finish sooner.
- winner = candidates.select {|c| c.state == 'Running'}.
+ winner = candidates.select {|c| c.state == Container::Running}.
sort {|a, b| [b.progress, a.started_at] <=> [a.progress, b.started_at]}.first
- winner if not winner.nil?
+ return winner if not winner.nil?
# The locked container that's most likely to start sooner.
- winner = candidates.select {|c| c.state == 'Locked'}.
+ winner = candidates.select {|c| c.state == Container::Locked}.
sort {|a, b| [b.priority, a.created_at] <=> [a.priority, b.created_at]}.first
- winner if not winner.nil?
+ return winner if not winner.nil?
# The queued container that's most likely to start sooner.
- winner = candidates.select {|c| c.state == 'Queued'}.
+ winner = candidates.select {|c| c.state == Container::Queued}.
sort {|a, b| [b.priority, a.created_at] <=> [a.priority, b.created_at]}.first
- winner if not winner.nil?
+ return winner if not winner.nil?
end
end
diff --git a/services/api/test/fixtures/containers.yml b/services/api/test/fixtures/containers.yml
index 906a8a2..65323f5 100644
--- a/services/api/test/fixtures/containers.yml
+++ b/services/api/test/fixtures/containers.yml
@@ -7,47 +7,81 @@ queued:
updated_at: 2016-01-11 11:11:11.111111111 Z
container_image: test
cwd: test
- output: test
output_path: test
command: ["echo", "hello"]
runtime_constraints:
ram: 12000000000
vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: queued
+
+queued_higher_priority:
+ uuid: zzzzz-dz642-queuedcontainerhigher
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ state: Queued
+ priority: 2
+ created_at: 2016-01-11 11:11:11.111111111 Z
+ updated_at: 2016-01-11 11:11:11.111111111 Z
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: queued
running:
uuid: zzzzz-dz642-runningcontainr
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
state: Running
priority: 1
+ progress: 10.0
created_at: <%= 1.minute.ago.to_s(:db) %>
updated_at: <%= 1.minute.ago.to_s(:db) %>
started_at: <%= 1.minute.ago.to_s(:db) %>
container_image: test
cwd: test
- output: test
output_path: test
command: ["echo", "hello"]
runtime_constraints:
ram: 12000000000
vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: running
auth_uuid: zzzzz-gj3su-077z32aux8dg2s1
-running-older:
+running_older:
uuid: zzzzz-dz642-runningcontain2
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
state: Running
priority: 1
+ progress: 15.0
created_at: <%= 2.minute.ago.to_s(:db) %>
updated_at: <%= 2.minute.ago.to_s(:db) %>
started_at: <%= 2.minute.ago.to_s(:db) %>
container_image: test
cwd: test
- output: test
output_path: test
command: ["echo", "hello"]
runtime_constraints:
ram: 12000000000
vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: running
locked:
uuid: zzzzz-dz642-lockedcontainer
@@ -58,12 +92,36 @@ locked:
updated_at: <%= 2.minute.ago.to_s(:db) %>
container_image: test
cwd: test
- output: test
output_path: test
command: ["echo", "hello"]
runtime_constraints:
ram: 12000000000
vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: locked
+
+locked_higher_priority:
+ uuid: zzzzz-dz642-lockedcontainerhigher
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ state: Locked
+ priority: 3
+ created_at: <%= 2.minute.ago.to_s(:db) %>
+ updated_at: <%= 2.minute.ago.to_s(:db) %>
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: locked
completed:
uuid: zzzzz-dz642-compltcontainer
@@ -85,7 +143,7 @@ completed:
test:
kind: json
environment:
- var: test
+ var: completed
runtime_constraints:
ram: 12000000000
vcpus: 4
@@ -103,8 +161,14 @@ completed_older:
container_image: test
cwd: test
output: test
+ log: test
output_path: test
command: ["echo", "hello"]
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: completed
runtime_constraints:
ram: 12000000000
vcpus: 4
@@ -155,8 +219,137 @@ failed_container:
container_image: test
cwd: test
output: test
+ log: test
output_path: test
command: ["echo", "hello"]
runtime_constraints:
ram: 12000000000
vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: failed
+
+completed_vs_running_winner:
+ uuid: zzzzz-dz642-cvrwinner
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ exit_code: 0
+ output: test
+ log: test
+ state: Complete
+ priority: 1
+ created_at: 2016-01-11 11:11:11.111111111 Z
+ updated_at: 2016-01-11 11:11:11.111111111 Z
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: completed_vs_running
+
+completed_vs_running_loser:
+ uuid: zzzzz-dz642-cvrloser
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ state: Running
+ priority: 1
+ created_at: 2016-01-11 11:11:11.111111111 Z
+ updated_at: 2016-01-11 11:11:11.111111111 Z
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: completed_vs_running
+
+running_vs_locked_winner:
+ uuid: zzzzz-dz642-rvlwinner
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ state: Running
+ priority: 1
+ created_at: 2016-01-11 11:11:11.111111111 Z
+ updated_at: 2016-01-11 11:11:11.111111111 Z
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: running_vs_locked
+
+running_vs_locked_loser:
+ uuid: zzzzz-dz642-rvlloser
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ state: Locked
+ priority: 1
+ created_at: 2016-01-11 11:11:11.111111111 Z
+ updated_at: 2016-01-11 11:11:11.111111111 Z
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: running_vs_locked
+
+locked_vs_queued_winner:
+ uuid: zzzzz-dz642-lvqwinner
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ state: Locked
+ priority: 1
+ created_at: 2016-01-11 11:11:11.111111111 Z
+ updated_at: 2016-01-11 11:11:11.111111111 Z
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: locked_vs_queued
+
+locked_vs_queued_loser:
+ uuid: zzzzz-dz642-lvqloser
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ state: Queued
+ priority: 1
+ created_at: 2016-01-11 11:11:11.111111111 Z
+ updated_at: 2016-01-11 11:11:11.111111111 Z
+ container_image: test
+ cwd: test
+ output_path: test
+ command: ["echo", "hello"]
+ runtime_constraints:
+ ram: 12000000000
+ vcpus: 4
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: locked_vs_queued
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index edc9db6..2f68246 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -78,7 +78,7 @@ class ContainerTest < ActiveSupport::TestCase
end
end
- test "Container serialized hash attributes sorted" do
+ test "Container serialized hash attributes sorted before save" do
env = {"C" => 3, "B" => 2, "A" => 1}
m = {"F" => 3, "E" => 2, "D" => 1}
rc = {"vcpus" => 1, "ram" => 1}
@@ -94,17 +94,49 @@ class ContainerTest < ActiveSupport::TestCase
assert_equal Container.deep_sort_hash(a).to_json, Container.deep_sort_hash(b).to_json
end
- test "Container find reusable method" do
+ [
+ ["completed", :completed_older],
+ ["running", :running_older],
+ ["locked", :locked_higher_priority],
+ ["queued", :queued_higher_priority],
+ ["completed_vs_running", :completed_vs_running_winner],
+ ["running_vs_locked", :running_vs_locked_winner],
+ ["locked_vs_queued", :locked_vs_queued_winner]
+ ].each do |state, c_to_be_selected|
+ test "find reusable method for #{state} container" do
+ set_user_from_auth :active
+ c = Container.find_reusable(container_image: "test",
+ cwd: "test",
+ command: ["echo", "hello"],
+ output_path: "test",
+ runtime_constraints: {"vcpus" => 4, "ram" => 12000000000},
+ mounts: {"test" => {"kind" => "json"}},
+ environment: {"var" => state})
+ assert_not_nil c
+ assert_equal c.uuid, containers(c_to_be_selected).uuid
+ end
+ end
+
+ test "Container find reusable method should not select failed" do
set_user_from_auth :active
- c = Container.find_reusable(container_image: "test",
- cwd: "test",
- command: ["echo", "hello"],
- output_path: "test",
- runtime_constraints: {"vcpus" => 4, "ram" => 12000000000},
- mounts: {"test" => {"kind" => "json"}},
- environment: {"var" => "test"})
- assert_not_nil c
- assert_equal c.uuid, containers(:completed).uuid
+ attrs = {container_image: "test",
+ cwd: "test",
+ command: ["echo", "hello"],
+ output_path: "test",
+ runtime_constraints: {"vcpus" => 4, "ram" => 12000000000},
+ mounts: {"test" => {"kind" => "json"}},
+ environment: {"var" => "failed"}}
+ cf = containers(:failed_container)
+ assert_equal cf.container_image, attrs[:container_image]
+ assert_equal cf.cwd, attrs[:cwd]
+ assert_equal cf.command, attrs[:command]
+ assert_equal cf.output_path, attrs[:output_path]
+ assert_equal cf.runtime_constraints, attrs[:runtime_constraints]
+ assert_equal cf.mounts, attrs[:mounts]
+ assert_equal cf.environment, attrs[:environment]
+ assert cf.exit_code != 0
+ c = Container.find_reusable(attrs)
+ assert_nil c
end
test "Container running" do
commit 8100ee8f50d0c8b0340640db10745e44c0f4571b
Author: Lucas Di Pentima <lucas at curoverse.com>
Date: Tue Sep 6 21:59:29 2016 -0300
9623: Added method to find a reusable container, used by ContainerRequest#resolve. Added some tests for this new method.
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index f8e27e7..435f5f4 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -77,6 +77,42 @@ class Container < ArvadosModel
end
end
+ def self.find_reusable(attrs)
+ candidates = Container.
+ where('command = ?', attrs[:command].to_yaml).
+ where('cwd = ?', attrs[:cwd]).
+ where('environment = ?', self.deep_sort_hash(attrs[:environment]).to_yaml).
+ where('output_path = ?', attrs[:output_path]).
+ where('container_image = ?', attrs[:container_image]).
+ where('mounts = ?', self.deep_sort_hash(attrs[:mounts]).to_yaml).
+ where('runtime_constraints = ?', self.deep_sort_hash(attrs[:runtime_constraints]).to_yaml).
+ where('state in (?)', ['Queued', 'Locked', 'Running', 'Complete']).
+ reject {|c| c.state == 'Complete' and c.exit_code != 0}
+
+ if candidates.empty?
+ nil
+ elsif candidates.count == 1
+ candidates.first
+ else
+ # Multiple candidates found, search for the best one:
+ # The most recent completed container
+ winner = candidates.select {|c| c.state == 'Complete'}.sort_by {|c| c.finished_at}.last
+ winner if not winner.nil?
+ # The running container that's most likely to finish sooner.
+ winner = candidates.select {|c| c.state == 'Running'}.
+ sort {|a, b| [b.progress, a.started_at] <=> [a.progress, b.started_at]}.first
+ winner if not winner.nil?
+ # The locked container that's most likely to start sooner.
+ winner = candidates.select {|c| c.state == 'Locked'}.
+ sort {|a, b| [b.priority, a.created_at] <=> [a.priority, b.created_at]}.first
+ winner if not winner.nil?
+ # The queued container that's most likely to start sooner.
+ winner = candidates.select {|c| c.state == 'Queued'}.
+ sort {|a, b| [b.priority, a.created_at] <=> [a.priority, b.created_at]}.first
+ winner if not winner.nil?
+ end
+ end
+
protected
def self.deep_sort_hash(x)
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index a56c341..47165a8 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -87,13 +87,19 @@ class ContainerRequest < ArvadosModel
c_runtime_constraints = runtime_constraints_for_container
c_container_image = container_image_for_container
c = act_as_system_user do
- Container.create!(command: self.command,
- cwd: self.cwd,
- environment: self.environment,
- output_path: self.output_path,
- container_image: c_container_image,
- mounts: c_mounts,
- runtime_constraints: c_runtime_constraints)
+ c_attrs = {command: self.command,
+ cwd: self.cwd,
+ environment: self.environment,
+ output_path: self.output_path,
+ container_image: c_container_image,
+ mounts: c_mounts,
+ runtime_constraints: c_runtime_constraints}
+ reusable = Container.find_reusable(c_attrs)
+ if not reusable.nil?
+ reusable
+ else
+ Container.create!(c_attrs)
+ end
end
self.container_uuid = c.uuid
end
diff --git a/services/api/test/fixtures/containers.yml b/services/api/test/fixtures/containers.yml
index 049cd3c..906a8a2 100644
--- a/services/api/test/fixtures/containers.yml
+++ b/services/api/test/fixtures/containers.yml
@@ -81,6 +81,11 @@ completed:
output: zzzzz-4zz18-znfnqtbbv4spc3w
output_path: test
command: ["echo", "hello"]
+ mounts:
+ test:
+ kind: json
+ environment:
+ var: test
runtime_constraints:
ram: 12000000000
vcpus: 4
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 7b2a644..edc9db6 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -94,6 +94,19 @@ class ContainerTest < ActiveSupport::TestCase
assert_equal Container.deep_sort_hash(a).to_json, Container.deep_sort_hash(b).to_json
end
+ test "Container find reusable method" do
+ set_user_from_auth :active
+ c = Container.find_reusable(container_image: "test",
+ cwd: "test",
+ command: ["echo", "hello"],
+ output_path: "test",
+ runtime_constraints: {"vcpus" => 4, "ram" => 12000000000},
+ mounts: {"test" => {"kind" => "json"}},
+ environment: {"var" => "test"})
+ assert_not_nil c
+ assert_equal c.uuid, containers(:completed).uuid
+ end
+
test "Container running" do
c, _ = minimal_new priority: 1
commit 40196e450ef489a31eeabf2c11a3969094e185b2
Author: Lucas Di Pentima <lucas at curoverse.com>
Date: Mon Sep 5 17:10:46 2016 -0300
9623: Sort Container serialized hashed attributes for efficient comparison. Copied Job.deep_sort_hash method into Container assuming Job will be deprecated in the future.
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 4c77008..f8e27e7 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -18,6 +18,7 @@ class Container < ArvadosModel
validate :validate_change
validate :validate_lock
after_validation :assign_auth
+ before_save :sort_serialized_attrs
after_save :handle_completed
has_many :container_requests, :foreign_key => :container_uuid, :class_name => 'ContainerRequest', :primary_key => :uuid
@@ -78,6 +79,18 @@ class Container < ArvadosModel
protected
+ def self.deep_sort_hash(x)
+ if x.is_a? Hash
+ x.sort.collect do |k, v|
+ [k, deep_sort_hash(v)]
+ end.to_h
+ elsif x.is_a? Array
+ x.collect { |v| deep_sort_hash(v) }
+ else
+ x
+ end
+ end
+
def fill_field_defaults
self.state ||= Queued
self.environment ||= {}
@@ -200,6 +213,12 @@ class Container < ArvadosModel
api_client_id: 0)
end
+ def sort_serialized_attrs
+ self.environment = self.class.deep_sort_hash(self.environment)
+ self.mounts = self.class.deep_sort_hash(self.mounts)
+ self.runtime_constraints = self.class.deep_sort_hash(self.runtime_constraints)
+ end
+
def handle_completed
# This container is finished so finalize any associated container requests
# that are associated with this container.
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 24186e8..7b2a644 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -78,6 +78,22 @@ class ContainerTest < ActiveSupport::TestCase
end
end
+ test "Container serialized hash attributes sorted" do
+ env = {"C" => 3, "B" => 2, "A" => 1}
+ m = {"F" => 3, "E" => 2, "D" => 1}
+ rc = {"vcpus" => 1, "ram" => 1}
+ c, _ = minimal_new(environment: env, mounts: m, runtime_constraints: rc)
+ assert_equal c.environment.to_json, Container.deep_sort_hash(env).to_json
+ assert_equal c.mounts.to_json, Container.deep_sort_hash(m).to_json
+ assert_equal c.runtime_constraints.to_json, Container.deep_sort_hash(rc).to_json
+ end
+
+ test 'deep_sort_hash on array of hashes' do
+ a = {'z' => [[{'a' => 'a', 'b' => 'b'}]]}
+ b = {'z' => [[{'b' => 'b', 'a' => 'a'}]]}
+ assert_equal Container.deep_sort_hash(a).to_json, Container.deep_sort_hash(b).to_json
+ end
+
test "Container running" do
c, _ = minimal_new priority: 1
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list