X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/04b9d6e93ca8bd18dca697e56689820516c8c572..7499f61a2912cfdb1a316808fafa6e6ee77ee2e0:/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 81b49ff4fc..90de800b2f 100644 --- a/services/api/test/unit/container_request_test.rb +++ b/services/api/test/unit/container_request_test.rb @@ -5,6 +5,7 @@ require 'test_helper' require 'helpers/container_test_helper' require 'helpers/docker_migration_helper' +require 'arvados/collection' class ContainerRequestTest < ActiveSupport::TestCase include DockerMigrationHelper @@ -245,18 +246,80 @@ class ContainerRequestTest < ActiveSupport::TestCase cr.reload assert_equal "Final", cr.state assert_equal users(:active).uuid, cr.modified_by_user_uuid - ['output', 'log'].each do |out_type| - pdh = Container.find_by_uuid(cr.container_uuid).send(out_type) - assert_equal(1, Collection.where(portable_data_hash: pdh, - owner_uuid: project.uuid).count, - "Container #{out_type} should be copied to #{project.uuid}") - end + assert_not_nil cr.output_uuid assert_not_nil cr.log_uuid 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_pdh, log.portable_data_hash + assert_equal log.manifest_text, ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar +./log\\040for\\040container\\040#{cr.container_uuid} 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n" + + 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 @@ -380,7 +443,7 @@ class ContainerRequestTest < ActiveSupport::TestCase [ ['running_container_auth', 'zzzzz-dz642-runningcontainr', 501], - ['active_no_prefs', nil, 0], + ['active_no_prefs', nil, 0] ].each do |token, expected, expected_priority| test "create as #{token} and expect requesting_container_uuid to be #{expected}" do set_user_from_auth token @@ -391,6 +454,15 @@ class ContainerRequestTest < ActiveSupport::TestCase end end + test "create as container_runtime_token and expect requesting_container_uuid to be zzzzz-dz642-20isqbkl8xwnsao" do + set_user_from_auth :container_runtime_token + Thread.current[:token] = "#{Thread.current[:token]}/zzzzz-dz642-20isqbkl8xwnsao" + cr = ContainerRequest.create(container_image: "img", output_path: "/tmp", command: ["echo", "foo"]) + assert_not_nil cr.uuid, 'uuid should be set for newly created container_request' + assert_equal 'zzzzz-dz642-20isqbkl8xwnsao', cr.requesting_container_uuid + assert_equal 1, cr.priority + end + [[{"vcpus" => [2, nil]}, lambda { |resolved| resolved["vcpus"] == 2 }], [{"vcpus" => [3, 7]}, @@ -432,6 +504,27 @@ class ContainerRequestTest < ActiveSupport::TestCase "path" => "/foo", } end], + [{"/out" => { + "kind" => "collection", + "portable_data_hash" => "1f4b0bc7583c2a7f9102c395f4ffc5e3+45", + "path" => "/foo"}}, + lambda do |resolved| + resolved["/out"] == { + "portable_data_hash" => "1f4b0bc7583c2a7f9102c395f4ffc5e3+45", + "kind" => "collection", + "path" => "/foo", + } + end], + # Empty collection + [{"/out" => { + "kind" => "collection", + "path" => "/foo"}}, + lambda do |resolved| + resolved["/out"] == { + "kind" => "collection", + "path" => "/foo", + } + end], ].each do |mounts, okfunc| test "resolve mounts #{mounts.inspect} to values" do set_user_from_auth :active @@ -465,9 +558,8 @@ class ContainerRequestTest < ActiveSupport::TestCase "path" => "/foo", }, } - assert_raises(ArgumentError) do - Container.resolve_mounts(m) - end + resolved_mounts = Container.resolve_mounts(m) + assert_equal m['portable_data_hash'], resolved_mounts['portable_data_hash'] end ['arvados/apitestfixture:latest', @@ -484,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.docker_image_formats = [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 @@ -503,8 +595,14 @@ class ContainerRequestTest < ActiveSupport::TestCase end end + test "allow unrecognized container when there are remote_hosts" do + set_user_from_auth :active + 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.docker_image_formats = ['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. @@ -522,7 +620,7 @@ class ContainerRequestTest < ActiveSupport::TestCase end test "use unmigrated docker image" do - Rails.configuration.docker_image_formats = ['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 @@ -541,7 +639,7 @@ class ContainerRequestTest < ActiveSupport::TestCase end test "incompatible docker image v1" do - Rails.configuration.docker_image_formats = ['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. @@ -554,7 +652,7 @@ class ContainerRequestTest < ActiveSupport::TestCase end test "incompatible docker image v2" do - Rails.configuration.docker_image_formats = ['v2'] + Rails.configuration.Containers.SupportedDockerImageFormats = ConfigLoader.to_OrderedOptions({'v2'=>{}}) # No migration link, don't return unsupported v1 image, set_user_from_auth :active @@ -593,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, @@ -668,6 +768,89 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_not_equal cr2.container_uuid, cr.container_uuid end + test "Retry on container cancelled with runtime_token" do + set_user_from_auth :spectator + spec = api_client_authorizations(:active) + cr = create_minimal_req!(priority: 1, state: "Committed", + runtime_token: spec.token, + container_count_max: 2) + prev_container_uuid = cr.container_uuid + + c = act_as_system_user do + c = Container.find_by_uuid(cr.container_uuid) + assert_equal spec.token, c.runtime_token + c.update_attributes!(state: Container::Locked) + c.update_attributes!(state: Container::Running) + c + end + + cr.reload + assert_equal "Committed", cr.state + assert_equal prev_container_uuid, cr.container_uuid + prev_container_uuid = cr.container_uuid + + act_as_system_user do + c.update_attributes!(state: Container::Cancelled) + end + + cr.reload + assert_equal "Committed", cr.state + assert_not_equal prev_container_uuid, cr.container_uuid + prev_container_uuid = cr.container_uuid + + c = act_as_system_user do + c = Container.find_by_uuid(cr.container_uuid) + assert_equal spec.token, c.runtime_token + c.update_attributes!(state: Container::Cancelled) + c + end + + cr.reload + assert_equal "Final", cr.state + assert_equal prev_container_uuid, cr.container_uuid + end + + + test "Retry saves logs from previous attempts" do + set_user_from_auth :active + cr = create_minimal_req!(priority: 1, state: "Committed", container_count_max: 3) + + 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 + + container_uuids = [] + + [0, 1, 2].each do + cr.reload + assert_equal "Committed", cr.state + container_uuids << cr.container_uuid + + c = act_as_system_user do + logc = Collection.new(manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n") + logc.save! + c = Container.find_by_uuid(cr.container_uuid) + c.update_attributes!(state: Container::Cancelled, log: logc.portable_data_hash) + c + end + end + + container_uuids.sort! + + cr.reload + assert_equal "Final", cr.state + assert_equal 3, cr.container_count + assert_equal ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar +./log\\040for\\040container\\040#{container_uuids[0]} 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar +./log\\040for\\040container\\040#{container_uuids[1]} 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar +./log\\040for\\040container\\040#{container_uuids[2]} 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar +" , Collection.find_by_uuid(cr.log_uuid).manifest_text + + end + test "Output collection name setting using output_name with name collision resolution" do set_user_from_auth :active output_name = 'unimaginative name' @@ -717,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.blob_signature_ttl.second, 10) + assert_in_delta(delete, now + Rails.configuration.Collections.BlobSigningTTL, 10) end def check_output_ttl_1y(now, trash, delete) @@ -730,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 @@ -765,7 +953,7 @@ class ContainerRequestTest < ActiveSupport::TestCase [false, ActiveRecord::RecordInvalid], [true, nil], ].each do |preemptible_conf, expected| - test "having Rails.configuration.preemptible_instances=#{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, @@ -773,7 +961,7 @@ class ContainerRequestTest < ActiveSupport::TestCase output_path: "test", scheduling_parameters: sp, mounts: {"test" => {"kind" => "json"}}} - Rails.configuration.preemptible_instances = preemptible_conf + Rails.configuration.Containers.UsePreemptibleInstances = preemptible_conf set_user_from_auth :active cr = create_minimal_req!(common_attrs) @@ -802,7 +990,7 @@ class ContainerRequestTest < ActiveSupport::TestCase scheduling_parameters: {"preemptible" => false}, mounts: {"test" => {"kind" => "json"}}} - Rails.configuration.preemptible_instances = true + Rails.configuration.Containers.UsePreemptibleInstances = true set_user_from_auth :active if requesting_c @@ -827,14 +1015,14 @@ class ContainerRequestTest < ActiveSupport::TestCase [false, 'zzzzz-dz642-runningcontainr', nil], [false, nil, nil], ].each do |preemptible_conf, requesting_c, schedule_preemptible| - test "having Rails.configuration.preemptible_instances=#{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.preemptible_instances = preemptible_conf + Rails.configuration.Containers.UsePreemptibleInstances = preemptible_conf set_user_from_auth :active if requesting_c @@ -898,7 +1086,7 @@ class ContainerRequestTest < ActiveSupport::TestCase state: ContainerRequest::Committed, mounts: {"test" => {"kind" => "json"}}} set_user_from_auth :active - Rails.configuration.preemptible_instances = true + Rails.configuration.Containers.UsePreemptibleInstances = true cr = with_container_auth(Container.find_by_uuid 'zzzzz-dz642-runningcontainr') do create_minimal_req!(common_attrs) @@ -1074,4 +1262,38 @@ class ContainerRequestTest < ActiveSupport::TestCase secret_mounts: sm) assert_equal [:secret_mounts], cr.errors.messages.keys end + + test "using runtime_token" do + set_user_from_auth :spectator + spec = api_client_authorizations(:active) + cr = create_minimal_req!(state: "Committed", runtime_token: spec.token, priority: 1) + cr.save! + c = Container.find_by_uuid cr.container_uuid + lock_and_run c + assert_nil c.auth_uuid + assert_equal c.runtime_token, spec.token + + assert_not_nil ApiClientAuthorization.find_by_uuid(spec.uuid) + + act_as_system_user do + c.update_attributes!(state: Container::Complete, + exit_code: 0, + output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45', + log: 'fa7aeb5140e2848d39b416daeef4ffc5+45') + end + + cr.reload + c.reload + assert_nil cr.runtime_token + assert_nil c.runtime_token + end + + test "invalid runtime_token" do + set_user_from_auth :active + spec = api_client_authorizations(:spectator) + assert_raises(ArgumentError) do + cr = create_minimal_req!(state: "Committed", runtime_token: "#{spec.token}xx") + cr.save! + end + end end