X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/6e9fcde0422b33d081e2985975e3104eb2434957..b9c5339d113c63ffc3d8a7c6bf1019616bb3f89a:/services/api/test/unit/container_request_test.rb diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb index 6f02333069..2d5c735181 100644 --- a/services/api/test/unit/container_request_test.rb +++ b/services/api/test/unit/container_request_test.rb @@ -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"}],