From 4eef0a298418a751b5941fd6f4bf32d91b817d54 Mon Sep 17 00:00:00 2001 From: Lucas Di Pentima Date: Wed, 7 Sep 2016 16:09:40 -0300 Subject: [PATCH] 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. --- services/api/app/models/container.rb | 25 +-- services/api/test/fixtures/containers.yml | 205 +++++++++++++++++++++- services/api/test/unit/container_test.rb | 54 ++++-- 3 files changed, 256 insertions(+), 28 deletions(-) diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index 435f5f4df8..2d3402f6ba 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 906a8a27e6..65323f5a9c 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 edc9db66e0..2f68246a60 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 -- 2.30.2