From: Lucas Di Pentima <lucas@curoverse.com>
Date: Wed, 7 Sep 2016 00:59:29 +0000 (-0300)
Subject: 9623: Added method to find a reusable container, used by ContainerRequest#resolve... 
X-Git-Tag: 1.1.0~731^2~15
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/8100ee8f50d0c8b0340640db10745e44c0f4571b

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 f8e27e7bbc..435f5f4df8 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 a56c34184d..47165a8598 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 049cd3c6db..906a8a27e6 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 7b2a64471b..edc9db66e0 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