17778: Merge branch 'master' into 17778-doc-update
[arvados.git] / services / api / test / unit / container_request_test.rb
index 6f0233306925d8cb44653c41e6b3ac6b1a69c96d..2d5c73518191056a8b72909bc8d235a9d2f2ed39 100644 (file)
@@ -153,7 +153,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
     cr.reload
 
-    assert_equal({"vcpus" => 2, "ram" => 30}, cr.runtime_constraints)
+    assert ({"vcpus" => 2, "ram" => 30}.to_a - cr.runtime_constraints.to_a).empty?
 
     assert_not_nil cr.container_uuid
     c = Container.find_by_uuid cr.container_uuid
@@ -164,7 +164,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal({}, c.environment)
     assert_equal({"/out" => {"kind"=>"tmp", "capacity"=>1000000}}, c.mounts)
     assert_equal "/out", c.output_path
-    assert_equal({"keep_cache_ram"=>268435456, "vcpus" => 2, "ram" => 30}, c.runtime_constraints)
+    assert ({"keep_cache_ram"=>268435456, "vcpus" => 2, "ram" => 30}.to_a - c.runtime_constraints.to_a).empty?
     assert_operator 0, :<, c.priority
 
     assert_raises(ActiveRecord::RecordInvalid) do
@@ -252,6 +252,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     output = Collection.find_by_uuid cr.output_uuid
     assert_equal output_pdh, output.portable_data_hash
     assert_equal output.owner_uuid, project.uuid, "Container output should be copied to #{project.uuid}"
+    assert_not_nil output.modified_at
 
     log = Collection.find_by_uuid cr.log_uuid
     assert_equal log.manifest_text, ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar
@@ -260,6 +261,67 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal log.owner_uuid, project.uuid, "Container log should be copied to #{project.uuid}"
   end
 
+  # This tests bug report #16144
+  test "Request is finalized when its container is completed even when log & output don't exist" do
+    set_user_from_auth :active
+    project = groups(:private)
+    cr = create_minimal_req!(owner_uuid: project.uuid,
+                             priority: 1,
+                             state: "Committed")
+    assert_equal users(:active).uuid, cr.modified_by_user_uuid
+
+    output_pdh = '1f4b0bc7583c2a7f9102c395f4ffc5e3+45'
+    log_pdh = 'fa7aeb5140e2848d39b416daeef4ffc5+45'
+
+    c = act_as_system_user do
+      c = Container.find_by_uuid(cr.container_uuid)
+      c.update_attributes!(state: Container::Locked)
+      c.update_attributes!(state: Container::Running,
+                           output: output_pdh,
+                           log: log_pdh)
+      c
+    end
+
+    cr.reload
+    assert_equal "Committed", cr.state
+
+    act_as_system_user do
+      Collection.where(portable_data_hash: output_pdh).delete_all
+      Collection.where(portable_data_hash: log_pdh).delete_all
+      c.update_attributes!(state: Container::Complete)
+    end
+
+    cr.reload
+    assert_equal "Final", cr.state
+  end
+
+  # This tests bug report #16144
+  test "Can destroy CR even if its container doesn't exist" do
+    set_user_from_auth :active
+    project = groups(:private)
+    cr = create_minimal_req!(owner_uuid: project.uuid,
+                             priority: 1,
+                             state: "Committed")
+    assert_equal users(:active).uuid, cr.modified_by_user_uuid
+
+    c = act_as_system_user do
+      c = Container.find_by_uuid(cr.container_uuid)
+      c.update_attributes!(state: Container::Locked)
+      c.update_attributes!(state: Container::Running)
+      c
+    end
+
+    cr.reload
+    assert_equal "Committed", cr.state
+
+    cr_uuid = cr.uuid
+    act_as_system_user do
+      Container.find_by_uuid(cr.container_uuid).destroy
+      cr.destroy
+    end
+    assert_nil ContainerRequest.find_by_uuid(cr_uuid)
+  end
+
   test "Container makes container request, then is cancelled" do
     set_user_from_auth :active
     cr = create_minimal_req!(priority: 5, state: "Committed", container_count_max: 1)
@@ -514,7 +576,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
   test "Container.resolve_container_image(pdh)" do
     set_user_from_auth :active
     [[:docker_image, 'v1'], [:docker_image_1_12, 'v2']].each do |coll, ver|
-      Rails.configuration.Containers["SupportedDockerImageFormats"] = [ver]
+      Rails.configuration.Containers.SupportedDockerImageFormats = ConfigLoader.to_OrderedOptions({ver=>{}})
       pdh = collections(coll).portable_data_hash
       resolved = Container.resolve_container_image(pdh)
       assert_equal resolved, pdh
@@ -535,12 +597,12 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
   test "allow unrecognized container when there are remote_hosts" do
     set_user_from_auth :active
-    Rails.configuration.RemoteClusters = {"foooo" => {"Host" => "bar.com"} }
+    Rails.configuration.RemoteClusters = Rails.configuration.RemoteClusters.merge({foooo: ActiveSupport::InheritableOptions.new({Host: "bar.com"})})
     Container.resolve_container_image('acbd18db4cc2f85cedef654fccc4a4d8+3')
   end
 
   test "migrated docker image" do
-    Rails.configuration.Containers["SupportedDockerImageFormats"] = ['v2']
+    Rails.configuration.Containers.SupportedDockerImageFormats = ConfigLoader.to_OrderedOptions({'v2'=>{}})
     add_docker19_migration_link
 
     # Test that it returns only v2 images even though request is for v1 image.
@@ -558,7 +620,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
   end
 
   test "use unmigrated docker image" do
-    Rails.configuration.Containers["SupportedDockerImageFormats"] = ['v1']
+    Rails.configuration.Containers.SupportedDockerImageFormats = ConfigLoader.to_OrderedOptions({'v1'=>{}})
     add_docker19_migration_link
 
     # Test that it returns only supported v1 images even though there is a
@@ -577,7 +639,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
   end
 
   test "incompatible docker image v1" do
-    Rails.configuration.Containers["SupportedDockerImageFormats"] = ['v1']
+    Rails.configuration.Containers.SupportedDockerImageFormats = ConfigLoader.to_OrderedOptions({'v1'=>{}})
     add_docker19_migration_link
 
     # Don't return unsupported v2 image even if we ask for it directly.
@@ -590,7 +652,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
   end
 
   test "incompatible docker image v2" do
-    Rails.configuration.Containers["SupportedDockerImageFormats"] = ['v2']
+    Rails.configuration.Containers.SupportedDockerImageFormats = ConfigLoader.to_OrderedOptions({'v2'=>{}})
     # No migration link, don't return unsupported v1 image,
 
     set_user_from_auth :active
@@ -629,6 +691,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
       set_user_from_auth :active
       cr1 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Committed,
                                                     environment: env1}))
+      run_container(cr1)
+      cr1.reload
       if use_existing.nil?
         # Testing with use_existing default value
         cr2 = create_minimal_req!(common_attrs.merge({state: ContainerRequest::Uncommitted,
@@ -836,7 +900,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_not_nil(trash)
     assert_not_nil(delete)
     assert_in_delta(trash, now + 1.second, 10)
-    assert_in_delta(delete, now + Rails.configuration.Collections["BlobSigningTTL"].second, 10)
+    assert_in_delta(delete, now + Rails.configuration.Collections.BlobSigningTTL, 10)
   end
 
   def check_output_ttl_1y(now, trash, delete)
@@ -849,13 +913,18 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
   def run_container(cr)
     act_as_system_user do
+      logc = Collection.new(owner_uuid: system_user_uuid,
+                            manifest_text: ". ef772b2f28e2c8ca84de45466ed19ee9+7815 0:0:arv-mount.txt\n")
+      logc.save!
+
       c = Container.find_by_uuid(cr.container_uuid)
       c.update_attributes!(state: Container::Locked)
       c.update_attributes!(state: Container::Running)
       c.update_attributes!(state: Container::Complete,
                            exit_code: 0,
                            output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45',
-                           log: 'fa7aeb5140e2848d39b416daeef4ffc5+45')
+                           log: logc.portable_data_hash)
+      logc.destroy
       c
     end
   end
@@ -884,7 +953,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     [false, ActiveRecord::RecordInvalid],
     [true, nil],
   ].each do |preemptible_conf, expected|
-    test "having Rails.configuration.Containers['UsePreemptibleInstances']=#{preemptible_conf}, create preemptible container request and verify #{expected}" do
+    test "having Rails.configuration.Containers.UsePreemptibleInstances=#{preemptible_conf}, create preemptible container request and verify #{expected}" do
       sp = {"preemptible" => true}
       common_attrs = {cwd: "test",
                       priority: 1,
@@ -892,7 +961,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
                       output_path: "test",
                       scheduling_parameters: sp,
                       mounts: {"test" => {"kind" => "json"}}}
-      Rails.configuration.Containers["UsePreemptibleInstances"] = preemptible_conf
+      Rails.configuration.Containers.UsePreemptibleInstances = preemptible_conf
       set_user_from_auth :active
 
       cr = create_minimal_req!(common_attrs)
@@ -904,56 +973,25 @@ class ContainerRequestTest < ActiveSupport::TestCase
         end
       else
         cr.save!
-        assert_equal sp, cr.scheduling_parameters
-      end
-    end
-  end
-
-  [
-    'zzzzz-dz642-runningcontainr',
-    nil,
-  ].each do |requesting_c|
-    test "having preemptible instances active on the API server, a committed #{requesting_c.nil? ? 'non-':''}child CR should not ask for preemptible instance if parameter already set to false" do
-      common_attrs = {cwd: "test",
-                      priority: 1,
-                      command: ["echo", "hello"],
-                      output_path: "test",
-                      scheduling_parameters: {"preemptible" => false},
-                      mounts: {"test" => {"kind" => "json"}}}
-
-      Rails.configuration.Containers["UsePreemptibleInstances"] = true
-      set_user_from_auth :active
-
-      if requesting_c
-        cr = with_container_auth(Container.find_by_uuid requesting_c) do
-          create_minimal_req!(common_attrs)
-        end
-        assert_not_nil cr.requesting_container_uuid
-      else
-        cr = create_minimal_req!(common_attrs)
+        assert (sp.to_a - cr.scheduling_parameters.to_a).empty?
       end
-
-      cr.state = ContainerRequest::Committed
-      cr.save!
-
-      assert_equal false, cr.scheduling_parameters['preemptible']
     end
   end
 
   [
     [true, 'zzzzz-dz642-runningcontainr', true],
-    [true, nil, nil],
-    [false, 'zzzzz-dz642-runningcontainr', nil],
-    [false, nil, nil],
+    [true, nil, false],
+    [false, 'zzzzz-dz642-runningcontainr', false],
+    [false, nil, false],
   ].each do |preemptible_conf, requesting_c, schedule_preemptible|
-    test "having Rails.configuration.Containers['UsePreemptibleInstances']=#{preemptible_conf}, #{requesting_c.nil? ? 'non-':''}child CR should #{schedule_preemptible ? '':'not'} ask for preemptible instance by default" do
+    test "having Rails.configuration.Containers.UsePreemptibleInstances=#{preemptible_conf}, #{requesting_c.nil? ? 'non-':''}child CR should #{schedule_preemptible ? '':'not'} ask for preemptible instance by default" do
       common_attrs = {cwd: "test",
                       priority: 1,
                       command: ["echo", "hello"],
                       output_path: "test",
                       mounts: {"test" => {"kind" => "json"}}}
 
-      Rails.configuration.Containers["UsePreemptibleInstances"] = preemptible_conf
+      Rails.configuration.Containers.UsePreemptibleInstances = preemptible_conf
       set_user_from_auth :active
 
       if requesting_c
@@ -999,11 +1037,11 @@ class ContainerRequestTest < ActiveSupport::TestCase
         end
       else
         cr = create_minimal_req!(common_attrs.merge({state: state}))
-        assert_equal sp, cr.scheduling_parameters
+        assert (sp.to_a - cr.scheduling_parameters.to_a).empty?
 
         if state == ContainerRequest::Committed
           c = Container.find_by_uuid(cr.container_uuid)
-          assert_equal sp, c.scheduling_parameters
+          assert (sp.to_a - c.scheduling_parameters.to_a).empty?
         end
       end
     end
@@ -1017,7 +1055,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
                     state: ContainerRequest::Committed,
                     mounts: {"test" => {"kind" => "json"}}}
     set_user_from_auth :active
-    Rails.configuration.Containers["UsePreemptibleInstances"] = true
+    Rails.configuration.Containers.UsePreemptibleInstances = true
 
     cr = with_container_auth(Container.find_by_uuid 'zzzzz-dz642-runningcontainr') do
       create_minimal_req!(common_attrs)
@@ -1033,6 +1071,31 @@ class ContainerRequestTest < ActiveSupport::TestCase
    ['Committed', false, {container_count: 2}],
    ['Committed', false, {container_count: 0}],
    ['Committed', false, {container_count: nil}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}}}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp"}}}],
+   # Addition of default values for mounts / runtime_constraints /
+   # scheduling_parameters, as happens in a round-trip through
+   # controller, does not have any real effect and should be
+   # accepted/ignored rather than causing an error when the CR state
+   # dictates those attributes are not allowed to change.
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "exclude_from_output": false}}}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "repository_name": ""}}}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "content": nil}}}],
+   ['Committed', false, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "content": {}}}}],
+   ['Committed', false, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "repository_name": "foo"}}}],
+   ['Committed', false, {priority: 0, mounts: {"/out" => {"kind" => "tmp", "capacity" => 1234567}}}],
+   ['Committed', false, {priority: 0, mounts: {}}],
+   ['Committed', true, {priority: 0, runtime_constraints: {"vcpus" => 1, "ram" => 2}}],
+   ['Committed', true, {priority: 0, runtime_constraints: {"vcpus" => 1, "ram" => 2, "keep_cache_ram" => 0}}],
+   ['Committed', true, {priority: 0, runtime_constraints: {"vcpus" => 1, "ram" => 2, "API" => false}}],
+   ['Committed', false, {priority: 0, runtime_constraints: {"vcpus" => 1, "ram" => 2, "keep_cache_ram" => 1}}],
+   ['Committed', false, {priority: 0, runtime_constraints: {"vcpus" => 1, "ram" => 2, "API" => true}}],
+   ['Committed', true, {priority: 0, scheduling_parameters: {"preemptible" => false}}],
+   ['Committed', true, {priority: 0, scheduling_parameters: {"partitions" => []}}],
+   ['Committed', true, {priority: 0, scheduling_parameters: {"max_run_time" => 0}}],
+   ['Committed', false, {priority: 0, scheduling_parameters: {"preemptible" => true}}],
+   ['Committed', false, {priority: 0, scheduling_parameters: {"partitions" => ["foo"]}}],
+   ['Committed', false, {priority: 0, scheduling_parameters: {"max_run_time" => 1}}],
    ['Final', false, {state: ContainerRequest::Committed, name: "foobar"}],
    ['Final', false, {name: "foobar", priority: 123}],
    ['Final', false, {name: "foobar", output_uuid: "zzzzz-4zz18-znfnqtbbv4spc3w"}],