9623: Added test cases on container request to check for container reuse flow. Also...

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 797fd16ebc9fff8fa6fbd0d36becf39e706b7796..7083b3533ac931c8b0d559de0e13ea7724abadc3 100644 (file)
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -395,4 +395,31 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_not_empty Container.readable_by(users(:active)).where(uuid: containers(:running).uuid)
     assert_empty Container.readable_by(users(:spectator)).where(uuid: containers(:running).uuid)
   end
+
+  [
+    [{"var" => "value1"}, {"var" => "value1"}],
+    [{"var" => "value1"}, {"var" => "value2"}]
+  ].each do |env1, env2|
+    test "Container request #{(env1 == env2) ? 'does' : 'does not'} reuse container when committed" do
+      common_attrs = {cwd: "test",
+                      priority: 1,
+                      command: ["echo", "hello"],
+                      output_path: "test",
+                      runtime_constraints: {"vcpus" => 4,
+                                            "ram" => 12000000000},
+                      mounts: {"test" => {"kind" => "json"}}}
+      set_user_from_auth :active
+      cr1 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Committed,
+                                                    environment: env1}))
+      cr2 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Uncommitted,
+                                                    environment: env2}))
+      assert_nil cr2.container_uuid
+
+      # Update cr2 to commited state and check for container equality on both cases,
+      # when env1 and env2 are equal the same container should be assigned, and
+      # when env1 and env2 are different, cr2 container should be different.
+      cr2.update_attributes!({state: ContainerRequest::Committed})
+      assert_equal (env1 == env2), (cr1.container_uuid == cr2.container_uuid)
+    end
+  end
 end

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index fec3caa83471502684bc3c5fe1dfa8fa4b108cb7..6501f21b77207482c02be5daf1a12de5ef1cd83f 100644 (file)
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -142,7 +142,32 @@ class ContainerTest < ActiveSupport::TestCase
     assert_equal reused.uuid, c_recent.uuid
   end
 
-  test "find_reusable method should select running container most likely to finish sooner" do
+  test "find_reusable method should not select completed container when inconsistent outputs exist" do
+    set_user_from_auth :active
+    common_attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"var" => "complete"}})
+    completed_attrs = {
+      state: Container::Complete,
+      exit_code: 0,
+      log: 'test',
+    }
+
+    c_older, _ = minimal_new(common_attrs)
+    c_recent, _ = minimal_new(common_attrs)
+
+    set_user_from_auth :dispatch1
+    c_older.update_attributes!({state: Container::Locked})
+    c_older.update_attributes!({state: Container::Running})
+    c_older.update_attributes!(completed_attrs.merge({output: 'output 1'}))
+
+    c_recent.update_attributes!({state: Container::Locked})
+    c_recent.update_attributes!({state: Container::Running})
+    c_recent.update_attributes!(completed_attrs.merge({output: 'output 2'}))
+
+    reused = Container.find_reusable(common_attrs)
+    assert_nil reused
+  end
+
+  test "find_reusable method should select running container by start date" do
     set_user_from_auth :active
     common_attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"var" => "running"}})
     c_slower, _ = minimal_new(common_attrs)
@@ -160,9 +185,32 @@ class ContainerTest < ActiveSupport::TestCase
                                                 progress: 0.15})
     reused = Container.find_reusable(common_attrs)
     assert_not_nil reused
+    # Winner is the one that started first
     assert_equal reused.uuid, c_faster_started_first.uuid
   end
 
+  test "find_reusable method should select running container by progress" do
+    set_user_from_auth :active
+    common_attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"var" => "running2"}})
+    c_slower, _ = minimal_new(common_attrs)
+    c_faster_started_first, _ = minimal_new(common_attrs)
+    c_faster_started_second, _ = minimal_new(common_attrs)
+    set_user_from_auth :dispatch1
+    c_slower.update_attributes!({state: Container::Locked})
+    c_slower.update_attributes!({state: Container::Running,
+                                 progress: 0.1})
+    c_faster_started_first.update_attributes!({state: Container::Locked})
+    c_faster_started_first.update_attributes!({state: Container::Running,
+                                               progress: 0.15})
+    c_faster_started_second.update_attributes!({state: Container::Locked})
+    c_faster_started_second.update_attributes!({state: Container::Running,
+                                                progress: 0.2})
+    reused = Container.find_reusable(common_attrs)
+    assert_not_nil reused
+    # Winner is the one with most progress done
+    assert_equal reused.uuid, c_faster_started_second.uuid
+  end
+
   test "find_reusable method should select locked container most likely to start sooner" do
     set_user_from_auth :active
     common_attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"var" => "locked"}})
@@ -181,6 +229,26 @@ class ContainerTest < ActiveSupport::TestCase
     assert_equal reused.uuid, c_high_priority_older.uuid
   end
 
+  test "find_reusable method should select running over failed container" do
+    set_user_from_auth :active
+    common_attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"var" => "failed_vs_running"}})
+    c_failed, _ = minimal_new(common_attrs)
+    c_running, _ = minimal_new(common_attrs)
+    set_user_from_auth :dispatch1
+    c_failed.update_attributes!({state: Container::Locked})
+    c_failed.update_attributes!({state: Container::Running})
+    c_failed.update_attributes!({state: Container::Complete,
+                                 exit_code: 42,
+                                 log: "test",
+                                 output: "test"})
+    c_running.update_attributes!({state: Container::Locked})
+    c_running.update_attributes!({state: Container::Running,
+                                  progress: 0.15})
+    reused = Container.find_reusable(common_attrs)
+    assert_not_nil reused
+    assert_equal reused.uuid, c_running.uuid
+  end
+
   test "find_reusable method should select complete over running container" do
     set_user_from_auth :active
     common_attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"var" => "completed_vs_running"}})